ALS Admin Service DELETE API not working as expected

[sri-miriyala]

Summary:
The DELETE API in ALS Admin Service is not deleting the required Oracle completely from Switch

Severity:
High

Priority:
Medium

Expected Behavior
After calling the DELETE API to delete a specific oracle id for a type, we should be able to add the same Oracle Type without any issues.

Steps to Reproduce

1. Get the list of existing Oracles using this command.

curl -L -X GET '<als-admin-serice>/oracles' \
-H 'Content-Type: application/vnd.interoperability.participants+json;version=1.0' \
-H 'Date: Thu, 13 Aug 2020 19:53:11 GMT'

2. Delete a Oracle type entry using its ID

curl -L -X DELETE '<als-admin-serice>/oracles/<ID>' \
-H 'Content-Type: application/vnd.interoperability.participants+json;version=1.0' \
-H 'Date: Thu, 13 Aug 2020 19:53:11 GMT'

3. Add the same oracle type again

curl -L -X POST '<als-admin-service>/oracles' \
-H 'Accept: application/vnd.interoperability.participants+json;version=1' \
-H 'Content-Type: application/vnd.interoperability.participants+json;version=1.0' \
-H 'Date;' \

--data-raw '{
  "oracleIdType": "<type>",
  "endpoint": {
    "value": "<value>",
    "endpointType": "URL"
  },
  "currency": "USD",
  "isDefault": true
}'

Specifications

• Component (if known): ALS service
• Version: account-lookup-service:v11.2.1
• Platform:
• Subsystem: ALS admin
• Type of testing:
• Bug found/raised by: Sridevi Miriyala

Notes:

• Severity when opened: High
• Priority when opened: High

[elnyry-sam-k]

After a quick review, it looks like the Oracle entries are marked as inactive and not deleted as such, hence the error. To fix - we need to either remove entries when DELETE call is made or 'activate' a disabled oracle if the same entry is retried on POST or use a different key (such as start date) and make every POST request for creating a new oracle a new entry..

[mdebarros]

@elnyry-sam-k, the approach of maintaining a history of oracles could be done as follows:

• Add a new migration script to reverse the following uniqueness constraints: https://github.com/mojaloop/account-lookup-service/blob/master/migrations/09_oracleEndpoint-indexes.js#L32
• Add a new Oracle record via the createOracle domain operation with an additional validation check for an existing record match ['partyIdTypeId', 'endpointTypeId', 'currencyId'] if and only if isActive=false. If a record is found with ['partyIdTypeId', 'endpointTypeId', 'currencyId'] AND isActive=true then fail the request with an appropriate error.
• Update updateOracle domain operation to only allow an oracle to be made active if and only if there is no existing record that matches ['partyIdTypeId', 'endpointTypeId', 'currencyId'] AND isActive=false, otherwise fail the request with an appropriate error.
• Update the admin-swagger.yaml and admin_swagger.json interface specs to correctly clarify this functionality.

This should allow a "history" of oracles to exists within the Database without impacting the API's functions.

This will also allow the deleteOracle domain operation to function as intended.

[kleyow]

Update updateOracle domain operation to only allow an oracle to be made active if and only if there is no existing record that matches ['partyIdTypeId', 'endpointTypeId', 'currencyId'] AND isActive=false, otherwise fail the request with an appropriate error.

A lot of this isActive logic is confusing and not used from what I'm seeing acting sort of like a private variable in a database?

https://github.com/mojaloop/account-lookup-service/blob/master/src/models/oracle/oracleEndpoint.js#L145 sets an oracle to be active but is never referenced in any src code.

The admin api interface for OracleInfo has no field for passing in any isActive type field so no update/create operations will set it to be true.
I'm not sure how any of the oracles are ever marked isActive=true for the delete operation to ever mark them isActive=false.

Nvm, its a default entry. The tests were giving me the wrong impression.

[kleyow]

Update updateOracle domain operation to only allow an oracle to be made active if and only if there is no existing record that matches ['partyIdTypeId', 'endpointTypeId', 'currencyId'] AND isActive=false, otherwise fail the request with an appropriate error.

Can you clarify this comment @mdebarros for me and what problem this solves?

made active

Do you mean setting isActive=true here? If so I don't believe updateOracle domain operation handles the changing of isActive.

[mdebarros]

Correction in bold (although its a bit hard to see):

Update updateOracle domain operation to only allow an oracle to be made active if and only if there is no existing record that matches ['partyIdTypeId', 'endpointTypeId', 'currencyId'] AND isActive=true, otherwise fail the request with an appropriate error.

Can you clarify this comment @mdebarros for me and what problem this solves?

I.e. it means you can only update an existing oracle with isActive=true IFF (if and only if) there is no other oracle with isActive=true with the these matching fields: ['partyIdTypeId', 'endpointTypeId', 'currencyId'].

Does that make more sense?

Do you mean setting isActive=true here? If so I don't believe updateOracle domain operation handles the changing of isActive.

Correct

[elnyry-sam-k]

Fixed in: https://github.com/mojaloop/account-lookup-service/releases/tag/v11.5.4.. Thanks @kleyow