Skip to content

Commit

Permalink
Fix paging logic in search result handling of resource provider
Browse files Browse the repository at this point in the history 
The results of a FHIR search query using search entry mode 'included'
could lead to infinite number of requests of incrementing result page
number if the number of resources of the join (with search mode
'included') in the results is less than the number of the primary
resources (with search mode 'match').
This fix only requests the next result page if the previous result
bundle contains a 'next' link.

Add test cases for OrganizationProviderImpl.
  • Loading branch information
@EmteZogaf
EmteZogaf committed Sep 18, 2023
1 parent 8060ca0 commit 4fac832
Show file tree
Hide file tree
Showing 2 changed files with 259 additions and 1 deletion.
4 changes: 3 additions & 1 deletion dsf-bpe/dsf-bpe-server/src/main/java/dev/dsf/bpe/v1/service/AbstractResourceProvider.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package dev.dsf.bpe.v1.service;

import static org.hl7.fhir.instance.model.api.IBaseBundle.LINK_NEXT;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -61,7 +63,7 @@ protected final <R extends Resource> List<R> search(Class<? extends Resource> se
.map(BundleEntryComponent::getResource).filter(targetType::isInstance).map(targetType::cast)
.filter(filter).toList());

hasMore = resultBundle.getTotal() > organizations.size();
hasMore = resultBundle.getLink(LINK_NEXT) != null;
}

return organizations;
Expand Down
256 changes: 256 additions & 0 deletions ...bpe/dsf-bpe-server/src/test/java/dev/dsf/bpe/v1/service/OrganizationProviderImplTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
package dev.dsf.bpe.v1.service;

import static org.hamcrest.CoreMatchers.hasItem;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hl7.fhir.instance.model.api.IBaseBundle.LINK_NEXT;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.when;

import java.util.List;
import java.util.Map;

import org.hl7.fhir.r4.model.Bundle;
import org.hl7.fhir.r4.model.Bundle.BundleEntrySearchComponent;
import org.hl7.fhir.r4.model.Bundle.BundleLinkComponent;
import org.hl7.fhir.r4.model.Bundle.SearchEntryMode;
import org.hl7.fhir.r4.model.Coding;
import org.hl7.fhir.r4.model.Identifier;
import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.OrganizationAffiliation;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.junit.MockitoJUnitRunner;

import dev.dsf.fhir.client.FhirWebserviceClient;

