Skip to content
This repository has been archived by the owner on Jul 17, 2024. It is now read-only.

Eupay 452 vrp consent event subscription #170

Merged
merged 24 commits into from
Feb 8, 2023
Merged

Eupay 452 vrp consent event subscription #170

merged 24 commits into from
Feb 8, 2023

Conversation

vivek0739
Copy link
Contributor

@vivek0739 vivek0739 commented Jan 30, 2023 •
edited
Loading

Context

To Improve the VRP experience, we want to get notified when User cancelled the VRP consent through ASPSP.
Open Banking provide Event API through which ASPSP can notify us whenever User revoked the consent.

Changes

  • Added event-openApi.yaml spec.
  • Implemented subscribeToAnEvent, getAllEventResources, changeAnEventResource, deleteAnEventResource
  • Unit test for each method.

ulianaostapenko
ulianaostapenko suggested changes Jan 31, 2023
View reviewed changes
specs/events-openapi.yaml Outdated Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/EventClient.java Outdated Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/EventClient.java Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/EventClient.java Outdated Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/RestEventClient.java Outdated Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/RestEventClient.java Outdated Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/RestEventClient.java Outdated Show resolved Hide resolved
@ulianaostapenko
Copy link
Contributor

Don't forget to bump version in gradle properties here

vivek0739 added a commit that referenced this pull request Feb 3, 2023
@vivek0739
fix for comments in #170
b6f0d1f
@vivek0739 vivek0739 closed this Feb 6, 2023
@vivek0739
Copy link
Contributor Author

Don't forget to bump version in gradle properties here

Updated in c8bf287

@vivek0739 vivek0739 reopened this Feb 6, 2023
ulianaostapenko
ulianaostapenko suggested changes Feb 7, 2023
View reviewed changes
Copy link
Contributor

@ulianaostapenko ulianaostapenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update CHANGELOG file

src/main/java/com/transferwise/openbanking/client/api/event/EventClient.java Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/EventClient.java Outdated Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/RestEventClient.java Outdated Show resolved Hide resolved
src/main/java/com/transferwise/openbanking/client/api/event/RestEventClient.java Outdated Show resolved Hide resolved
src/test/java/com/transferwise/openbanking/client/api/event/RestEventClientTest.java Outdated Show resolved Hide resolved
src/test/java/com/transferwise/openbanking/client/api/event/RestEventClientTest.java Outdated Show resolved Hide resolved
src/test/java/com/transferwise/openbanking/client/api/event/RestEventClientTest.java Outdated Show resolved Hide resolved
ulianaostapenko
ulianaostapenko reviewed Feb 7, 2023
View reviewed changes
Copy link
Contributor

@ulianaostapenko ulianaostapenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash all your commits before I can approve it as we cannot force push to master and you have 17 commits in the PR.
Also please rebase/merge master to get latest changes

vivek0739 added a commit that referenced this pull request Feb 7, 2023
@vivek0739
fix for comments in #170
d4a4f54
@vivek0739 vivek0739 force-pushed the EUPAY-452-vrp-consent-event-subscription branch 2 times, most recently from d1f4638 to 00a5220 Compare February 7, 2023 16:44
@vivek0739 vivek0739 self-assigned this Feb 7, 2023
@vivek0739 vivek0739 added the enhancement New feature or request label Feb 7, 2023
@vivek0739
Copy link
Contributor Author

vivek0739 commented Feb 7, 2023 •
edited
Loading

Please squash all your commits before I can approve it as we cannot force push to master and you have 17 commits in the PR.
Did you mean squash and merge? which I will do when merging. I didn't understand. You want to squash all these changes into one commit in this branch EUPAY-452-vrp-consent-event-subscription itself.
We will all commits informations.

Also please rebase/merge master to get latest changes
I have rebased to master now.

@ulianaostapenko
Copy link
Contributor

Please make sure you answered and resolved all my questions in this PR.

Did you mean squash and merge? which I will do when merging. I didn't understand. You want to squash all these changes into one commit in this branch EUPAY-452-vrp-consent-event-subscription itself.
We will all commits informations.

I mean squash them in the brach as you can make a mistake and choose merge instead squash & merge so all this commits will go to master. It has happen before, we cannot rewrite history on master and it is public library I advice you to squash them before. You can still have all your commit messaged listed in the commit.

@vivek0739 vivek0739 marked this pull request as ready for review February 8, 2023 13:50
@vivek0739 vivek0739 requested a review from a team as a code owner February 8, 2023 13:50
ulianaostapenko
ulianaostapenko previously approved these changes Feb 8, 2023
View reviewed changes
Copy link
Contributor

@ulianaostapenko ulianaostapenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vivek0739
Part 1 - [Open Banking] [Event Subscription] Adding Event subscriptio…
1ebb044 
…n API spec.

Added specs from:
https://github.com/OpenBankingUK/read-write-api-specs/blob/8a1f99fbe443c7c7eba2eff933e7f952811941c6/dist/openapi/events-openapi.yaml
https://github.com/OpenBankingUK/read-write-api-specs/blob/8a1f99fbe443c7c7eba2eff933e7f952811941c6/dist/openapi/payment-initiation-openapi.json

build using:
./gradlew build

