33571 support merging zos connect's OpenAPI extension between multiple docs #33577
Conversation
|
!build (view Open Liberty Personal Build - ❌ completed with errors/failures.) Note: Target locations of links might be accessible only to IBM employees. |
Code analysis and actionsDO NOT DELETE THIS COMMENT.
|
benjamin-confino
changed the title
There was a problem hiding this comment.
This all looks good 👍
We do have unit tests for MergeProcessor in the io.openliberty.microprofile.openapi.2.0.internal_test project. There's some slight hackiness to run the same tests on mpOpenAPI-2.0 and 4.0, you have to run them from the MergeTests suite but it should be quicker to set up and run than adding FAT tests.
There are a few more tests I think we should add, these could just be unit tests:
- two apps with conflicting
x-ibm-zcon-roles-allowedand a third app withoutx-ibm-zcon-roles-allowed - one app with
x-ibm-zcon-roles-allowedand a second app withoutx-ibm-zcon-roles-allowed- In this case I think
x-ibm-zcon-roles-allowedshould still be moved under the operations (which I don't think the current code will do)
- In this case I think
| @RunWith(FATRunner.class) | ||
| public class ZOSConnectExtensionTest { |
There was a problem hiding this comment.
I think this should be a FULL mode test (especially if we add unit tests)
|
!build (view Open Liberty Personal Build - ❌ completed with errors/failures.) Note: Target locations of links might be accessible only to IBM employees. |
Code analysis and actionsDO NOT DELETE THIS COMMENT.
|
| private static boolean isZConRolesAllowedIdentical(List<InProgressModel> models) { | ||
| List<Object> zconExtensions = models.stream().map(d -> d.model.getExtensions()) | ||
| .filter(Objects::nonNull).map(e -> e.get("x-ibm-zcon-roles-allowed")).collect(toList()); | ||
| return models.size() == zconExtensions.size() && allEqual(zconExtensions, ModelEquality::equals); |
There was a problem hiding this comment.
Wouldn't this return false if there are no zcon roles in any model?
There was a problem hiding this comment.
Looking closer, I think yes, but only if at least one model has >=1 extensions other than x-ibm...
benjamin-confino
force-pushed
the
33571-zos-connect
branch
from
ba79586 to
90e0536
Compare
3 participants
release buglabel if applicable: https://github.com/OpenLiberty/open-liberty/wiki/Open-Liberty-Conventions).ZOS has their own non-standard roles attribute, this adds support for merging it between multiple OpenAPI docs.
fixes #33571