@RunWith(MockitoJUnitRunner.class)
public class OrganizationProviderImplTest
{

private static final String ENDPOINT = "endpoint";
private static final BundleEntrySearchComponent INCLUDE_MODE = new BundleEntrySearchComponent()
.setMode(SearchEntryMode.INCLUDE);
private static final BundleEntrySearchComponent MATCH_MODE = new BundleEntrySearchComponent()
.setMode(SearchEntryMode.MATCH);

@Mock
private FhirWebserviceClientProvider clientProvider;
@Mock
private FhirWebserviceClient client;

private OrganizationProviderImpl organizationProvider;

@Captor
ArgumentCaptor<Map<String, List<String>>> parametersCaptor;


@Before
public void setup()
{
organizationProvider = new OrganizationProviderImpl(clientProvider, ENDPOINT);
}

@Test
public void getOrganizationsWithOneOrganizationAndOrganizationAffiliation() throws Exception
{
String system = "foo";
String code = "bar";
Identifier identifier = new Identifier().setSystem(system).setValue(code);
Organization org = new Organization().setActive(true);
OrganizationAffiliation affiliation = new OrganizationAffiliation().setActive(true);
Bundle results = new Bundle();
results.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
results.addEntry().setSearch(INCLUDE_MODE).setResource(org);
results.setTotal(1);
when(clientProvider.getLocalWebserviceClient()).thenReturn(client);
when(client.searchWithStrictHandling(Mockito.eq(OrganizationAffiliation.class), parametersCaptor.capture()))
.thenReturn(results);

List<Organization> organizations = organizationProvider.getOrganizations(identifier);

assertThat(parametersCaptor.getAllValues().size(), is(1));
assertTrue("Parameters do not contain 'primary-organization:identifier'.",
parametersCaptor.getValue().containsKey("primary-organization:identifier"));
assertThat(parametersCaptor.getValue().get("primary-organization:identifier").size(), is(1));
assertThat(parametersCaptor.getValue().get("primary-organization:identifier").get(0), is(system + "|" + code));
assertThat(organizations.size(), is(1));
assertThat(organizations, hasItem(org));
}

@Test
public void getOrganizationsWithOneOrganizationAndOrganizationAffiliationSearchingWithRole() throws Exception
{
String systemIdentifier = "foo";
String systemRole = "bar";
String code = "baz";
Identifier identifier = new Identifier().setSystem(systemIdentifier).setValue(code);
Coding role = new Coding().setSystem(systemRole).setCode(code);
Organization org = new Organization().setActive(true);
OrganizationAffiliation affiliation = new OrganizationAffiliation().setActive(true);
Bundle results = new Bundle();
results.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
results.addEntry().setSearch(INCLUDE_MODE).setResource(org);
results.setTotal(1);
when(clientProvider.getLocalWebserviceClient()).thenReturn(client);
when(client.searchWithStrictHandling(Mockito.eq(OrganizationAffiliation.class), parametersCaptor.capture()))
.thenReturn(results);

List<Organization> organizations = organizationProvider.getOrganizations(identifier, role);

assertThat(parametersCaptor.getAllValues().size(), is(1));
assertTrue("Parameters do not contain 'primary-organization:identifier'.",
parametersCaptor.getValue().containsKey("primary-organization:identifier"));
assertThat(parametersCaptor.getValue().get("primary-organization:identifier").size(), is(1));
assertThat(parametersCaptor.getValue().get("primary-organization:identifier").get(0),
is(systemIdentifier + "|" + code));
assertTrue("Parameters do not contain 'role'.", parametersCaptor.getValue().containsKey("role"));
assertThat(parametersCaptor.getValue().get("role").size(), is(1));
assertThat(parametersCaptor.getValue().get("role").get(0), is(systemRole + "|" + code));
assertThat(organizations.size(), is(1));
assertThat(organizations, hasItem(org));
}

@Test
public void getOrganizationsWithSameNumberOfOrganizationsAndOrganizationAffiliations() throws Exception
{
Identifier identifier = new Identifier().setSystem("foo").setValue("bar");
Organization org = new Organization().setActive(true);
OrganizationAffiliation affiliation = new OrganizationAffiliation().setActive(true);
Bundle results = new Bundle();
int count = 10;
for (int i = 0; i < count; i++)
{
results.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
results.addEntry().setSearch(INCLUDE_MODE).setResource(org);
}
results.setTotal(10);
when(clientProvider.getLocalWebserviceClient()).thenReturn(client);
when(client.searchWithStrictHandling(Mockito.eq(OrganizationAffiliation.class), parametersCaptor.capture()))
.thenReturn(results);

List<Organization> organizations = organizationProvider.getOrganizations(identifier);

assertThat(parametersCaptor.getAllValues().size(), is(1));
assertThat(organizations.size(), is(count));
}

@Test
public void getOrganizationsWithNumberOfOrganizationsLessThanNumberOfOrganizationAffiliations() throws Exception
{
Identifier identifier = new Identifier().setSystem("foo").setValue("bar");
Organization org = new Organization().setActive(true);
OrganizationAffiliation affiliation = new OrganizationAffiliation().setActive(true);
Bundle results = new Bundle();
int countOrganizations = 8;
int countAffiliations = 10;
for (int i = 0; i < countOrganizations; i++)
{
results.addEntry().setSearch(INCLUDE_MODE).setResource(org);
}
for (int i = 0; i < countAffiliations; i++)
{
results.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
}
results.setTotal(countAffiliations);
when(clientProvider.getLocalWebserviceClient()).thenReturn(client);
when(client.searchWithStrictHandling(Mockito.eq(OrganizationAffiliation.class), parametersCaptor.capture()))
.thenReturn(results);

List<Organization> organizations = organizationProvider.getOrganizations(identifier);

assertThat(parametersCaptor.getAllValues().size(), is(1));
assertThat(organizations.size(), is(countOrganizations));
}

@Test
public void getOrganizationsWithSameNumberOfOrganizationsAndOrganizationAffiliationsOnMultiplePages()
throws Exception
{
Identifier identifier = new Identifier().setSystem("foo").setValue("bar");
Organization org = new Organization().setActive(true);
OrganizationAffiliation affiliation = new OrganizationAffiliation().setActive(true);
Bundle firstPageResults = new Bundle();
Bundle secondPageResults = new Bundle();
Bundle morePageResults = new Bundle();
BundleLinkComponent nextLink = new BundleLinkComponent().setRelation(LINK_NEXT).setUrl("foo.bar");
int count = 10;
for (int i = 0; i < count / 2; i++)
{
firstPageResults.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
firstPageResults.addEntry().setSearch(INCLUDE_MODE).setResource(org);
}
for (int i = count / 2; i < count; i++)
{
secondPageResults.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
secondPageResults.addEntry().setSearch(INCLUDE_MODE).setResource(org);
}
firstPageResults.setTotal(count);
secondPageResults.setTotal(count);
morePageResults.setTotal(count);
firstPageResults.setLink(List.of(nextLink));
when(clientProvider.getLocalWebserviceClient()).thenReturn(client);
when(client.searchWithStrictHandling(Mockito.eq(OrganizationAffiliation.class), parametersCaptor.capture()))
.thenReturn(firstPageResults, secondPageResults, morePageResults);

List<Organization> organizations = organizationProvider.getOrganizations(identifier);

assertThat(parametersCaptor.getAllValues().size(), is(2));
assertThat(parametersCaptor.getAllValues().get(1).keySet(), hasItem("_page"));
assertThat(parametersCaptor.getAllValues().get(1).get("_page").size(), is(1));
assertThat(parametersCaptor.getAllValues().get(1).get("_page").get(0), is("2"));
assertThat(organizations.size(), is(count));
}

@Test
public void getOrganizationsWithNumberOfOrganizationsLessThanNumberOfOrganizationAffiliationsOnMultiplePages()
throws Exception
{
Identifier identifier = new Identifier().setSystem("foo").setValue("bar");
Organization org = new Organization().setActive(true);
OrganizationAffiliation affiliation = new OrganizationAffiliation().setActive(true);
int countOrganizations = 8;
int countAffiliations = 10;
Bundle firstPageResults = new Bundle();
Bundle secondPageResults = new Bundle();
Bundle morePageResults = new Bundle();
BundleLinkComponent nextLink = new BundleLinkComponent().setRelation(LINK_NEXT).setUrl("foo.bar");
for (int i = 0; i < countOrganizations / 2; i++)
{
firstPageResults.addEntry().setSearch(INCLUDE_MODE).setResource(org);
}
for (int i = 0; i < countAffiliations / 2; i++)
{
firstPageResults.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
}
for (int i = countOrganizations / 2; i < countOrganizations; i++)
{
secondPageResults.addEntry().setSearch(INCLUDE_MODE).setResource(org);
}
for (int i = countAffiliations / 2; i < countAffiliations; i++)
{
secondPageResults.addEntry().setSearch(MATCH_MODE).setResource(affiliation);
}
firstPageResults.setTotal(countAffiliations);
secondPageResults.setTotal(countAffiliations);
morePageResults.setTotal(countAffiliations);
firstPageResults.setLink(List.of(nextLink));
when(clientProvider.getLocalWebserviceClient()).thenReturn(client);
when(client.searchWithStrictHandling(Mockito.eq(OrganizationAffiliation.class), parametersCaptor.capture()))
.thenReturn(firstPageResults, secondPageResults, morePageResults);

List<Organization> organizations = organizationProvider.getOrganizations(identifier);

assertThat(parametersCaptor.getAllValues().size(), is(2));
assertThat(parametersCaptor.getAllValues().get(1).keySet(), hasItem("_page"));
assertThat(parametersCaptor.getAllValues().get(1).get("_page").size(), is(1));
assertThat(parametersCaptor.getAllValues().get(1).get("_page").get(0), is("2"));
assertThat(organizations.size(), is(countOrganizations));
}

}

0 comments on commit 4fac832

Please sign in to comment.