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?
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 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.
@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:
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.
@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.
As part of the functionality it provides, SwaggerSpecificationCreator, which is being tested by SwaggerSpecificationCreatorTest, scans through classes in the class path.