Generated following java class:

                        ├── Links.java
                        ├── Meta.java
                        ├── OBError1.java
                        ├── OBErrorResponse1.java
                        ├── OBEventPolling1.java
                        ├── OBEventPolling1SetErrs.java
                        ├── OBEventPollingResponse1.java
                        ├── OBEventSubscription1.java
                        ├── OBEventSubscription1Data.java
                        ├── OBEventSubscriptionResponse1.java
                        ├── OBEventSubscriptionResponse1Data.java
                        ├── OBEventSubscriptionsResponse1.java
                        ├── OBEventSubscriptionsResponse1Data.java
                        └── OBEventSubscriptionsResponse1DataEventSubscription.java

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[Open Banking] [Event Notification] Fixing openapi config with correc…
06af892 
…t attributes.

I had used  open API JSON spec in event-openapi-config but it should contain
attrbibutes which will help swagger in generating correct package and format.
format is similar what was there in  VRP (vrp-openapi-config.json)

There were some missing entries in gradle.build for currect formating of auto generating files
I have added those entries too  (similar to VRP)

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[Refactoring] [OpenBankingClient] [BasePaymentClient] Adding BASE_API…
de4f4ff 
…_MAJOR_VERSION constant in file to use in multiple API

Usages: generateApiUrl, generateVrpApiUrl

Why? checkstyleMain produce error if some constant are repeated for 3 types. String "3" was passed in above 2 API and I am creating a new function for EventAPI.

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Part 2 [Open Banking] [Event Notification] Adding EventSubscription A…
db96912 
…PI in RestEventClent

EventClient is interface.
RestEventClient is implementation of EventClient and logic to call ASPSP reside here.
EventApiCallException is exception thrown in case EventSubscription call error.
BasePaymentClient contains the URL generation util function for ASPSP event API.

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
EUPAY-452 Part -3 [OpenBanking] [Event Notification] Adding test for …
5824c50 
…subscribeToAnEvent.

Used:
1. MockRestServiceServer
2. Mockito to mock OAuthClient and jwtClaimsSigner.

Verified the following.

URL : https://aspsp.co.uk/open-banking/v3.1/event-subscriptions
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Making RestEventClient Constructor public to be used in PISP service
549c9e3 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[OB] [Event] Implementing getEventResources API.
2eb7250 
it calls to:
GET /event-supscriptions

Data Model:
Response: OBEventSubscriptionsResponse1

Test:
verified GET URL: https://aspsp.co.uk/open-banking/v3.1/event-subscriptions

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[OB] [Event] Implementing Event resource Delete API
7181a1a 
URL:
DELETE /event-subscriptions/{event-subscription-id}

DataModel
No Model is requested or returned.

Result:
Verify 200 code.

Test
URL /event-subscriptions/{event-subscription-id} is called.
Test somehow is not working, Will fix in later commit.

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
fix for comments in #170
b8ed399 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[UK][OB][Event] Fixing some exceptions string and removing unused import
69725ff 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[UK][OB][Event] Adding Override annotation for getEventResources in R…
a16e62d 
…estEventClient

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
fixing delete event subscription API Endpoint URL
d325835 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[UK][OB][Event] Implementing Event resource Change API
cac816d 
URL:
PUT /event-subscriptions/{EventSubscriptionId}

Data Model:
Request: OBEventSubscriptionResponse1
Response: OBEventSubscriptionResponse1

Test:
RestEventClientTest::changeEventSubscriptions
Verified URL is correct.

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Fixing Javadoc
4fe6c66 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Renaming getEventResources to getAllEventResources
b06b404 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Updating Gradle version 11.1.0 -> 12.0.0
ca5f5f0 
Description:
Considering it as a major version as we are provide support for a new Event API

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Refactoring RestEventClientTest to remove PMD error
2b6e8e3 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
formatting based on comment in PR
116d81b 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Adding 12.0.0: Openbanking Event API change in Changelog
255cdc4 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Fixing CheckStyle Error
cd90ec0 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Format: breaking some longer line into smaller line
3474f89 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Removing checkstyle:variabledeclarationusagedistance from RestEventCl…
87bcdf3 
…ientTest

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
[OpenBanking][Event] Changing x-x-namespaced-enum to enum
3b2bd3d 
Why?

We need to unified error handling for VRP, PISP and Event. Enum will be handy to merge them.

Guarantees:
Bank have option to send a new Custom error string which is not listed in enum. But they will introduce a new version for any update.
We can raise ticket to bank for any custom changes.

Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739
Removing extra whitelines from RestEventClientTest
002e5cb 
Signed-off-by: Vivek Kumar <[email protected]>
@vivek0739 vivek0739 force-pushed the EUPAY-452-vrp-consent-event-subscription branch from ca2a82e to 002e5cb Compare February 8, 2023 14:53
@vivek0739 vivek0739 merged commit 03561da into master Feb 8, 2023
@vivek0739 vivek0739 deleted the EUPAY-452-vrp-consent-event-subscription branch February 8, 2023 15:29
@vivek0739
Copy link
Contributor Author

Merged with Master!
Description is branch name. We have just added support event subscription API. That's all we need in Open Banking client.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ulianaostapenko ulianaostapenko ulianaostapenko approved these changes

Assignees

@vivek0739 vivek0739

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vivek0739 @ulianaostapenko