ejp-rd-vp / vp-portal-issues

0 stars 0 forks source link

Beacon catalogs query returns 500 internal server error when unsupported filters are used #39

Closed ammarbarakat closed 1 year ago

ammarbarakat commented 1 year ago

Describe your problem.

The beacon catalogs query is currently returning a 500 internal server error when unsupported filters are used. However, the expected behavior should be to ignore these unsupported filters and provide a "warnings" section in the response, describing which filters were ignored due to lack of support.

Describe the solution you'd like

The API should handle queries with unsupported filters gracefully. Instead of returning a 500 internal server error, the server should process the supported filters and respond with a "warnings" section in the response. This section should indicate which filters were ignored because they are not supported.

Screenshot 2023-07-19 132048

ammarbarakat commented 1 year ago

@rini21 & @Orphanet , I'm not entirely sure which one of you is handling this endpoint, but could one of you please take a look at the issue? Any assistance you can provide would be greatly appreciated.

rini21 commented 1 year ago

@Ammarbr thanks for raising this issue. I have checked and can confirm that I do not have access to this endpoint, so I assume this is something @Orphanet should be able to help you with.

Orphanet commented 1 year ago

Hi, we will check the beacon catalog specifications for the appropriate response code in this case. Thanks

OuchenneB commented 1 year ago

Dear @ammarbarakat : Following the beacon specification (see the attached picture), the type of 'value' field is 'String'. So, if you replace "value":["NCIT_C124294"] by "value": "[NCIT_C124294]" you will not have a problem form the Orphanet API Side. Please not also that, the Orphanet API handle also queries with unsupported filters and ignore them.

Dear @rini21 could you point me to the "warnings" section in the response if any filter is not supported by our API. StringRep

svituz commented 1 year ago

Dear @OuchenneB sorry if I jump in, but I don't think this is the right behavior. The correct way to send the array should be using ["value1", "value2"] as the VP is doing. At least, this is what vp-api-specs explains. Here you can see the example for Ontology Filter, but for obo:NCIT_C28421 should be similar.

OuchenneB commented 1 year ago

Dear @svituz, Totally agree with you. I only followed the specs. but in this case, the specification must be changed. the "value" field must be changed to array type instead of string.

ammarbarakat commented 1 year ago

Thank you, @OuchenneB , for your response!

Based on the vp-api-specs, the "value" attribute has two accepted options:

  1. A String: This means the user want to use the Logical AND operator with the next filter.
  2. An Array of Strings: This implies that the user is using the Logical OR operator for all filter specifications in the array.

Currently, when I send a single string within an array to this endpoint, it returns a "500 internal server error." However, this behavior seems illogical, as logically sending one string in an array is equivalent to saying "A OR A," which should be evaluated to just "A" rather than throwing an "500 internal server error."

OuchenneB commented 1 year ago

Dear @ammarbarakat , I fixed that issue. However, the 'formal' specification https://app.swaggerhub.com/apis/VM172_1/vp_individuals/v0.2#/Query%20Endpoints/catalogs_request indicates that the type of 'value' field is 'String'. So, it has to be changed.

rini21 commented 1 year ago

Dear @OuchenneB, @ammarbarakat many thanks for flagging this up. We have now updated the spec to properly reflect the documentation on Swagger and GitHub. Please feel free to feedback.

ammarbarakat commented 1 year ago

Thank you all so much for your help in resolving this issue.