Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mobility Layer2 config: check out the issues with validation with openAPI version 3.1 #266

Closed
3 of 4 tasks
rajaneeshk90 opened this issue Sep 5, 2024 · 10 comments
Closed
3 of 4 tasks
Assignees
Labels
beckn onix beckn onix project

Comments

@rajaneeshk90
Copy link
Collaborator

rajaneeshk90 commented Sep 5, 2024

Description

We have a sample layer2 config for the mobility domain. Layer2. This layer2 config uses the opneAPI version 3.1.

Beckn Protocol server uses the "express-openapi-validator" module internally for request validations.

To test out the above layer2 config with Beckn protocol server, Tanya upgraded her local instances of the protocol server to a new version of express-openapi-validator library which supports openAPI 3.1.

This version was used for the validations.

While doing the testing, we faced some issues, as part of this ticket, we need to investigate those issues and find solutions.

Key Points:
Protocol server uses node module "express-openapi-validator" for the validations.
While developing the l2 config, We used node module "openapi-backend" module for the validations.

There might be some differences between how these libraries do the validations.

Goals

  • Tanya to share the list of the exact issues.
  • Investigate the issues faced with the validations.
  • Create a postman collection with the positive and negative test cases.
  • Validate the positive and negative test cases in the postman collection with a test server.

Expected Outcome

No response

Acceptance Criteria

No response

Implementation Details

NA

Mockups/Wireframes

No response

Product Name

protocol server

Domain

No response

Tech Skills Needed

Express.js, JavaScript, Other

Complexity

High

Category

Research

@rajaneeshk90 rajaneeshk90 self-assigned this Sep 5, 2024
@rajaneeshk90
Copy link
Collaborator Author

From what was discussed on the call, we tested the below items:

  • Reference errors: I checked the validity of the mobility layer2 config in the https://editor-next.swagger.io/ and https://json-schema.hyperjump.io/ tools, and both of these are considering it a valid openAPI doc. Also I have been using a lightweight node server to validate the rules in the layer2, that server also is considering the doc as valid.
  • allOf inside if block is not working: I tried a search request in POSTMAN and the backend is able to successfully enforce the rules added in the layer2.
  • Action enum should not be in the layer2: This is a trivial change, this can be easily removed.
  • Search not using the rules from the shared directory: Search has a different message structure than other APIs so as per the things we have explored till now it cant use the shared rules.

There could be additional issues that I might have missed. But from my testing I found these things to be working.
The mismatch could be due to the different library being used.

Sent a mail to Tanya to get the details of the issues so that the team can dig deeper into it.

@rajaneeshk90
Copy link
Collaborator Author

rajaneeshk90 commented Sep 12, 2024

Extended the postman collected which is used to test the layer2 configuration on the test server (not the protocol server instance)

This was used while developing the layer2 as the existing protocol server did not have support for openAPI 3.1 at the time.

This could be used for testing when we implement the openAPI 3.1 in protocol serevr.

Took considerable time to add the cases and test.

OAS3.1-validator-Mobility-Test-Server.postman_collection.json

@rajaneeshk90
Copy link
Collaborator Author

cc @emmayank

@rajaneeshk90
Copy link
Collaborator Author

rajaneeshk90 commented Sep 13, 2024

All the test cases tested on the l2 config with the test server are listed in this file:

https://docs.google.com/spreadsheets/d/1tEp8TxpSfuOLXimGH3gXIFWIxvffZoWW1fsaoEMuY9E/edit?gid=0#gid=0

@rajaneeshk90
Copy link
Collaborator Author

cc @faizmagic

@rajaneeshk90
Copy link
Collaborator Author

rajaneeshk90 commented Sep 16, 2024

Tanya have not shared the list or the details of the exact issues yet, we investigated the L2 based on the verbal feedback she had given on a call.

@rajaneeshk90
Copy link
Collaborator Author

Had a call with Tanya on 16th of September.

I Demonstrated my postman collection and showed that the contains and allof combination is working for validation.

Unfortunately, we could not continue the call, as she had to drop off. Nevertheless I was able to show that contains and allof combination works fine.

shared with her my postman collection, she can use the postman to try it out offline.

shared with her the ajv version that the openapi-backend library is using.

@rajaneeshk90
Copy link
Collaborator Author

Had a discussion with Abhishek Yadav regarding these issues.

  • Discussed how to create a sample rule in the yaml so that when it is required while testing the layer2 yaml, they can do it.
  • provided the mobility layer2 config and added it in the protocol server local code.
  • tested the changes locally, encountered reference issue. Abhishek narrowed it down to the problem with using the duplicate "id" field in the examples in yaml. Its giving error for a duplicate across the whole document. i.e we have used "id": "1", in on_search, on_select, on_init etc responses, that should not be a problem but openapi-express-validator is treating that as reference error.
  • created a new layer2 yaml without the examples in it, used that in the local protocol server code. The reference error not thrown this time.
  • shared with him the postman collection with positive and negative test cases in it. We tried the cases on the local protocol server. It was not validating some of the rules added in the layer2.
  • We tried the same postman collection on the backend that we have provisioned which uses "openapi-backend" library, that backend is validating the rules successfully.

Based on this exercise it is clear that "openapi-express-validator" library has some issues in it when testing out the openAPI version 3.1. as the "openapi-backend" library is able to validate the requests successfully using the same layer2.

Asked Abhishek to research more on it and decide upon the way forward from here.

@rajaneeshk90
Copy link
Collaborator Author

rajaneeshk90 commented Sep 18, 2024

Had a call with Tanya, she highlighted some of the stuffs that she had tested with layer2 config:

She tried 3 ways.

  • using express-openapi-validator3.1 with protocol server
  • creating a custom backend which uses latest ajv
  • the test backend (oas-validator.becknprotocol.io:4000/mobility/) shared by us which uses openapi-backend library

out of the above 3, the later 2 are validating the rules added in the layer2 config as expected
The syntax that we are using in the current layer2 config is fine, it seems that the express-openapi-validator3.1 library has some issues in their implementation.

Next step is to finalise one way we are going to use to add openapi3,1 support in the protocol server based on the above info.

we can either:

  • try to fix the issues in the openapi-express-validator3.1
  • try to create a custom middleware using ajv
  • try to use openapi-backend library in the protocol server
  • or research about any other library

This decision is to be taken by the Dev team. Tanya wants to connect to discuss the same, as she has done considerable testing with this, her inputs might be helpful.

@rajaneeshk90
Copy link
Collaborator Author

Issue exploration and the necessary discussions are done, further progress related to this can be tracked in this ticket.

@rajaneeshk90 rajaneeshk90 transferred this issue from beckn/protocol-server Oct 15, 2024
@rajaneeshk90 rajaneeshk90 added the beckn onix beckn onix project label Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beckn onix beckn onix project
Projects
None yet
Development

No branches or pull requests

1 participant