diff --git a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/MsSqlEmbeddedDatabase.java b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/MsSqlEmbeddedDatabase.java index ae9fb896378..f864beca5e2 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/MsSqlEmbeddedDatabase.java +++ b/hapi-fhir-jpaserver-test-utilities/src/main/java/ca/uhn/fhir/jpa/embedded/MsSqlEmbeddedDatabase.java @@ -20,6 +20,8 @@ package ca.uhn.fhir.jpa.embedded; import ca.uhn.fhir.jpa.migrate.DriverTypeEnum; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.testcontainers.containers.MSSQLServerContainer; import org.testcontainers.utility.DockerImageName; @@ -36,6 +38,7 @@ import java.util.Map; * @see MS SQL Server TestContainer */ public class MsSqlEmbeddedDatabase extends JpaEmbeddedDatabase { + private static final Logger ourLog = LoggerFactory.getLogger(MsSqlEmbeddedDatabase.class); private final MSSQLServerContainer myContainer; @@ -92,8 +95,18 @@ public class MsSqlEmbeddedDatabase extends JpaEmbeddedDatabase { List sql = new ArrayList<>(); List> queryResults = query("SELECT * FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS"); for (Map row : queryResults) { - String tableName = row.get("TABLE_NAME").toString(); - String constraintName = row.get("CONSTRAINT_NAME").toString(); + Object tableNameEntry = row.get("TABLE_NAME"); + if (tableNameEntry == null) { + ourLog.warn("Found a constraint with no table name: {}", row); + continue; + } + String tableName = tableNameEntry.toString(); + Object constraintNameEntry = row.get("CONSTRAINT_NAME"); + if (constraintNameEntry == null) { + ourLog.warn("Found a constraint with no constraint name: {}", row); + continue; + } + String constraintName = constraintNameEntry.toString(); sql.add(String.format("ALTER TABLE \"%s\" DROP CONSTRAINT \"%s\"", tableName, constraintName)); } executeSqlAsBatch(sql); diff --git a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java index 623dcb54cd6..725895d607d 100644 --- a/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java +++ b/hapi-fhir-jpaserver-test-utilities/src/test/java/ca/uhn/fhir/jpa/embedded/HapiSchemaMigrationTest.java @@ -34,13 +34,22 @@ public class HapiSchemaMigrationTest { private static final Logger ourLog = LoggerFactory.getLogger(HapiSchemaMigrationTest.class); public static final String TEST_SCHEMA_NAME = "test"; + static { + HapiSystemProperties.enableUnitTestMode(); + } + @RegisterExtension static HapiEmbeddedDatabasesExtension myEmbeddedServersExtension = new HapiEmbeddedDatabasesExtension(); @AfterEach public void afterEach() { - myEmbeddedServersExtension.clearDatabases(); - HapiSystemProperties.enableUnitTestMode(); + try { + myEmbeddedServersExtension.clearDatabases(); + // The stack trace for this failure does not appear in CI logs. Catching and rethrowing to log the error. + } catch (Exception e) { + ourLog.error("Failed to clear databases", e); + throw e; + } } @ParameterizedTest diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java index d66eb706f2a..be2af7c03f8 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/api/server/IBundleProvider.java @@ -19,8 +19,8 @@ */ package ca.uhn.fhir.rest.api.server; -import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.context.ConfigurationException; +import ca.uhn.fhir.i18n.Msg; import org.apache.commons.lang3.Validate; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; @@ -195,7 +195,7 @@ public interface IBundleProvider { Integer size(); /** - * This method returns true if the bundle provider knows that at least + * This method returns false if the bundle provider knows that at least * one result exists. */ default boolean isEmpty() { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java index 872ffb24c64..0c5427729c1 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBinding.java @@ -19,15 +19,27 @@ */ package ca.uhn.fhir.rest.server.method; -import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.context.ConfigurationException; import ca.uhn.fhir.context.FhirContext; -import ca.uhn.fhir.interceptor.api.HookParams; -import ca.uhn.fhir.interceptor.api.Pointcut; +import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.model.api.IResource; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.parser.DataFormatException; -import ca.uhn.fhir.rest.annotation.*; +import ca.uhn.fhir.rest.annotation.AddTags; +import ca.uhn.fhir.rest.annotation.Create; +import ca.uhn.fhir.rest.annotation.Delete; +import ca.uhn.fhir.rest.annotation.DeleteTags; +import ca.uhn.fhir.rest.annotation.GetPage; +import ca.uhn.fhir.rest.annotation.GraphQL; +import ca.uhn.fhir.rest.annotation.History; +import ca.uhn.fhir.rest.annotation.Metadata; +import ca.uhn.fhir.rest.annotation.Operation; +import ca.uhn.fhir.rest.annotation.Patch; +import ca.uhn.fhir.rest.annotation.Read; +import ca.uhn.fhir.rest.annotation.Search; +import ca.uhn.fhir.rest.annotation.Transaction; +import ca.uhn.fhir.rest.annotation.Update; +import ca.uhn.fhir.rest.annotation.Validate; import ca.uhn.fhir.rest.api.MethodOutcome; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; @@ -37,7 +49,6 @@ import ca.uhn.fhir.rest.server.BundleProviders; import ca.uhn.fhir.rest.server.IResourceProvider; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; -import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails; import ca.uhn.fhir.util.ReflectionUtil; import org.hl7.fhir.instance.model.api.IAnyResource; import org.hl7.fhir.instance.model.api.IBaseBundle; @@ -243,17 +254,9 @@ public abstract class BaseMethodBinding { populateRequestDetailsForInterceptor(theRequest, theMethodParams); // Interceptor invoke: SERVER_INCOMING_REQUEST_PRE_HANDLED - HookParams preHandledParams = new HookParams(); - preHandledParams.add(RestOperationTypeEnum.class, theRequest.getRestOperationType()); - preHandledParams.add(RequestDetails.class, theRequest); - preHandledParams.addIfMatchesType(ServletRequestDetails.class, theRequest); - if (theRequest.getInterceptorBroadcaster() != null) { - theRequest - .getInterceptorBroadcaster() - .callHooks(Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED, preHandledParams); - } + PageMethodBinding.callPreHandledHooks(theRequest); - } + } // Actually invoke the method try { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java index 40c34c35f93..0c2788a686a 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/BaseResourceReturningMethodBinding.java @@ -30,18 +30,14 @@ import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.rest.api.BundleLinks; import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.EncodingEnum; import ca.uhn.fhir.rest.api.IVersionSpecificBundleFactory; import ca.uhn.fhir.rest.api.MethodOutcome; -import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.SummaryEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.IRestfulServer; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.ResponseDetails; -import ca.uhn.fhir.rest.server.IPagingProvider; import ca.uhn.fhir.rest.server.RestfulServerUtils; -import ca.uhn.fhir.rest.server.RestfulServerUtils.ResponseEncoding; import ca.uhn.fhir.rest.server.exceptions.BaseServerResponseException; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; @@ -59,20 +55,12 @@ import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.Date; -import java.util.List; -import java.util.Objects; import java.util.Set; -import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; -import static org.apache.commons.lang3.StringUtils.isBlank; -import static org.apache.commons.lang3.StringUtils.isNotBlank; - public abstract class BaseResourceReturningMethodBinding extends BaseMethodBinding { - private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(BaseResourceReturningMethodBinding.class); + protected final ResponseBundleBuilder myResponseBundleBuilder; private MethodReturnTypeEnum myMethodReturnType; private String myResourceName; @@ -128,6 +116,7 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi } } + myResponseBundleBuilder = new ResponseBundleBuilder(isOffsetModeHistory()); } /** @@ -137,187 +126,10 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi return null; } - IBaseResource createBundleFromBundleProvider(IRestfulServer theServer, RequestDetails theRequest, Integer theLimit, String theLinkSelf, Set theIncludes, - IBundleProvider theResult, int theOffset, BundleTypeEnum theBundleType, EncodingEnum theLinkEncoding, String theSearchId) { - IVersionSpecificBundleFactory bundleFactory = theServer.getFhirContext().newBundleFactory(); - final Integer offset; - Integer limit = theLimit; - - if (theResult.getCurrentPageOffset() != null) { - offset = theResult.getCurrentPageOffset(); - limit = theResult.getCurrentPageSize(); - Validate.notNull(limit, "IBundleProvider returned a non-null offset, but did not return a non-null page size"); - } else { - offset = RestfulServerUtils.tryToExtractNamedParameter(theRequest, Constants.PARAM_OFFSET); - } - - int numToReturn; - String searchId = null; - List resourceList; - Integer numTotalResults = theResult.size(); - - int pageSize; - if (offset != null || !theServer.canStoreSearchResults()) { - if (limit != null) { - pageSize = limit; - } else { - if (theServer.getDefaultPageSize() != null) { - pageSize = theServer.getDefaultPageSize(); - } else { - pageSize = numTotalResults != null ? numTotalResults : Integer.MAX_VALUE; - } - } - numToReturn = pageSize; - - if ((offset != null && !isOffsetModeHistory()) || theResult.getCurrentPageOffset() != null) { - // When offset query is done theResult already contains correct amount (+ their includes etc.) so return everything - resourceList = theResult.getResources(0, Integer.MAX_VALUE); - } else if (numToReturn > 0) { - resourceList = theResult.getResources(0, numToReturn); - } else { - resourceList = Collections.emptyList(); - } - RestfulServerUtils.validateResourceListNotNull(resourceList); - - } else { - IPagingProvider pagingProvider = theServer.getPagingProvider(); - if (limit == null || ((Integer) limit).equals(0)) { - pageSize = pagingProvider.getDefaultPageSize(); - } else { - pageSize = Math.min(pagingProvider.getMaximumPageSize(), limit); - } - numToReturn = pageSize; - - if (numTotalResults != null) { - numToReturn = Math.min(numToReturn, numTotalResults - theOffset); - } - - if (numToReturn > 0 || theResult.getCurrentPageId() != null) { - resourceList = theResult.getResources(theOffset, numToReturn + theOffset); - } else { - resourceList = Collections.emptyList(); - } - RestfulServerUtils.validateResourceListNotNull(resourceList); - - if (numTotalResults == null) { - numTotalResults = theResult.size(); - } - - if (theSearchId != null) { - searchId = theSearchId; - } else { - if (numTotalResults == null || numTotalResults > numToReturn) { - searchId = pagingProvider.storeResultList(theRequest, theResult); - if (isBlank(searchId)) { - ourLog.info("Found {} results but paging provider did not provide an ID to use for paging", numTotalResults); - searchId = null; - } - } - } - } - - /* - * Remove any null entries in the list - This generally shouldn't happen but can if - * data has been manually purged from the JPA database - */ - boolean hasNull = false; - for (IBaseResource next : resourceList) { - if (next == null) { - hasNull = true; - break; - } - } - if (hasNull) { - resourceList.removeIf(Objects::isNull); - } - - /* - * Make sure all returned resources have an ID (if not, this is a bug - * in the user server code) - */ - for (IBaseResource next : resourceList) { - if (next.getIdElement() == null || next.getIdElement().isEmpty()) { - if (!(next instanceof IBaseOperationOutcome)) { - throw new InternalErrorException(Msg.code(435) + "Server method returned resource of type[" + next.getClass().getSimpleName() + "] with no ID specified (IResource#setId(IdDt) must be called)"); - } - } - } - - BundleLinks links = new BundleLinks(theRequest.getFhirServerBase(), theIncludes, RestfulServerUtils.prettyPrintResponse(theServer, theRequest), theBundleType); - links.setSelf(theLinkSelf); - - if (theResult.getCurrentPageOffset() != null) { - - if (isNotBlank(theResult.getNextPageId())) { - links.setNext(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), offset + limit, limit, theRequest.getParameters())); - } - if (isNotBlank(theResult.getPreviousPageId())) { - links.setNext(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), Math.max(offset - limit, 0), limit, theRequest.getParameters())); - } - - } - - if (offset != null || (!theServer.canStoreSearchResults() && !isEverythingOperation(theRequest)) || isOffsetModeHistory()) { - // Paging without caching - // We're doing offset pages - int requestedToReturn = numToReturn; - if (theServer.getPagingProvider() == null && offset != null) { - // There is no paging provider at all, so assume we're querying up to all the results we need every time - requestedToReturn += offset; - } - if (numTotalResults == null || requestedToReturn < numTotalResults) { - if (!resourceList.isEmpty()) { - links.setNext(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), defaultIfNull(offset, 0) + numToReturn, numToReturn, theRequest.getParameters())); - } - } - if (offset != null && offset > 0) { - int start = Math.max(0, offset - pageSize); - links.setPrev(RestfulServerUtils.createOffsetPagingLink(links, theRequest.getRequestPath(), theRequest.getTenantId(), start, pageSize, theRequest.getParameters())); - } - } else if (isNotBlank(theResult.getCurrentPageId())) { - // We're doing named pages - searchId = theResult.getUuid(); - if (isNotBlank(theResult.getNextPageId())) { - links.setNext(RestfulServerUtils.createPagingLink(links, theRequest, searchId, theResult.getNextPageId(), theRequest.getParameters())); - } - if (isNotBlank(theResult.getPreviousPageId())) { - links.setPrev(RestfulServerUtils.createPagingLink(links, theRequest, searchId, theResult.getPreviousPageId(), theRequest.getParameters())); - } - } else if (searchId != null) { - /* - * We're doing offset pages - Note that we only return paging links if we actually - * included some results in the response. We do this to avoid situations where - * people have faked the offset number to some huge number to avoid them getting - * back paging links that don't make sense. - */ - if (resourceList.size() > 0) { - if (numTotalResults == null || theOffset + numToReturn < numTotalResults) { - links.setNext((RestfulServerUtils.createPagingLink(links, theRequest, searchId, theOffset + numToReturn, numToReturn, theRequest.getParameters()))); - } - if (theOffset > 0) { - int start = Math.max(0, theOffset - pageSize); - links.setPrev(RestfulServerUtils.createPagingLink(links, theRequest, searchId, start, pageSize, theRequest.getParameters())); - } - } - } - - bundleFactory.addRootPropertiesToBundle(theResult.getUuid(), links, theResult.size(), theResult.getPublished()); - bundleFactory.addResourcesToBundle(new ArrayList<>(resourceList), theBundleType, links.serverBase, theServer.getBundleInclusionRule(), theIncludes); - - return bundleFactory.getResourceBundle(); - - } - protected boolean isOffsetModeHistory() { return false; } - private boolean isEverythingOperation(RequestDetails theRequest) { - return (theRequest.getRestOperationType() == RestOperationTypeEnum.EXTENDED_OPERATION_TYPE - || theRequest.getRestOperationType() == RestOperationTypeEnum.EXTENDED_OPERATION_INSTANCE) - && theRequest.getOperation() != null && theRequest.getOperation().equals("$everything"); - } - public IBaseResource doInvokeServer(IRestfulServer theServer, RequestDetails theRequest) { Object[] params = createMethodParams(theRequest); @@ -337,8 +149,10 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi * Figure out the self-link for this request */ - BundleLinks bundleLinks = new BundleLinks(theRequest.getServerBaseForRequest(), null, RestfulServerUtils.prettyPrintResponse(theServer, theRequest), getResponseBundleType()); - bundleLinks.setSelf(RestfulServerUtils.createLinkSelf(theRequest.getFhirServerBase(), theRequest)); + BundleTypeEnum responseBundleType = getResponseBundleType(); + BundleLinks bundleLinks = new BundleLinks(theRequest.getServerBaseForRequest(), null, RestfulServerUtils.prettyPrintResponse(theServer, theRequest), responseBundleType); + String linkSelf = RestfulServerUtils.createLinkSelf(theRequest.getFhirServerBase(), theRequest); + bundleLinks.setSelf(linkSelf); if (getMethodReturnType() == MethodReturnTypeEnum.BUNDLE_RESOURCE) { IBaseResource resource; @@ -361,41 +175,21 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi responseObject = resource; } else { - Set includes = getRequestIncludesFromParams(params); - - IBundleProvider result = (IBundleProvider) resultObj; - if (count == null) { - count = result.preferredPageSize(); - } - - Integer offset = RestfulServerUtils.tryToExtractNamedParameter(theRequest, Constants.PARAM_PAGINGOFFSET); - if (offset == null || offset < 0) { - offset = 0; - } - - Integer resultSize = result.size(); - int start = offset; - if (resultSize != null) { - start = Math.max(0, Math.min(offset, resultSize)); - } - - ResponseEncoding responseEncoding = RestfulServerUtils.determineResponseEncodingNoDefault(theRequest, theServer.getDefaultResponseEncoding()); - EncodingEnum linkEncoding = theRequest.getParameters().containsKey(Constants.PARAM_FORMAT) && responseEncoding != null ? responseEncoding.getEncoding() : null; - - responseObject = createBundleFromBundleProvider(theServer, theRequest, count, RestfulServerUtils.createLinkSelf(theRequest.getFhirServerBase(), theRequest), includes, result, start, getResponseBundleType(), linkEncoding, null); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(theServer, theRequest, params, (IBundleProvider) resultObj, count, responseBundleType, linkSelf); + responseObject = myResponseBundleBuilder.buildResponseBundle(responseBundleRequest); } break; } case RESOURCE: { IBundleProvider result = (IBundleProvider) resultObj; - if (result.size() == 0) { + Integer size = result.size(); + if (size == null || size == 0) { throw new ResourceNotFoundException(Msg.code(436) + "Resource " + theRequest.getId() + " is not known"); - } else if (result.size() > 1) { + } else if (size > 1) { throw new InternalErrorException(Msg.code(437) + "Method returned multiple resources"); } - IBaseResource resource = result.getResources(0, 1).get(0); - responseObject = resource; + responseObject = result.getResources(0, 1).get(0); break; } default: @@ -404,6 +198,18 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi return responseObject; } + private ResponseBundleRequest buildResponseBundleRequest(IRestfulServer theServer, RequestDetails theRequest, Object[] theParams, IBundleProvider theBundleProvider, Integer theCount, BundleTypeEnum theBundleTypeEnum, String theLinkSelf) { + Set includes = getRequestIncludesFromParams(theParams); + + if (theCount == null) { + theCount = theBundleProvider.preferredPageSize(); + } + + int offset = OffsetCalculator.calculateOffset(theRequest, theBundleProvider); + + return new ResponseBundleRequest(theServer, theBundleProvider, theRequest, offset, theCount, theLinkSelf, includes, theBundleTypeEnum, null); + } + public MethodReturnTypeEnum getMethodReturnType() { return myMethodReturnType; } @@ -489,9 +295,7 @@ public abstract class BaseResourceReturningMethodBinding extends BaseMethodBindi responseParams.add(HttpServletRequest.class, servletRequest); responseParams.add(HttpServletResponse.class, servletResponse); if (theRequest.getInterceptorBroadcaster() != null) { - if (!theRequest.getInterceptorBroadcaster().callHooks(Pointcut.SERVER_OUTGOING_RESPONSE, responseParams)) { - return false; - } + return theRequest.getInterceptorBroadcaster().callHooks(Pointcut.SERVER_OUTGOING_RESPONSE, responseParams); } return true; } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/OffsetCalculator.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/OffsetCalculator.java new file mode 100644 index 00000000000..4c73a37ddd8 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/OffsetCalculator.java @@ -0,0 +1,29 @@ +package ca.uhn.fhir.rest.server.method; + +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.RestfulServerUtils; + +public class OffsetCalculator { + /** + * Calculate the offset into the list of resources that should be used to create the returned bundle. + * @param theRequest + * @param theBundleProvider + * @return + */ + + public static int calculateOffset(RequestDetails theRequest, IBundleProvider theBundleProvider) { + Integer offset = RestfulServerUtils.tryToExtractNamedParameter(theRequest, Constants.PARAM_PAGINGOFFSET); + if (offset == null || offset < 0) { + offset = 0; + } + + Integer resultSize = theBundleProvider.size(); + int retval = offset; + if (resultSize != null) { + retval = Math.max(0, Math.min(offset, resultSize)); + } + return retval; + } +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java index 0757010b0f8..22199202c2b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/PageMethodBinding.java @@ -26,7 +26,6 @@ import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.model.api.Include; import ca.uhn.fhir.model.valueset.BundleTypeEnum; import ca.uhn.fhir.rest.api.Constants; -import ca.uhn.fhir.rest.api.EncodingEnum; import ca.uhn.fhir.rest.api.RequestTypeEnum; import ca.uhn.fhir.rest.api.RestOperationTypeEnum; import ca.uhn.fhir.rest.api.server.IBundleProvider; @@ -34,7 +33,6 @@ import ca.uhn.fhir.rest.api.server.IRestfulServer; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.server.IPagingProvider; import ca.uhn.fhir.rest.server.RestfulServerUtils; -import ca.uhn.fhir.rest.server.RestfulServerUtils.ResponseEncoding; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException; @@ -90,18 +88,14 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding { // Interceptor invoke: SERVER_INCOMING_REQUEST_PRE_HANDLED populateRequestDetailsForInterceptor(theRequest, ReflectionUtil.EMPTY_OBJECT_ARRAY); - HookParams preHandledParams = new HookParams(); - preHandledParams.add(RestOperationTypeEnum.class, theRequest.getRestOperationType()); - preHandledParams.add(RequestDetails.class, theRequest); - preHandledParams.addIfMatchesType(ServletRequestDetails.class, theRequest); - if (theRequest.getInterceptorBroadcaster() != null) { - theRequest - .getInterceptorBroadcaster() - .callHooks(Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED, preHandledParams); - } + callPreHandledHooks(theRequest); - Integer offsetI; - int start = 0; + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(theServer, theRequest, thePagingAction, pagingProvider); + return myResponseBundleBuilder.buildResponseBundle(responseBundleRequest); + } + + private ResponseBundleRequest buildResponseBundleRequest(IRestfulServer theServer, RequestDetails theRequest, String thePagingAction, IPagingProvider thePagingProvider) { + int offset = 0; IBundleProvider bundleProvider; String pageId = null; @@ -117,29 +111,21 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding { if (pageId != null) { // This is a page request by Search ID and Page ID - bundleProvider = pagingProvider.retrieveResultList(theRequest, thePagingAction, pageId); + bundleProvider = thePagingProvider.retrieveResultList(theRequest, thePagingAction, pageId); validateHaveBundleProvider(thePagingAction, bundleProvider); } else { // This is a page request by Search ID and Offset - bundleProvider = pagingProvider.retrieveResultList(theRequest, thePagingAction); + bundleProvider = thePagingProvider.retrieveResultList(theRequest, thePagingAction); validateHaveBundleProvider(thePagingAction, bundleProvider); - offsetI = RestfulServerUtils.tryToExtractNamedParameter(theRequest, Constants.PARAM_PAGINGOFFSET); - if (offsetI == null || offsetI < 0) { - offsetI = 0; - } - - Integer totalNum = bundleProvider.size(); - start = offsetI; - if (totalNum != null) { - start = Math.min(start, totalNum); - } + offset = OffsetCalculator.calculateOffset(theRequest, bundleProvider); } - ResponseEncoding responseEncoding = RestfulServerUtils.determineResponseEncodingNoDefault(theRequest, theServer.getDefaultResponseEncoding()); - + /** + * TODO KHS can this be consolidated with PageMethodBinding.getRequestIncludesFromParams ? + */ Set includes = new HashSet<>(); String[] reqIncludes = theRequest.getParameters().get(Constants.PARAM_INCLUDE); if (reqIncludes != null) { @@ -158,19 +144,28 @@ public class PageMethodBinding extends BaseResourceReturningMethodBinding { bundleType = BundleTypeEnum.VALUESET_BINDER.fromCodeString(bundleTypeValues[0]); } - EncodingEnum encodingEnum = null; - if (responseEncoding != null) { - encodingEnum = responseEncoding.getEncoding(); - } - Integer count = RestfulServerUtils.extractCountParameter(theRequest); if (count == null) { - count = pagingProvider.getDefaultPageSize(); - } else if (count > pagingProvider.getMaximumPageSize()) { - count = pagingProvider.getMaximumPageSize(); + count = thePagingProvider.getDefaultPageSize(); + } else if (count > thePagingProvider.getMaximumPageSize()) { + count = thePagingProvider.getMaximumPageSize(); } - return createBundleFromBundleProvider(theServer, theRequest, count, linkSelf, includes, bundleProvider, start, bundleType, encodingEnum, thePagingAction); + ResponseBundleRequest responseBundleRequest = new ResponseBundleRequest(theServer, bundleProvider, theRequest, offset, count, linkSelf, includes, bundleType, thePagingAction); + return responseBundleRequest; + } + + + static void callPreHandledHooks(RequestDetails theRequest) { + HookParams preHandledParams = new HookParams(); + preHandledParams.add(RestOperationTypeEnum.class, theRequest.getRestOperationType()); + preHandledParams.add(RequestDetails.class, theRequest); + preHandledParams.addIfMatchesType(ServletRequestDetails.class, theRequest); + if (theRequest.getInterceptorBroadcaster() != null) { + theRequest + .getInterceptorBroadcaster() + .callHooks(Pointcut.SERVER_INCOMING_REQUEST_PRE_HANDLED, preHandledParams); + } } private void validateHaveBundleProvider(String thePagingAction, IBundleProvider theBundleProvider) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RequestedPage.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RequestedPage.java new file mode 100644 index 00000000000..8aea1613e34 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/RequestedPage.java @@ -0,0 +1,21 @@ +package ca.uhn.fhir.rest.server.method; + + +/** + * This is an intermediate record object that holds the offset and limit (count) the user requested for the page of results. + */ +public class RequestedPage { + /** + * The search results offset requested by the user + */ + public final Integer offset; + /** + * The number of results starting from the offset requested by the user + */ + public final Integer limit; + + public RequestedPage(Integer theOffset, Integer theLimit) { + offset = theOffset; + limit = theLimit; + } +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilder.java new file mode 100644 index 00000000000..3c7b769f33a --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilder.java @@ -0,0 +1,260 @@ +package ca.uhn.fhir.rest.server.method; + +import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.rest.api.BundleLinks; +import ca.uhn.fhir.rest.api.IVersionSpecificBundleFactory; +import ca.uhn.fhir.rest.api.RestOperationTypeEnum; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.IRestfulServer; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.IPagingProvider; +import ca.uhn.fhir.rest.server.RestfulServerUtils; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import org.apache.commons.lang3.ObjectUtils; +import org.apache.commons.lang3.StringUtils; +import org.hl7.fhir.instance.model.api.IBaseBundle; +import org.hl7.fhir.instance.model.api.IBaseOperationOutcome; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Objects; + +/** + * Service to build a FHIR Bundle from a request and a Bundle Provider + */ +public class ResponseBundleBuilder { + private static final Logger ourLog = LoggerFactory.getLogger(ResponseBundleBuilder.class); + + private final boolean myIsOffsetModeHistory; + + public ResponseBundleBuilder(boolean theIsOffsetModeHistory) { + myIsOffsetModeHistory = theIsOffsetModeHistory; + } + + IBaseBundle buildResponseBundle(ResponseBundleRequest theResponseBundleRequest) { + final ResponsePage responsePage = buildResponsePage(theResponseBundleRequest); + + removeNulls(responsePage.resourceList); + validateIds(responsePage.resourceList); + + BundleLinks links = buildLinks(theResponseBundleRequest, responsePage); + + return buildBundle(theResponseBundleRequest, responsePage, links); + } + + private static IBaseBundle buildBundle(ResponseBundleRequest theResponseBundleRequest, ResponsePage pageResponse, BundleLinks links) { + final IRestfulServer server = theResponseBundleRequest.server; + final IVersionSpecificBundleFactory bundleFactory = server.getFhirContext().newBundleFactory(); + final IBundleProvider bundleProvider = theResponseBundleRequest.bundleProvider; + + bundleFactory.addRootPropertiesToBundle(bundleProvider.getUuid(), links, bundleProvider.size(), bundleProvider.getPublished()); + bundleFactory.addResourcesToBundle(new ArrayList<>(pageResponse.resourceList), theResponseBundleRequest.bundleType, links.serverBase, server.getBundleInclusionRule(), theResponseBundleRequest.includes); + + return (IBaseBundle) bundleFactory.getResourceBundle(); + } + + private ResponsePage buildResponsePage(ResponseBundleRequest theResponseBundleRequest) { + final IRestfulServer server = theResponseBundleRequest.server; + final IBundleProvider bundleProvider = theResponseBundleRequest.bundleProvider; + final Integer bundleProviderSize = bundleProvider.size(); + final RequestedPage requestedPage = theResponseBundleRequest.requestedPage; + final List resourceList; + final int pageSize; + + int numToReturn; + String searchId = null; + + if (requestedPage.offset != null || !server.canStoreSearchResults()) { + pageSize = offsetCalculatePageSize(server, requestedPage, bundleProviderSize); + numToReturn = pageSize; + + resourceList = offsetBuildResourceList(bundleProvider, requestedPage, numToReturn); + RestfulServerUtils.validateResourceListNotNull(resourceList); + } else { + pageSize = pagingCalculatePageSize(requestedPage, server.getPagingProvider()); + + if (bundleProviderSize == null) { + numToReturn = pageSize; + } else { + numToReturn = Math.min(pageSize, bundleProviderSize - theResponseBundleRequest.offset); + } + + resourceList = pagingBuildResourceList(theResponseBundleRequest, bundleProvider, numToReturn); + RestfulServerUtils.validateResourceListNotNull(resourceList); + + searchId = pagingBuildSearchId(theResponseBundleRequest, numToReturn, bundleProviderSize); + } + + return new ResponsePage(searchId, resourceList, pageSize, numToReturn, bundleProviderSize); + } + + private static String pagingBuildSearchId(ResponseBundleRequest theResponseBundleRequest, int theNumToReturn, Integer theNumTotalResults) { + final IPagingProvider pagingProvider = theResponseBundleRequest.server.getPagingProvider(); + String retval = null; + + if (theResponseBundleRequest.searchId != null) { + retval = theResponseBundleRequest.searchId; + } else { + if (theNumTotalResults == null || theNumTotalResults > theNumToReturn) { + retval = pagingProvider.storeResultList(theResponseBundleRequest.requestDetails, theResponseBundleRequest.bundleProvider); + if (StringUtils.isBlank(retval)) { + ourLog.info("Found {} results but paging provider did not provide an ID to use for paging", theNumTotalResults); + retval = null; + } + } + } + return retval; + } + + private static List pagingBuildResourceList(ResponseBundleRequest theResponseBundleRequest, IBundleProvider theBundleProvider, int theNumToReturn) { + final List retval; + if (theNumToReturn > 0 || theBundleProvider.getCurrentPageId() != null) { + retval = theBundleProvider.getResources(theResponseBundleRequest.offset, theNumToReturn + theResponseBundleRequest.offset); + } else { + retval = Collections.emptyList(); + } + return retval; + } + + private static int pagingCalculatePageSize(RequestedPage theRequestedPage, IPagingProvider thePagingProvider) { + if (theRequestedPage.limit == null || theRequestedPage.limit.equals(0)) { + return thePagingProvider.getDefaultPageSize(); + } else { + return Math.min(thePagingProvider.getMaximumPageSize(), theRequestedPage.limit); + } + } + + private List offsetBuildResourceList(IBundleProvider theBundleProvider, RequestedPage theRequestedPage, int theNumToReturn) { + final List retval; + if ((theRequestedPage.offset != null && !myIsOffsetModeHistory) || theBundleProvider.getCurrentPageOffset() != null) { + // When offset query is done theResult already contains correct amount (+ their includes etc.) so return everything + retval = theBundleProvider.getResources(0, Integer.MAX_VALUE); + } else if (theNumToReturn > 0) { + retval = theBundleProvider.getResources(0, theNumToReturn); + } else { + retval = Collections.emptyList(); + } + return retval; + } + + private static int offsetCalculatePageSize(IRestfulServer server, RequestedPage theRequestedPage, Integer theNumTotalResults) { + final int retval; + if (theRequestedPage.limit != null) { + retval = theRequestedPage.limit; + } else { + if (server.getDefaultPageSize() != null) { + retval = server.getDefaultPageSize(); + } else { + retval = theNumTotalResults != null ? theNumTotalResults : Integer.MAX_VALUE; + } + } + return retval; + } + + private static void validateIds(List theResourceList) { + /* + * Make sure all returned resources have an ID (if not, this is a bug + * in the user server code) + */ + for (IBaseResource next : theResourceList) { + if (next.getIdElement() == null || next.getIdElement().isEmpty()) { + if (!(next instanceof IBaseOperationOutcome)) { + throw new InternalErrorException(Msg.code(435) + "Server method returned resource of type[" + next.getClass().getSimpleName() + "] with no ID specified (IResource#setId(IdDt) must be called)"); + } + } + } + } + + private static void removeNulls(List resourceList) { + /* + * Remove any null entries in the list - This generally shouldn't happen but can if + * data has been manually purged from the JPA database + */ + boolean hasNull = false; + for (IBaseResource next : resourceList) { + if (next == null) { + hasNull = true; + break; + } + } + if (hasNull) { + resourceList.removeIf(Objects::isNull); + } + } + + private BundleLinks buildLinks(ResponseBundleRequest theResponseBundleRequest, ResponsePage theResponsePage) { + final IRestfulServer server = theResponseBundleRequest.server; + final IBundleProvider bundleProvider = theResponseBundleRequest.bundleProvider; + final RequestedPage pageRequest = theResponseBundleRequest.requestedPage; + + BundleLinks retval = new BundleLinks(theResponseBundleRequest.requestDetails.getFhirServerBase(), theResponseBundleRequest.includes, RestfulServerUtils.prettyPrintResponse(server, theResponseBundleRequest.requestDetails), theResponseBundleRequest.bundleType); + retval.setSelf(theResponseBundleRequest.linkSelf); + + if (bundleProvider.getCurrentPageOffset() != null) { + + if (StringUtils.isNotBlank(bundleProvider.getNextPageId())) { + retval.setNext(RestfulServerUtils.createOffsetPagingLink(retval, theResponseBundleRequest.requestDetails.getRequestPath(), theResponseBundleRequest.requestDetails.getTenantId(), pageRequest.offset + pageRequest.limit, pageRequest.limit, theResponseBundleRequest.getRequestParameters())); + } + if (StringUtils.isNotBlank(bundleProvider.getPreviousPageId())) { + retval.setNext(RestfulServerUtils.createOffsetPagingLink(retval, theResponseBundleRequest.requestDetails.getRequestPath(), theResponseBundleRequest.requestDetails.getTenantId(), Math.max(pageRequest.offset - pageRequest.limit, 0), pageRequest.limit, theResponseBundleRequest.getRequestParameters())); + } + + } + + if (pageRequest.offset != null || (!server.canStoreSearchResults() && !isEverythingOperation(theResponseBundleRequest.requestDetails)) || myIsOffsetModeHistory) { + // Paging without caching + // We're doing offset pages + int requestedToReturn = theResponsePage.numToReturn; + if (server.getPagingProvider() == null && pageRequest.offset != null) { + // There is no paging provider at all, so assume we're querying up to all the results we need every time + requestedToReturn += pageRequest.offset; + } + if (theResponsePage.numTotalResults == null || requestedToReturn < theResponsePage.numTotalResults) { + if (!theResponsePage.resourceList.isEmpty()) { + retval.setNext(RestfulServerUtils.createOffsetPagingLink(retval, theResponseBundleRequest.requestDetails.getRequestPath(), theResponseBundleRequest.requestDetails.getTenantId(), ObjectUtils.defaultIfNull(pageRequest.offset, 0) + theResponsePage.numToReturn, theResponsePage.numToReturn, theResponseBundleRequest.getRequestParameters())); + } + } + if (pageRequest.offset != null && pageRequest.offset > 0) { + int start = Math.max(0, pageRequest.offset - theResponsePage.pageSize); + retval.setPrev(RestfulServerUtils.createOffsetPagingLink(retval, theResponseBundleRequest.requestDetails.getRequestPath(), theResponseBundleRequest.requestDetails.getTenantId(), start, theResponsePage.pageSize, theResponseBundleRequest.getRequestParameters())); + } + } else if (StringUtils.isNotBlank(bundleProvider.getCurrentPageId())) { + // We're doing named pages + final String uuid = bundleProvider.getUuid(); + if (StringUtils.isNotBlank(bundleProvider.getNextPageId())) { + retval.setNext(RestfulServerUtils.createPagingLink(retval, theResponseBundleRequest.requestDetails, uuid, bundleProvider.getNextPageId(), theResponseBundleRequest.getRequestParameters())); + } + if (StringUtils.isNotBlank(bundleProvider.getPreviousPageId())) { + retval.setPrev(RestfulServerUtils.createPagingLink(retval, theResponseBundleRequest.requestDetails, uuid, bundleProvider.getPreviousPageId(), theResponseBundleRequest.getRequestParameters())); + } + } else if (theResponsePage.searchId != null) { + /* + * We're doing offset pages - Note that we only return paging links if we actually + * included some results in the response. We do this to avoid situations where + * people have faked the offset number to some huge number to avoid them getting + * back paging links that don't make sense. + */ + if (theResponsePage.size() > 0) { + if (theResponsePage.numTotalResults == null || theResponseBundleRequest.offset + theResponsePage.numToReturn < theResponsePage.numTotalResults) { + retval.setNext((RestfulServerUtils.createPagingLink(retval, theResponseBundleRequest.requestDetails, theResponsePage.searchId, theResponseBundleRequest.offset + theResponsePage.numToReturn, theResponsePage.numToReturn, theResponseBundleRequest.getRequestParameters()))); + } + if (theResponseBundleRequest.offset > 0) { + int start = Math.max(0, theResponseBundleRequest.offset - theResponsePage.pageSize); + retval.setPrev(RestfulServerUtils.createPagingLink(retval, theResponseBundleRequest.requestDetails, theResponsePage.searchId, start, theResponsePage.pageSize, theResponseBundleRequest.getRequestParameters())); + } + } + } + return retval; + } + + + private boolean isEverythingOperation(RequestDetails theRequest) { + return (theRequest.getRestOperationType() == RestOperationTypeEnum.EXTENDED_OPERATION_TYPE + || theRequest.getRestOperationType() == RestOperationTypeEnum.EXTENDED_OPERATION_INSTANCE) + && theRequest.getOperation() != null && theRequest.getOperation().equals("$everything"); + } +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleRequest.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleRequest.java new file mode 100644 index 00000000000..9b7c1bcfb12 --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponseBundleRequest.java @@ -0,0 +1,81 @@ +package ca.uhn.fhir.rest.server.method; + +import ca.uhn.fhir.model.api.Include; +import ca.uhn.fhir.model.valueset.BundleTypeEnum; +import ca.uhn.fhir.rest.api.Constants; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.IRestfulServer; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.server.RestfulServerUtils; +import org.apache.commons.lang3.Validate; + +import java.util.Map; +import java.util.Set; + +/** + * This is a request object for selecting resources from a bundle provider and returning a bundle to the client + */ +public class ResponseBundleRequest { + /** + * The FHIR REST server the request is coming from. This is used to determine default page size. + */ + public final IRestfulServer server; + /** + * The bundle provider that will be used as the source of resources for the returned bundle. + */ + public final IBundleProvider bundleProvider; + /** + * The user request details. This is used to parse out parameters used to create the final bundle. + */ + public final RequestDetails requestDetails; + /** + * The requested offset into the list of resources that should be used to create the returned bundle. + */ + public final int offset; + /** + * The response bundle link to self. This is used to create "self" link in the returned bundle. + */ + public final String linkSelf; + /** + * The set of includes requested by the user. This is used to determine which resources should be additionally + * included in the returned bundle. + */ + public final Set includes; + /** + * The type of bundle that should be returned to the client. + */ + public final BundleTypeEnum bundleType; + /** + * The id of the search used to page through search results + */ + public final String searchId; + public final RequestedPage requestedPage; + + public ResponseBundleRequest(IRestfulServer theServer, IBundleProvider theBundleProvider, RequestDetails theRequest, int theOffset, Integer theLimit, String theLinkSelf, Set theIncludes, BundleTypeEnum theBundleType, String theSearchId) { + server = theServer; + bundleProvider = theBundleProvider; + requestDetails = theRequest; + offset = theOffset; + linkSelf = theLinkSelf; + includes = theIncludes; + bundleType = theBundleType; + searchId = theSearchId; + requestedPage = getRequestedPage(theLimit); + } + + public Map getRequestParameters() { + return requestDetails.getParameters(); + } + + private RequestedPage getRequestedPage(Integer theLimit) { + // If the BundleProvider has an offset and page size, we use that + if (bundleProvider.getCurrentPageOffset() != null) { + Validate.notNull(bundleProvider.getCurrentPageSize(), "IBundleProvider returned a non-null offset, but did not return a non-null page size"); + return new RequestedPage(bundleProvider.getCurrentPageOffset(), bundleProvider.getCurrentPageSize()); + // Otherwise, we build it from the request + } else { + Integer parameterOffset = RestfulServerUtils.tryToExtractNamedParameter(requestDetails, Constants.PARAM_OFFSET); + return new RequestedPage(parameterOffset, theLimit); + } + } +} diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponsePage.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponsePage.java new file mode 100644 index 00000000000..d36823c9cfd --- /dev/null +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/ResponsePage.java @@ -0,0 +1,44 @@ +package ca.uhn.fhir.rest.server.method; + +import org.hl7.fhir.instance.model.api.IBaseResource; + +import java.util.List; + +/** + * This is an intermediate record object that holds all the fields required to make the final bundle that will be returned to the client. + */ +public class ResponsePage { + /** + * The id of the search used to page through search results + */ + public final String searchId; + /** + * The list of resources that will be used to create the bundle + */ + public final List resourceList; + /** + * The total number of results that matched the search + */ + public final Integer numTotalResults; + /** + * The number of resources that should be returned in each page + */ + public final int pageSize; + /** + * The number of resources that should be returned in the bundle. Can be smaller than pageSize when the bundleProvider + * has fewer results than the page size. + */ + public final int numToReturn; + + public ResponsePage(String theSearchId, List theResourceList, int thePageSize, int theNumToReturn, Integer theNumTotalResults) { + searchId = theSearchId; + resourceList = theResourceList; + pageSize = thePageSize; + numToReturn = theNumToReturn; + numTotalResults = theNumTotalResults; + } + + public int size() { + return resourceList.size(); + } +} diff --git a/hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java new file mode 100644 index 00000000000..9f991ff42a4 --- /dev/null +++ b/hapi-fhir-storage/src/test/java/ca/uhn/fhir/rest/server/method/ResponseBundleBuilderTest.java @@ -0,0 +1,469 @@ +package ca.uhn.fhir.rest.server.method; + +import ca.uhn.fhir.context.FhirContext; +import ca.uhn.fhir.model.api.Include; +import ca.uhn.fhir.model.valueset.BundleTypeEnum; +import ca.uhn.fhir.rest.api.server.IBundleProvider; +import ca.uhn.fhir.rest.api.server.IRestfulServer; +import ca.uhn.fhir.rest.api.server.RequestDetails; +import ca.uhn.fhir.rest.api.server.SystemRequestDetails; +import ca.uhn.fhir.rest.server.BundleProviderWithNamedPages; +import ca.uhn.fhir.rest.server.IPagingProvider; +import ca.uhn.fhir.rest.server.SimpleBundleProvider; +import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.r4.model.Bundle; +import org.hl7.fhir.r4.model.Patient; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nonnull; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import static ca.uhn.fhir.rest.api.Constants.LINK_NEXT; +import static ca.uhn.fhir.rest.api.Constants.LINK_PREVIOUS; +import static ca.uhn.fhir.rest.api.Constants.LINK_SELF; +import static java.lang.Math.max; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasSize; +import static org.hl7.fhir.r4.model.Bundle.BundleType.SEARCHSET; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class ResponseBundleBuilderTest { + private static final Logger ourLog = LoggerFactory.getLogger(ResponseBundleBuilderTest.class); + public static final String TEST_LINK_SELF = "http://test.link"; + private static final String TEST_SERVER_BASE = "http://test.server/base"; + public static final int RESOURCE_COUNT = 50; + public static final int LIMIT = 20; + public static final int DEFAULT_PAGE_SIZE = 15; + public static final int CURRENT_PAGE_OFFSET = 2; + private static final int CURRENT_PAGE_SIZE = 8; + private static final Integer MAX_PAGE_SIZE = 43; + private static final String SEARCH_ID = "test-search-id"; + private static final FhirContext ourFhirContext = FhirContext.forR4Cached(); + private static final String TEST_REQUEST_PATH = "test/request/path"; + private static final Integer REQUEST_OFFSET = 3; + @Mock + IRestfulServer myServer; + @Mock + IPagingProvider myPagingProvider; + private Integer myLimit = null; + + @BeforeEach + public void before() { + lenient().when(myServer.getFhirContext()).thenReturn(ourFhirContext); + } + + @AfterEach + public void after() { + reset(); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testEmpty(boolean theCanStoreSearchResults) { + // setup + setCanStoreSearchResults(theCanStoreSearchResults); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(new SimpleBundleProvider()); + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, 0, 0); + assertThat(bundle.getLink(), hasSize(1)); + assertSelfLink(bundle); + } + + @Test + void testOffsetNoPageSize() { + // setup + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(); + bundleProvider.setCurrentPageOffset(CURRENT_PAGE_OFFSET); + + // run + try { + buildResponseBundleRequest(bundleProvider); + + // verify + } catch (NullPointerException e) { + assertEquals("IBundleProvider returned a non-null offset, but did not return a non-null page size", e.getMessage()); + } + } + + @Test + void testNullId() { + // setup + setCanStoreSearchResults(true); + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(new Patient()); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider); + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + try { + svc.buildResponseBundle(responseBundleRequest); + + // verify + } catch (InternalErrorException e) { + assertEquals("HAPI-0435: Server method returned resource of type[Patient] with no ID specified (IResource#setId(IdDt) must be called)", e.getMessage()); + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testNoLimit(boolean theCanStoreSearchResults) { + // setup + setCanStoreSearchResults(theCanStoreSearchResults); + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(buildPatientList()); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider); + if (!theCanStoreSearchResults) { + when(myServer.getDefaultPageSize()).thenReturn(DEFAULT_PAGE_SIZE); + } + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, DEFAULT_PAGE_SIZE); + + assertThat(bundle.getLink(), hasSize(2)); + assertSelfLink(bundle); + assertNextLink(bundle, DEFAULT_PAGE_SIZE); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testFilterNulls(boolean theCanStoreSearchResults) { + // setup + setCanStoreSearchResults(theCanStoreSearchResults); + List list = buildPatientList(); + list.set(7, null); + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(list); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider); + if (!theCanStoreSearchResults) { + when(myServer.getDefaultPageSize()).thenReturn(DEFAULT_PAGE_SIZE); + } + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, DEFAULT_PAGE_SIZE - 1, "A0", "A14"); + + assertThat(bundle.getLink(), hasSize(2)); + assertSelfLink(bundle); + assertNextLink(bundle, DEFAULT_PAGE_SIZE); + } + + // TODO KHS add test that relies on Constants.PARAM_OFFSET supplied from request details + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testWithLimit(boolean theCanStoreSearchResults) { + // setup + myLimit = LIMIT; + setCanStoreSearchResults(theCanStoreSearchResults); + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(buildPatientList()); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider); + + responseBundleRequest.requestDetails.setFhirServerBase(TEST_SERVER_BASE); + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, LIMIT); + assertThat(bundle.getLink(), hasSize(2)); + assertSelfLink(bundle); + assertNextLink(bundle, LIMIT); + } + + @Test + void testNoLimitNoDefaultPageSize() { + // setup + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(buildPatientList()); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider); + + when(myServer.getDefaultPageSize()).thenReturn(null); + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, RESOURCE_COUNT); + assertThat(bundle.getLink(), hasSize(1)); + assertSelfLink(bundle); + } + + @Test + void testOffset() { + // setup + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(buildPatientList()); + bundleProvider.setCurrentPageOffset(CURRENT_PAGE_OFFSET); + bundleProvider.setCurrentPageSize(CURRENT_PAGE_SIZE); + + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider); + responseBundleRequest.requestDetails.setFhirServerBase(TEST_SERVER_BASE); + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, RESOURCE_COUNT); + assertThat(bundle.getLink(), hasSize(3)); + assertSelfLink(bundle); + assertNextLink(bundle, CURRENT_PAGE_SIZE, CURRENT_PAGE_OFFSET + CURRENT_PAGE_SIZE); + //noinspection ConstantValue + assertPrevLink(bundle, max(0, CURRENT_PAGE_OFFSET - CURRENT_PAGE_SIZE)); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void unknownBundleSize(boolean theCanStoreSearchResults) { + // setup + myLimit = LIMIT; + setCanStoreSearchResults(theCanStoreSearchResults); + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(buildPatientList()); + bundleProvider.setSize(null); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider, SEARCH_ID); + + responseBundleRequest.requestDetails.setFhirServerBase(TEST_SERVER_BASE); + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, null, LIMIT); + assertThat(bundle.getLink(), hasSize(2)); + assertSelfLink(bundle); + assertNextLink(bundle, LIMIT); + } + + @Test + void testCustomLinks() { + // setup + setCanStoreSearchResults(true); + String pageId = "testPageId"; + String nextPageId = "testNextPageId"; + String prevPageId = "testPrevPageId"; + BundleProviderWithNamedPages bundleProvider = new BundleProviderWithNamedPages(buildPatientList(), SEARCH_ID, pageId, RESOURCE_COUNT); + bundleProvider.setNextPageId(nextPageId); + bundleProvider.setPreviousPageId(prevPageId); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider, SEARCH_ID); + ResponseBundleBuilder svc = new ResponseBundleBuilder(false); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, RESOURCE_COUNT); + assertThat(bundle.getLink(), hasSize(3)); + assertSelfLink(bundle); + + Bundle.BundleLinkComponent nextLink = bundle.getLink().get(1); + assertEquals(LINK_NEXT, nextLink.getRelation()); + assertEquals(TEST_SERVER_BASE + "?_getpages=" + SEARCH_ID + "&_pageId=" + nextPageId + "&_bundletype=" + SEARCHSET.toCode(), nextLink.getUrl()); + + Bundle.BundleLinkComponent prevLink = bundle.getLink().get(2); + assertEquals(LINK_PREVIOUS, prevLink.getRelation()); + assertEquals(TEST_SERVER_BASE + "?_getpages=" + SEARCH_ID + "&_pageId=" + prevPageId + "&_bundletype=" + SEARCHSET.toCode(), prevLink.getUrl()); + } + + @Test + void testCustomLinksWithPageOffset() { + // setup + String pageId = "testPageId"; + String nextPageId = "testNextPageId"; + String prevPageId = "testPrevPageId"; + BundleProviderWithNamedPages bundleProvider = new BundleProviderWithNamedPages(buildPatientList(), SEARCH_ID, pageId, RESOURCE_COUNT); + bundleProvider.setNextPageId(nextPageId); + bundleProvider.setPreviousPageId(prevPageId); + // Even though next and prev links are provided, a page offset will override them and force page offset mode + bundleProvider.setCurrentPageOffset(CURRENT_PAGE_OFFSET); + bundleProvider.setCurrentPageSize(CURRENT_PAGE_SIZE); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider); + ResponseBundleBuilder svc = new ResponseBundleBuilder(true); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, RESOURCE_COUNT); + assertThat(bundle.getLink(), hasSize(3)); + assertSelfLink(bundle); + assertNextLink(bundle, CURRENT_PAGE_SIZE, CURRENT_PAGE_OFFSET + CURRENT_PAGE_SIZE); + assertPrevLink(bundle, 0); + } + + + @Test + void offsetSinceNonNullSearchId() { + // setup + myLimit = LIMIT; + setCanStoreSearchResults(true); + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(buildPatientList()); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider, SEARCH_ID); + + responseBundleRequest.requestDetails.setFhirServerBase(TEST_SERVER_BASE); + ResponseBundleBuilder svc = new ResponseBundleBuilder(false); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, LIMIT); + assertThat(bundle.getLink(), hasSize(2)); + assertSelfLink(bundle); + + assertNextLinkOffset(bundle, LIMIT, LIMIT); + } + + @Test + void offsetSinceNonNullSearchIdWithRequestOffset() { + // setup + setCanStoreSearchResults(true); + SimpleBundleProvider bundleProvider = new SimpleBundleProvider(buildPatientList()); + ResponseBundleRequest responseBundleRequest = buildResponseBundleRequest(bundleProvider, SEARCH_ID, REQUEST_OFFSET); + + responseBundleRequest.requestDetails.setFhirServerBase(TEST_SERVER_BASE); + ResponseBundleBuilder svc = new ResponseBundleBuilder(false); + + // run + Bundle bundle = (Bundle) svc.buildResponseBundle(responseBundleRequest); + + // verify + verifyBundle(bundle, RESOURCE_COUNT, DEFAULT_PAGE_SIZE, "A3", "A17"); + assertThat(bundle.getLink(), hasSize(3)); + assertSelfLink(bundle); + + assertNextLinkOffset(bundle, DEFAULT_PAGE_SIZE + REQUEST_OFFSET, DEFAULT_PAGE_SIZE); + assertPrevLinkOffset(bundle); + } + + + private static void assertNextLinkOffset(Bundle theBundle, Integer theOffset, Integer theCount) { + Bundle.BundleLinkComponent nextLink = theBundle.getLink().get(1); + assertEquals(LINK_NEXT, nextLink.getRelation()); + assertEquals(TEST_SERVER_BASE + "?_getpages=" + SEARCH_ID + "&_getpagesoffset=" + theOffset + "&_count=" + theCount + "&_bundletype=" + SEARCHSET.toCode(), nextLink.getUrl()); + } + + private static void assertPrevLinkOffset(Bundle theBundle) { + Bundle.BundleLinkComponent nextLink = theBundle.getLink().get(2); + assertEquals(LINK_PREVIOUS, nextLink.getRelation()); + assertEquals(TEST_SERVER_BASE + "?_getpages=" + SEARCH_ID + "&_getpagesoffset=" + 0 + "&_count=" + ResponseBundleBuilderTest.DEFAULT_PAGE_SIZE + "&_bundletype=" + SEARCHSET.toCode(), nextLink.getUrl()); + } + private static void assertNextLink(Bundle theBundle, int theCount) { + assertNextLink(theBundle, theCount, theCount); + } + + private static void assertNextLink(Bundle theBundle, int theCount, int theOffset) { + Bundle.BundleLinkComponent link = theBundle.getLink().get(1); + assertEquals(LINK_NEXT, link.getRelation()); + assertEquals(TEST_SERVER_BASE + "/" + TEST_REQUEST_PATH + "?_count=" + theCount + "&_offset=" + theOffset, link.getUrl()); + } + + private static void assertPrevLink(Bundle theBundle, int theOffset) { + Bundle.BundleLinkComponent link = theBundle.getLink().get(2); + assertEquals(LINK_PREVIOUS, link.getRelation()); + assertEquals(TEST_SERVER_BASE + "/" + TEST_REQUEST_PATH + "?_count=" + ResponseBundleBuilderTest.CURRENT_PAGE_SIZE + "&_offset=" + theOffset, link.getUrl()); + } + + private static void assertSelfLink(Bundle bundle) { + Bundle.BundleLinkComponent link = bundle.getLinkFirstRep(); + assertEquals(LINK_SELF, link.getRelation()); + assertEquals(TEST_LINK_SELF, link.getUrl()); + } + + private List buildPatientList() { + List retval = new ArrayList<>(); + for (int i = 0; i < ResponseBundleBuilderTest.RESOURCE_COUNT; ++i) { + Patient p = new Patient(); + p.setId("A" + i); + p.setActive(true); + retval.add(p); + } + return retval; + } + + private void setCanStoreSearchResults(boolean theCanStoreSearchResults) { + when(myServer.canStoreSearchResults()).thenReturn(theCanStoreSearchResults); + when(myServer.getPagingProvider()).thenReturn(myPagingProvider); + if (theCanStoreSearchResults) { + if (myLimit == null) { + when(myPagingProvider.getDefaultPageSize()).thenReturn(DEFAULT_PAGE_SIZE); + } else { + when(myPagingProvider.getMaximumPageSize()).thenReturn(MAX_PAGE_SIZE); + } + } + } + + @Nonnull + private ResponseBundleRequest buildResponseBundleRequest(IBundleProvider theBundleProvider) { + return buildResponseBundleRequest(theBundleProvider, null); + } + + @Nonnull + private ResponseBundleRequest buildResponseBundleRequest(IBundleProvider theBundleProvider, String theSearchId) { + return buildResponseBundleRequest(theBundleProvider, theSearchId, 0); + } + + @Nonnull + private ResponseBundleRequest buildResponseBundleRequest(IBundleProvider theBundleProvider, String theSearchId, Integer theOffset) { + Set includes = Collections.emptySet(); + BundleTypeEnum bundleType = BundleTypeEnum.SEARCHSET; + + SystemRequestDetails systemRequestDetails = new SystemRequestDetails(); + systemRequestDetails.setFhirServerBase(TEST_SERVER_BASE); + systemRequestDetails.setRequestPath(TEST_REQUEST_PATH); + + return new ResponseBundleRequest(myServer, theBundleProvider, systemRequestDetails, theOffset, myLimit, TEST_LINK_SELF, includes, bundleType, theSearchId); + } + + private static void verifyBundle(Bundle theBundle, Integer theExpectedTotal, int theExpectedEntryCount) { + String firstId = null; + String lastId = null; + if (theExpectedEntryCount > 0) { + firstId = "A0"; + lastId = "A" + (theExpectedEntryCount - 1); + } + verifyBundle(theBundle, theExpectedTotal, theExpectedEntryCount, firstId, lastId); + } + + private static void verifyBundle(Bundle theBundle, Integer theExpectedTotal, int theExpectedEntryCount, String theFirstId, String theLastId) { + ourLog.trace(ourFhirContext.newJsonParser().setPrettyPrint(true).encodeResourceToString(theBundle)); + assertFalse(theBundle.isEmpty()); + assertEquals(SEARCHSET, theBundle.getType()); + assertEquals(theExpectedTotal, theBundle.getTotalElement().getValue()); + List entries = theBundle.getEntry(); + assertEquals(theExpectedEntryCount, entries.size()); + if (theFirstId != null) { + assertEquals(theFirstId, entries.get(0).getResource().getId()); + } + if (theLastId != null) { + assertEquals(theLastId, entries.get(theExpectedEntryCount - 1).getResource().getId()); + } + } +} diff --git a/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java b/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java index 4924dee5b58..415d23553f0 100644 --- a/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java +++ b/hapi-fhir-structures-r4/src/main/java/org/hl7/fhir/r4/hapi/rest/server/R4BundleFactory.java @@ -44,6 +44,7 @@ import org.hl7.fhir.r4.model.IdType; import org.hl7.fhir.r4.model.Resource; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.util.ArrayList; import java.util.Date; import java.util.HashSet; @@ -57,7 +58,7 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; public class R4BundleFactory implements IVersionSpecificBundleFactory { private String myBase; private Bundle myBundle; - private FhirContext myContext; + private final FhirContext myContext; public R4BundleFactory(FhirContext theContext) { myContext = theContext; @@ -67,18 +68,18 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { public void addResourcesToBundle(List theResult, BundleTypeEnum theBundleType, String theServerBase, BundleInclusionRule theBundleInclusionRule, Set theIncludes) { ensureBundle(); - List includedResources = new ArrayList(); - Set addedResourceIds = new HashSet(); + List includedResources = new ArrayList<>(); + Set addedResourceIds = new HashSet<>(); for (IBaseResource next : theResult) { - if (next.getIdElement().isEmpty() == false) { + if (!next.getIdElement().isEmpty()) { addedResourceIds.add(next.getIdElement()); } } for (IBaseResource next : theResult) { - Set containedIds = new HashSet(); + Set containedIds = new HashSet<>(); if (next instanceof DomainResource) { for (Resource nextContained : ((DomainResource) next).getContained()) { @@ -90,7 +91,7 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { List references = myContext.newTerser().getAllResourceReferences(next); do { - List addedResourcesThisPass = new ArrayList(); + List addedResourcesThisPass = new ArrayList<>(); for (ResourceReferenceInfo nextRefInfo : references) { if (theBundleInclusionRule != null && !theBundleInclusionRule.shouldIncludeReferencedResource(nextRefInfo, theIncludes)) { @@ -106,7 +107,7 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { } IIdType id = nextRes.getIdElement(); - if (id.hasResourceType() == false) { + if (!id.hasResourceType()) { String resName = myContext.getResourceType(nextRes); id = id.withResourceType(resName); } @@ -128,7 +129,7 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { List newReferences = myContext.newTerser().getAllResourceReferences(iResource); references.addAll(newReferences); } - } while (references.isEmpty() == false); + } while (!references.isEmpty()); BundleEntryComponent entry = myBundle.addEntry().setResource((Resource) next); Resource nextAsResource = (Resource) next; @@ -152,13 +153,16 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { case BATCH_RESPONSE: case TRANSACTION_RESPONSE: case HISTORY: - if ("1".equals(id.getVersionIdPart())) { - entry.getResponse().setStatus("201 Created"); - } else if (isNotBlank(id.getVersionIdPart())) { - entry.getResponse().setStatus("200 OK"); - } - if (isNotBlank(id.getVersionIdPart())) { - entry.getResponse().setEtag(RestfulServerUtils.createEtag(id.getVersionIdPart())); + if (id != null) { + String version = id.getVersionIdPart(); + if ("1".equals(version)) { + entry.getResponse().setStatus("201 Created"); + } else if (isNotBlank(version)) { + entry.getResponse().setStatus("200 OK"); + } + if (isNotBlank(version)) { + entry.getResponse().setEtag(RestfulServerUtils.createEtag(version)); + } } break; } @@ -197,13 +201,13 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { myBundle.getMeta().getLastUpdatedElement().setValueAsString(theLastUpdated.getValueAsString()); } - if (!hasLink(Constants.LINK_SELF, myBundle) && isNotBlank(theBundleLinks.getSelf())) { + if (hasNoLinkOfType(Constants.LINK_SELF, myBundle) && isNotBlank(theBundleLinks.getSelf())) { myBundle.addLink().setRelation(Constants.LINK_SELF).setUrl(theBundleLinks.getSelf()); } - if (!hasLink(Constants.LINK_NEXT, myBundle) && isNotBlank(theBundleLinks.getNext())) { + if (hasNoLinkOfType(Constants.LINK_NEXT, myBundle) && isNotBlank(theBundleLinks.getNext())) { myBundle.addLink().setRelation(Constants.LINK_NEXT).setUrl(theBundleLinks.getNext()); } - if (!hasLink(Constants.LINK_PREVIOUS, myBundle) && isNotBlank(theBundleLinks.getPrev())) { + if (hasNoLinkOfType(Constants.LINK_PREVIOUS, myBundle) && isNotBlank(theBundleLinks.getPrev())) { myBundle.addLink().setRelation(Constants.LINK_PREVIOUS).setUrl(theBundleLinks.getPrev()); } @@ -238,13 +242,13 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { return myBundle; } - private boolean hasLink(String theLinkType, Bundle theBundle) { + private boolean hasNoLinkOfType(String theLinkType, Bundle theBundle) { for (BundleLinkComponent next : theBundle.getLink()) { if (theLinkType.equals(next.getRelation())) { - return true; + return false; } } - return false; + return true; } @Override @@ -252,16 +256,18 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { myBundle = (Bundle) theBundle; } - private IIdType populateBundleEntryFullUrl(IBaseResource next, BundleEntryComponent entry) { - IIdType idElement = null; - if (next.getIdElement().hasBaseUrl()) { - idElement = next.getIdElement(); - entry.setFullUrl(idElement.toVersionless().getValue()); + @Nullable + private IIdType populateBundleEntryFullUrl(IBaseResource theResource, BundleEntryComponent theEntry) { + final IIdType idElement; + if (theResource.getIdElement().hasBaseUrl()) { + idElement = theResource.getIdElement(); + theEntry.setFullUrl(idElement.toVersionless().getValue()); } else { - if (isNotBlank(myBase) && next.getIdElement().hasIdPart()) { - idElement = next.getIdElement(); - idElement = idElement.withServerBase(myBase, myContext.getResourceType(next)); - entry.setFullUrl(idElement.toVersionless().getValue()); + if (isNotBlank(myBase) && theResource.getIdElement().hasIdPart()) { + idElement = theResource.getIdElement().withServerBase(myBase, myContext.getResourceType(theResource)); + theEntry.setFullUrl(idElement.toVersionless().getValue()); + } else { + idElement = null; } } return idElement; @@ -269,11 +275,11 @@ public class R4BundleFactory implements IVersionSpecificBundleFactory { @Override public List toListOfResources() { - ArrayList retVal = new ArrayList(); + ArrayList retVal = new ArrayList<>(); for (BundleEntryComponent next : myBundle.getEntry()) { if (next.getResource() != null) { retVal.add(next.getResource()); - } else if (next.getResponse().getLocationElement().isEmpty() == false) { + } else if (!next.getResponse().getLocationElement().isEmpty()) { IdType id = new IdType(next.getResponse().getLocation()); String resourceType = id.getResourceType(); if (isNotBlank(resourceType)) {