Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #2027 support prefer result=minimal for search requests #3148

Merged
merged 8 commits into from
Jan 14, 2022
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2021
* (C) Copyright IBM Corp. 2021, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -8,6 +8,7 @@

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

import com.ibm.fhir.model.resource.Resource;
Expand All @@ -26,23 +27,26 @@ public ReadResultDTO() {
// No Operation
}

public ReadResultDTO(List<Resource> resources) {
public ReadResultDTO(List<? extends Resource> resources) {
this.resources.addAll(resources);
}

/**
* @return the resources
*/
public List<Resource> getResources() {
return resources;
public List<? extends Resource> getResources() {
return Collections.unmodifiableList(this.resources);
}

/**
* Replace the contents of the internal resources list with the contents
* of the given resources list
* @param resources
* the resources to set
*/
public void setResources(List<Resource> resources) {
this.resources = resources;
public void setResources(List<? extends Resource> resources) {
this.resources.clear();
this.resources.addAll(resources);
}

public void addResource(Resource resource) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2021
* (C) Copyright IBM Corp. 2021, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand All @@ -26,6 +26,7 @@
import com.ibm.fhir.model.resource.Resource;
import com.ibm.fhir.operation.bulkdata.model.type.BulkDataContext;
import com.ibm.fhir.persistence.FHIRPersistence;
import com.ibm.fhir.persistence.ResourceResult;
import com.ibm.fhir.persistence.context.FHIRPersistenceContext;
import com.ibm.fhir.persistence.context.FHIRPersistenceContextFactory;
import com.ibm.fhir.search.SearchConstants;
Expand Down Expand Up @@ -128,8 +129,8 @@ public List<Resource> executeSearch(Set<String> patientIds) throws Exception {
FHIRPersistenceContext persistenceContext = FHIRPersistenceContextFactory.createPersistenceContext(null, searchContext);

Date startTime = new Date(System.currentTimeMillis());
List<Resource> resources = fhirPersistence.search(persistenceContext, resourceType).getResource();

List<ResourceResult<? extends Resource>> resourceResults = fhirPersistence.search(persistenceContext, resourceType).getResourceResults();
List<? extends Resource> resources = ResourceResult.toResourceList(resourceResults);
if (isDoDuplicationCheck) {
resources = resources.stream()
// the add returns false if the id already exists, which filters it out of the collection
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2021
* (C) Copyright IBM Corp. 2021, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -35,7 +35,7 @@ public SystemExportResourceHandler() {
// No Operation
}

public void fillChunkData(String exportFormat, ExportTransientUserData chunkData, List<Resource> resources) throws Exception {
public void fillChunkData(String exportFormat, ExportTransientUserData chunkData, List<? extends Resource> resources) throws Exception {
int resSubTotal = 0;
if (chunkData == null) {
String msg = "fillChunkDataBuffer: chunkData is null, this should never happen!";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019, 2021
* (C) Copyright IBM Corp. 2019, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -82,7 +82,7 @@ public SparkParquetWriter(boolean useIAM, String cosEndpoint, String apiKeyOrAcc
* @throws FHIRGeneratorException
* @implnote If a cos URI is passed, the file will be created if needed. For a file URI
*/
public void writeParquet(List<Resource> resources, String outDirName)
public void writeParquet(List<? extends Resource> resources, String outDirName)
throws FHIRGeneratorException {
List<String> jsonResources = new ArrayList<>();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019, 2021
* (C) Copyright IBM Corp. 2019, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -50,6 +50,7 @@
import com.ibm.fhir.operation.bulkdata.model.type.BulkDataContext;
import com.ibm.fhir.operation.bulkdata.model.type.OperationFields;
import com.ibm.fhir.persistence.FHIRPersistence;
import com.ibm.fhir.persistence.ResourceResult;
import com.ibm.fhir.persistence.context.FHIRPersistenceContext;
import com.ibm.fhir.persistence.context.FHIRPersistenceContextFactory;
import com.ibm.fhir.persistence.helper.FHIRPersistenceHelper;
Expand Down Expand Up @@ -203,7 +204,8 @@ public Object readItem() throws Exception {
try {
FHIRPersistenceContext persistenceContext = FHIRPersistenceContextFactory.createPersistenceContext(null, searchContext);
Date startTime = new Date(System.currentTimeMillis());
List<Resource> patientResources = fhirPersistence.search(persistenceContext, Patient.class).getResource();
List<ResourceResult<? extends Resource>> resourceResults = fhirPersistence.search(persistenceContext, Patient.class).getResourceResults();
List<? extends Resource> patientResources = ResourceResult.toResourceList(resourceResults);
if (isDoDuplicationCheck) {
patientResources = patientResources.stream()
.filter(r -> loadedPatientIds.add(r.getId()))
Expand Down Expand Up @@ -248,7 +250,7 @@ public Object readItem() throws Exception {
if (!patientIds.isEmpty()) {
handler.register(chunkData, ctx, fhirPersistence, pageSize, resourceType, searchParametersForResoureTypes, ctx.getSource());

List<Resource> resources = Patient.class.isAssignableFrom(resourceType) ?
List<? extends Resource> resources = Patient.class.isAssignableFrom(resourceType) ?
patientResources : handler.executeSearch(patientIds);
if (FHIRMediaType.APPLICATION_PARQUET.equals(ctx.getFhirExportFormat())) {
dto.setResources(resources);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2019, 2021
* (C) Copyright IBM Corp. 2019, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -48,6 +48,7 @@
import com.ibm.fhir.operation.bulkdata.model.type.BulkDataContext;
import com.ibm.fhir.operation.bulkdata.model.type.OperationFields;
import com.ibm.fhir.persistence.FHIRPersistence;
import com.ibm.fhir.persistence.ResourceResult;
import com.ibm.fhir.persistence.context.FHIRPersistenceContext;
import com.ibm.fhir.persistence.context.FHIRPersistenceContextFactory;
import com.ibm.fhir.persistence.helper.FHIRPersistenceHelper;
Expand Down Expand Up @@ -211,7 +212,7 @@ public Object readItem() throws Exception {
FHIRSearchContext searchContext = SearchUtil.parseQueryParameters(resourceType, queryParameters);
searchContext.setPageSize(pageSize);
searchContext.setPageNumber(pageNum);
List<Resource> resources = null;
List<? extends Resource> resources = null;

ReadResultDTO dto = new ReadResultDTO();

Expand All @@ -223,7 +224,9 @@ public Object readItem() throws Exception {
try {
// Execute the search query to obtain the page of resources
persistenceContext = FHIRPersistenceContextFactory.createPersistenceContext(null, searchContext);
resources = fhirPersistence.search(persistenceContext, resourceType).getResource();
List<ResourceResult<? extends Resource>> resourceResults = fhirPersistence.search(persistenceContext, resourceType).getResourceResults();
resources = ResourceResult.toResourceList(resourceResults);

if (isDoDuplicationCheck) {
resources = resources.stream()
// the add returns false if the id already exists, which filters it out of the collection
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2021
* (C) Copyright IBM Corp. 2021, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -442,7 +442,7 @@ private void pushFhirJsonsToCos(InputStream in, int dataLength) throws Exception
}
}

private void pushFhirParquetToCos(List<Resource> resources) throws Exception {
private void pushFhirParquetToCos(List<? extends Resource> resources) throws Exception {
if (chunkData == null) {
logger.warning("pushFhirParquetToCos: chunkData is null, this should never happen!");
throw new Exception("pushFhirParquetToCos: chunkData is null, this should never happen!");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2017, 2021
* (C) Copyright IBM Corp. 2017, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -111,11 +111,12 @@ List<Resource> search(String sqlSelect)
* Searches for Resources that contain one of the passed ids.
* @param resourceType - The type of the FHIR Resource
* @param resourceIds - A List of resource ids.
* @param includeResourceData - fetch the resource DATA column
* @return List<Resource> - A List of resources matching the the passed list of ids.
* @throws FHIRPersistenceDataAccessException
* @throws FHIRPersistenceDBConnectException
*/
List<Resource> searchByIds(String resourceType, List<Long> resourceIds) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException;
List<Resource> searchByIds(String resourceType, List<Long> resourceIds, boolean includeResourceData) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException;

/**
* Executes a count query based on the data contained in the passed {@link Select} statement, using its encapsulated search string and bind variables.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2017, 2021
* (C) Copyright IBM Corp. 2017, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -127,6 +127,10 @@ public class ResourceDAOImpl extends FHIRDbDAOImpl implements ResourceDAO {
"SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, R.DATA, LR.LOGICAL_ID, R.RESOURCE_PAYLOAD_KEY " +
"FROM %s_RESOURCES R, %s_LOGICAL_RESOURCES LR WHERE R.LOGICAL_RESOURCE_ID = LR.LOGICAL_RESOURCE_ID AND " +
"R.RESOURCE_ID IN ";
private static final String SQL_SEARCH_BY_IDS_NO_DATA =
"SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, CAST(NULL AS BLOB) AS DATA, LR.LOGICAL_ID, R.RESOURCE_PAYLOAD_KEY " +
"FROM %s_RESOURCES R, %s_LOGICAL_RESOURCES LR WHERE R.LOGICAL_RESOURCE_ID = LR.LOGICAL_RESOURCE_ID AND " +
"R.RESOURCE_ID IN ";

private static final String SQL_ORDER_BY_IDS = "ORDER BY CASE R.RESOURCE_ID ";

Expand Down Expand Up @@ -658,7 +662,7 @@ public List<Resource> search(String sqlSelect) throws FHIRPersistenceDataAccessE
}

@Override
public List<Resource> searchByIds(String resourceType, List<Long> resourceIds)
public List<Resource> searchByIds(String resourceType, List<Long> resourceIds, boolean includeResourceData)
throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException {
final String METHODNAME = "searchByIds";
log.entering(CLASSNAME, METHODNAME);
Expand All @@ -678,7 +682,11 @@ public List<Resource> searchByIds(String resourceType, List<Long> resourceIds)
double dbCallDuration;

try {
stmtString = getSearchByIdsSql(resourceType);
if (includeResourceData) {
stmtString = getSearchByIdsSql(resourceType);
punktilious marked this conversation as resolved.
Show resolved Hide resolved
} else {
stmtString = getSearchByIdsNoDataSql(resourceType);
}
idQuery.append(stmtString);
idQuery.append("(");
// resourceIds should have a max length of 1000 (the max page size)
Expand Down Expand Up @@ -719,6 +727,10 @@ protected String getSearchByIdsSql(String resourceType) {
return String.format(SQL_SEARCH_BY_IDS, resourceType, resourceType);
}

protected String getSearchByIdsNoDataSql(String resourceType) {
return String.format(SQL_SEARCH_BY_IDS_NO_DATA, resourceType, resourceType);
}

@Override
public int searchCount(String sqlSelectCount) throws FHIRPersistenceDataAccessException, FHIRPersistenceDBConnectException {
final String METHODNAME = "searchCount";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* (C) Copyright IBM Corp. 2021
* (C) Copyright IBM Corp. 2021, 2022
*
* SPDX-License-Identifier: Apache-2.0
*/
Expand Down Expand Up @@ -131,17 +131,21 @@ public class SearchQueryRenderer implements SearchQueryVisitor<QueryData> {
// Enable use of legacy whole-system search parameters for the search request
private final boolean legacyWholeSystemSearchParamsEnabled;

// Include DATA in the data fetch queries
private final boolean includeResourceData;
/**
* Public constructor
* @param identityCache
* @param rowOffset
* @param rowsPerPage
* @param includeResourceData
*/
public SearchQueryRenderer(JDBCIdentityCache identityCache,
int rowOffset, int rowsPerPage) {
int rowOffset, int rowsPerPage, boolean includeResourceData) {
this.identityCache = identityCache;
this.rowOffset = rowOffset;
this.rowsPerPage = rowsPerPage;
this.includeResourceData = includeResourceData;
this.legacyWholeSystemSearchParamsEnabled =
FHIRConfigHelper.getBooleanProperty(PROPERTY_SEARCH_ENABLE_LEGACY_WHOLE_SYSTEM_SEARCH_PARAMS, false);
}
Expand Down Expand Up @@ -352,7 +356,7 @@ public QueryData joinResources(QueryData queryData, boolean includeResourceTypeI
final String xxResources = resourceResources(queryData.getResourceType());
final String lrAliasName = "LR";
SelectAdapter select = Select.select("R.RESOURCE_ID", "R.LOGICAL_RESOURCE_ID", "R.VERSION_ID", "R.LAST_UPDATED",
"R.IS_DELETED", "R.DATA", "LR.LOGICAL_ID", "R.RESOURCE_PAYLOAD_KEY");
"R.IS_DELETED", getDataCol(), "LR.LOGICAL_ID", "R.RESOURCE_PAYLOAD_KEY");

// Resource type id is used for whole-system-search cases where the query
// can return resources of different types (e.g. both Patient and Observation)
Expand Down Expand Up @@ -401,7 +405,7 @@ public QueryData wrapInclude(QueryData query) {
final String rAlias = "R";
final String rTable = query.getResourceType() + "_RESOURCES";
SelectAdapter select = Select.select("LR.RESOURCE_ID", "LR.LOGICAL_RESOURCE_ID", "LR.VERSION_ID",
"LR.LAST_UPDATED", "LR.IS_DELETED", "R.DATA", "LR.LOGICAL_ID", "R.RESOURCE_PAYLOAD_KEY");
"LR.LAST_UPDATED", "LR.IS_DELETED", getDataCol(), "LR.LOGICAL_ID", "R.RESOURCE_PAYLOAD_KEY");
select.from(query.getQuery().build(), alias(lrAlias))
.innerJoin(rTable, alias(rAlias), on(lrAlias, "RESOURCE_ID").eq(rAlias, "RESOURCE_ID"));
return new QueryData(select, lrAlias, null, query.getResourceType(), 0);
Expand Down Expand Up @@ -2727,4 +2731,14 @@ private List<String> getValueAttributeNames(Type type) throws FHIRPersistenceExc
private boolean isWholeSystemSearch(String resourceType) {
return Resource.class.getSimpleName().equals(resourceType);
}

/**
* Get the select column entry for the resource data column. If
* the includeResourceData flag is false, the column is replaced
* with a literal NULL.
* @return
*/
private String getDataCol() {
return this.includeResourceData ? "R.DATA" : "CAST(NULL AS BLOB) AS DATA";
}
}
Loading