OAGi / Score

Score
MIT License
9 stars 6 forks source link

OpenAPI generated expression: Extra schemas created, two unused, same referenced BIE should not have be duplicated but set as global property #1591

Closed dubnemo closed 8 months ago

dubnemo commented 11 months ago

This was previously working.

image

Since the same BIE was referenced across both GET response and POST request, only shippedItemInstanceArray and shippedItemInstanceArrayEntry should have been created. image

Interestingly, the createShippedItemInstance and queryShippedItemInstance have the root property suppressed.

elena-jelisic commented 10 months ago

@dubnemo @hakjuoh @kbserm I have reverted changes made OpenAPI functionality, and here are the results.

  1. The Open API document content for "AgGateway In-Field Product Id" on the oagiscore instance looks like this:
Screenshot 2024-01-11 at 11 17 54 PM
  1. Next is the .yml file generated from the current oagiscore instance:

after_x-oagis-* properties_changes_AgGateway In-Field Product Id-V4-1705032079871.yml.zip

  1. Next is the .yml file generated on my local instance after I have reverted all changes previously made (according to issue #1574):

before_x-oagis-* properties_changes_AgGateway In-Field Product Id-V4-1705032041734.yml.zip

Can you please take a look and check if the correct .yml file is generated? Except the expected differences regarding x-oagis-.properties, these two .yml files are the same.

dubnemo commented 10 months ago

@hakjuoh @elena-jelisic @kbserm I had suggested simply using the previously 3.2.0 release, not just reverting changes related to #1574. I was not 'blaming' that work -- its just that I am not aware of all the changes in 3.2.1 that have could have affected behavior. Or data changes with respect to an OAGIS release.

dubnemo commented 10 months ago

I cannot unzip the attached file. Either way, if the file looks the same as we looked at the other day, it is NOT properly generated and something is wrong.

image

elena-jelisic commented 10 months ago

@dubnemo I am uploading the files again. Let me know if it works for you.

beforex-oagis- properties_changes_AgGateway In-Field Product Id-V4-1705032041734.zip

afterx-oagis- properties_changes_AgGateway In-Field Product Id-V4-1705032079871.zip

dubnemo commented 10 months ago

@elena-jelisic
The zips are OK this time. As you state, other than the extension, the files are relatively the same.

So there must be something in the data that is causing this behavior or other code changes in that release. Have you looked at Bingqi's older issues related to the requirements on what the yml should look like? It may seem a bit daunting.

Perhaps you can review the DB records for Traceability API (Original) vs. In-Field Product Id. The Traceability API express well. Are there columns that are not populated in the In-Field Product ID API that are populated in the Traceability API?

Lastly did you see my comment prior to the correct zip comment?

@hakjuoh @kbserm

dubnemo commented 10 months ago

I will try to find key Slack messages like this:

image

image

dubnemo commented 10 months ago

Can you explain what guid is displayed here?

image

I cannot find it in the yaml: image

I previously suggest to Bingqi to display the URI to the BIE; e..g, https://oagiscore.net/profile_bie/860

elena-jelisic commented 10 months ago

Can you explain what guid is displayed here?

image

I cannot find it in the yaml: image

I previously suggest to Bingqi to display the URI to the BIE; e..g, https://oagiscore.net/profile_bie/860

@dubnemo @hakjuoh @kbserm I found out that the GUID of the ASCCP is displayed on the UI. Meanwhile, the GUID of the associated ABIE is shown in the YAML file. That is odd. Was there a reason to display ASCCP/ABIE GUIDs instead of ASBIEP GUID?

dubnemo commented 10 months ago

@elena-jelisic I just tested in test.oagiscore.net, and I still see the same issues:

image I am available today 2pm Central.

@hakjuoh

elena-jelisic commented 10 months ago

@dubnemo Sorry, Scott, I just saw you message. It turned out that the test instance didn't have the latest code. @hakjuoh just uploaded the latest version. Can you please test it again? We apologize for this.

This is a screenshot how it looks like on my local.

Screenshot 2024-01-26 at 2 51 57 PM

elena-jelisic commented 10 months ago

@dubnemo We have another question. As you can see in the generated Open API document there is the following error message:

Screenshot 2024-01-26 at 3 10 52 PM

The current logic works with either the 'sinceLastDateTime' or 'id' parameter when the array indicator is checked or not, respectively. And as you can see in the data:

Screenshot 2024-01-26 at 3 06 45 PM

The array indicators for these shipped item instances are checked and it has '{id}' path parameter in the resource name.

Can you please confirm if the current logic is fine or we need to fix it?

@hakjuoh

hakjuoh commented 10 months ago

The array indicators for these shipped item instances are checked and it has '{id}' path parameter in the resource name.

Can you please confirm if the current logic is fine or we need to fix it?

@hakjuoh

@elena-jelisic @dubnemo Previously, the logic takes either the 'sinceLastDateTime' query parameter if the array indicator is checked or the 'id' path parameter if the resource name has the 'id' parameter in its path. I fixed this behavior, now the path can have both parameters.

Screenshot 2024-01-26 at 3 52 26 PM

I'm unsure whether we'll use the array Indicator and id parameter together in practice, though.

dubnemo commented 10 months ago

Parameters

I told @bingqi2045 that the query and path logic would be enhanced when we revisit #862. @hakjuoh yes, it is related to the array selection, AND it was intended only when BIE selections are HTTP verb = GET and the Message Body = response body. I am not sure why sinceLastDateTime was a path parameter, but perhaps a simple fix is to check those values and suppress sinceLastDateTime for all other verb-combinations.

I LIKE the check to see if the {id} is in the resource path.

Duplicate schema objects

@elena-jelisic I have checked test.oagiscore.net, and it is generally working now. If you recall in the meeting, I thought it may be valuable to cache or use a hashtable to store the top level bie id value and check the hashtable if the specific BIE has already been expressed. Here is a duplicate (id=896):

replaceShippedItemInstanceListEntry:
  x-oagis-bie-guid: "db527237f0f14307bf0ba0f5935cd94d"
  x-oagis-bie-date-time: "2023-12-26T20:24:28+0000"
  x-oagis-bie-version: "4.0"
  x-oagis-bie-uri: "https://test.oagiscore.net/profile_bie/896"
  x-oagis-release: "10.10.2"
  x-oagis-release-date: "2023-12-18"
  type: "object"
  additionalProperties: false

queryShippedItemInstanceListEntry:
  x-oagis-bie-guid: "db527237f0f14307bf0ba0f5935cd94d"
  x-oagis-bie-date-time: "2023-12-26T20:24:28+0000"
  x-oagis-bie-version: "4.0"
  x-oagis-bie-uri: "https://test.oagiscore.net/profile_bie/896"
  x-oagis-release: "10.10.2"
  x-oagis-release-date: "2023-12-18"
  type: "object"
  additionalProperties: false
hakjuoh commented 9 months ago

@hakjuoh yes, it is related to the array selection, AND it was intended only when BIE selections are HTTP verb = GET and the Message Body = response body. I am not sure why sinceLastDateTime was a path parameter, but perhaps a simple fix is to check those values and suppress sinceLastDateTime for all other verb-combinations.

@elena-jelisic The 'sinceLastDateTime' query parameter is allowed for only the 'GET' verb. https://github.com/OAGi/Score/blob/97157b100587db9656d22128cbe4f6d1e5c60b7e/score-http/score-http/src/main/java/org/oagi/score/gateway/http/api/oas_management/service/generate_openapi_expression/OpenAPIGenerateExpression.java#L171-L185

dubnemo commented 9 months ago

@elena-jelisic While you are in there, please default line 187 .put("description", "Returns resources that have been updated since the last time the endpoint has been called") @hakjuoh

dubnemo commented 9 months ago

@hakjuoh @elena-jelisic I remember talking with Bingqi about the ORDER BY when retrieve the BIE list when generating the OpenAPI document. It does not seem there is an order by at all or it is not the best approach as you see the scrambling in the image below. Using ORDER BY Tag Name ASC, Operation ID ASC will provide the best results for ordering in the YML file. For example, the BOM schema objects would be next to each other where they are not today. image

Second I think it would helpful to have a simpler naming convention for the schema objects, and not use the DEN.

ABNF: <schemaObject Name> ::= <resourceName>-<messageBodyType>-[entry]

NOTE: '-entry' is optional, only when the <resourceName> text includes '-list'. See examples

The DEN be part of an extension (x-oagis-bie-den):

      x-oagis-bie-guid: "bce7462b32d549e48a4f029c4e46f102"
      x-oagis-bie-date-time: "2023-11-15T22:54:29+0000"
      x-oagis-bie-version: "V2"
      x-oagis-bie-den: "WIP Status"
      x-oagis-bie-uri: "https://test.oagiscore.net/profile_bie/883"
      x-oagis-release: "10.10.1"
      x-oagis-release-date: "2023-11-01"

Example schema objects: So ignoring the ASCCPs that reused with in the selected BIEs, instead of the fourteen (14) I see in the yml, of the six (6) displayed BIEs below, there should only be eight (8) schema objects appearing in this sequence:

List of BIEs for the OpenAPI document: image