REST WS: Consistent way for filtering in/out retired metadata?

Looking at a couple of resources in REST WS there seems not to be clear rules as to how filtering retired entities is done.

Example 1 - VisitType: Only unretired visit types can be searched, see here.

Example 2 - Provider: There is no way to exclude retired providers during searches, see here.
(It basically leverages the upstream Core service method that always sets includeRetired to true, here.)

On one resource there is no way to filter out the retired instances, on the other they are always stripped out. How should we approach this in the general case?

Shouldn’t it be up to MetadataDelegatingCrudResource to handle this in a general way for all metadata resources? As in providing a consistent way to filter or not retired metadata from GET requests.
Or am I missing some search handler that already does this somehow?

Cc @dev5 @shruthipitta

All metadata resources should support includeRetired. If there are inconsistencies like those you mentioned, we need to address them.

I’m in support of addressing the issue in a general way for all metadata, by implementing doGetAll and doSearch in MetadataDelegatingCrudResource and getting rid of most (if not all) custom implementations. To make it efficient we would need to make direct Hibernate queries, instead of calling service methods to fetch data. Doing it with Hibernate queries also means we can implement efficient paging on the db level, since our service methods rarely implement paging and we are forced to do it in memory.

Ok I see that there is method ResourceContext.getIncludeAll() that can be used in any resource’s doSearch(ResourceContext ctx)

@shruthipitta for info this is how this parameter is passed to the context, so from the client-side.
This is an example where it is used with DrugResource.


@raff, you are looking at something a lot bigger here. It makes a lot of sense but that’s taking us quite further into design discussions. I guess for the sake of what we need we will go through a quick fix, still leveraging the usual Core services and the includeAll=false parameter.

I’m happy to keep brainstorming though…

For the time being:
RESTWS-696 - ProviderResource to process includeAll parameter

I’m not sure if it’s possible to generically do this for all metadata, but the convention in our rest API is to always exclude retired items by default unless includeAll is set to true, the provider resource might have just slipped through the cracks and needs to be fixed.

I disapprove of having our REST API bypass our Java API. If we need better/more consistent/more performant queries in openmrs-core we should add them. I doubt that our search methods do crazy stuff like this, but it’s not really okay to assume this.

+1

Thanks for clarifying @wyclif, I have amended the ticket accordingly.

i’m wokring on RESTWS-696

is there a similar convention in Context when calling getAllProviders() ?

@dkayiwa, just writing here again what I said on IRC earlier. We encountered a weird problem where SwaggerSpecificationCreatorTest starts failing because of a new test that we brought into omod-2.0 (ProviderController2_0Test see here).

Both tests pass fine when run in isolation, but when launching the full build, SwaggerSpecificationCreatorTest eventually fails.

Would you mind trying a mvn clean install on your side on this branch? In your openmrs-module-webservices.rest folder that’d be:

git remote add chinzou https://github.com/chinzou/openmrs-module-webservices.rest
git fetch chinzou
git checkout RESTWS-696
mvn clean install

@zouchine rightfully spotted that what upsets the SwaggerSpecificationCreatorTest is the fact of using lambda expressions in the other one.

The convention in our Java API is:

  • getAll[Data] => excludes voided by default
  • getAll[Metadata] => includes retired by default

The convention is our REST API is:

  • GET dataresource => excludes voided by default
  • GET metadataresource => excludes retired by default

Note that the treatment of metadata is different in our Java and REST APIs.

Providers are metadata, so getAllProviders() in Java should include retired ones, but GET provider in REST should exclude them.

1 Like

Thanks @darius, that’s crystal clear.

I found this on the wiki regarding the REST API:

Including Retired and Deleted data

A normal REST query will not include Retired metadata or Deleted/Voided data. You may send an includeAll=true query parameter to include retired/deleted data.

I added this just under:

image

Also for into I created RESTWS-700 - VisitTypeResource to process includeAll parameter.

Also could anyone change the issue type of RESTWS-696 for it to become a bug? I don’t have this edit right.

@mksd during the class path scanning process, the SwaggerSpecificationCreatorTest which was compiled using a lower java source version, fails to process the ProviderController2_0Test class file which was compiled using a higher java source version. So move the SwaggerSpecificationCreatorTest to omod-2.0 such that it gets compiled using the higher java source version.

I did this.

1 Like

@dkayiwa, thanks for spending the time, moving SwaggerSpecificationCreatorTest solves the problem.

I get the idea but I fail to see why the former would have to “process” the latter? How did you troubleshoot that such thing was happening?

As part of the functionality it provides, SwaggerSpecificationCreator, which is being tested by SwaggerSpecificationCreatorTest, scans through classes in the class path.