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

fix: OpenAPI - object list extraction [DHIS2-17200] #19320

Merged
merged 5 commits into from
Nov 29, 2024
Merged

Conversation

jbee
Copy link
Contributor

@jbee jbee commented Nov 27, 2024

Summary

The CRUD getObjectList method has 2 major complications for the OpenAPI analysis

  • it uses a type variable as parameter object
  • it has a "virtual" response type that gets substituted based in the current value of @EntityType

Type variable parameter objects were not supported before so a simple version was added that only supports this particular case.

The response broke due to too many properties escaping the parameter type analysis as it would pick up a property from the annotated field (which was renamed) and another one from the getter (which lacked the renaming annotation and therefore wasn't ignored). Therefore the de-duplication is now based on the Java property name, not the name the property gets after considering potential renames from an annotation.

I also added OpenAPI descriptions to all special parameters.

Shared Parameter and Inheritance

Previously there was no support for inheritance which worked but would result in each subclass repeating (duplicating) the parameter properties defined in supertypes. With the changes in this PR a shared parameter will use the declaring class simple name as the "namespace" if no explicit name is defined via @OpenApi.Shared(name="SchemaName").

Automatic Testing

Added a few new OpenAPI tests to make sure the added or fixed features keep working

Manual Testing

  • check e.g. /api/openapi/openapi.html?scope=entity:Attribute&skipCache=true&source=true&scope=entity:OrganisationUnit#OrganisationUnit.getObjectList , there should be plenty of parameters with description, the return type should be summarized as <object{#,organisationUnits:array[OrganisationUnit]}>
  • check e.g. /api/openapi/openapi.json?scope=entity:OrganisationUnit if you know OpenAPI speak

@jbee jbee self-assigned this Nov 27, 2024
@@ -304,6 +304,8 @@ private Query<?> getUserQuery(UserQueryParams params, List<String> orders, Query
hql += hlp.whereAnd() + " u.selfRegistered = true ";
}

// TODO(JB) shoundn't UserInvitationStatus.NONE match "u.invitation = false" and null mean no
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: I left this intentionally for another PR

@@ -217,7 +255,12 @@ private UserQueryParams toUserQueryParams(GetUserObjectListParams params) {
res.setInvitationStatus(params.getInvitationStatus());
res.setUserOrgUnits(params.isUserOrgUnits());
res.setIncludeOrgUnitChildren(params.isIncludeChildren());
res.setOrgUnitBoundary(params.getOrgUnitBoundary());
String ou = params.getOu();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: moving this code here is in a way a bugfix so that the orgUnitBoundary parameter can be used and is not always overridden by the effect of addOrganisationUnit.

@jbee jbee marked this pull request as ready for review November 28, 2024 15:52
@jbee jbee enabled auto-merge (squash) November 28, 2024 17:36
@jbee jbee merged commit 4f4187f into master Nov 29, 2024
15 checks passed
@jbee jbee deleted the DHIS2-17200-openapi branch November 29, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants