Fix $everything in SQL Server post-jakarta (#5580)

* Fix $everything in SQL Server post-jakarta

* Spotless
This commit is contained in:
James Agnew 2024-01-03 17:41:28 -05:00 committed by GitHub
parent b5c028f80a
commit b8cfdb086c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 142 additions and 4 deletions

View File

@ -110,6 +110,7 @@ public class SearchQueryBuilder {
private boolean dialectIsMySql;
private boolean myNeedResourceTableRoot;
private int myNextNearnessColumnId = 0;
private DbColumn mySelectedResourceIdColumn;
/**
* Constructor
@ -432,7 +433,8 @@ public class SearchQueryBuilder {
mySelect.addCustomColumns(
FunctionCall.count().setIsDistinct(true).addColumnParams(root.getResourceIdColumn()));
} else {
mySelect.addColumns(root.getResourceIdColumn());
mySelectedResourceIdColumn = root.getResourceIdColumn();
mySelect.addColumns(mySelectedResourceIdColumn);
}
mySelect.addFromTable(root.getTable());
myFirstPredicateBuilder = root;
@ -514,6 +516,26 @@ public class SearchQueryBuilder {
boolean isSqlServer = (myDialect instanceof SQLServerDialect);
if (isSqlServer) {
/*
* SQL server requires an ORDER BY clause to be present in the SQL if there is
* an OFFSET/FETCH FIRST clause, so if there isn't already an ORDER BY clause,
* the dialect will automatically add an order by with a pseudo-column name. This
* happens in SQLServer2012LimitHandler.
*
* But, SQL Server also pukes if you include an ORDER BY on a column that you
* aren't also SELECTing, if the select statement is DISTINCT. Who knows why SQL
* Server is so picky.. but anyhow, this causes an issue, so we manually replace
* the pseudo-column with an actual selected column.
*/
if (sql.startsWith("SELECT DISTINCT ")) {
if (sql.contains("order by @@version")) {
if (mySelectedResourceIdColumn != null) {
sql = sql.replace(
"order by @@version", "order by " + mySelectedResourceIdColumn.getColumnNameSQL());
}
}
}
// The SQLServerDialect has a bunch of one-off processing to deal with rules on when
// a limit can be used, so we can't rely on the flags that the limithandler exposes since
// the exact structure of the query depends on the parameters

View File

@ -1,45 +1,87 @@
package ca.uhn.fhir.jpa.dao.r5.database;
import ca.uhn.fhir.batch2.jobs.export.BulkDataExportProvider;
import ca.uhn.fhir.batch2.jobs.expunge.DeleteExpungeProvider;
import ca.uhn.fhir.batch2.jobs.reindex.ReindexProvider;
import ca.uhn.fhir.context.FhirContext;
import ca.uhn.fhir.jpa.api.dao.DaoRegistry;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.api.dao.IFhirResourceDaoPatient;
import ca.uhn.fhir.jpa.api.dao.PatientEverythingParameters;
import ca.uhn.fhir.jpa.embedded.JpaEmbeddedDatabase;
import ca.uhn.fhir.jpa.fql.provider.HfqlRestProvider;
import ca.uhn.fhir.jpa.graphql.GraphQLProvider;
import ca.uhn.fhir.jpa.migrate.HapiMigrationStorageSvc;
import ca.uhn.fhir.jpa.migrate.MigrationTaskList;
import ca.uhn.fhir.jpa.migrate.SchemaMigrator;
import ca.uhn.fhir.jpa.migrate.dao.HapiMigrationDao;
import ca.uhn.fhir.jpa.migrate.tasks.HapiFhirJpaMigrationTasks;
import ca.uhn.fhir.jpa.provider.DiffProvider;
import ca.uhn.fhir.jpa.provider.JpaCapabilityStatementProvider;
import ca.uhn.fhir.jpa.provider.ProcessMessageProvider;
import ca.uhn.fhir.jpa.provider.SubscriptionTriggeringProvider;
import ca.uhn.fhir.jpa.provider.TerminologyUploaderProvider;
import ca.uhn.fhir.jpa.provider.ValueSetOperationProvider;
import ca.uhn.fhir.jpa.search.DatabaseBackedPagingProvider;
import ca.uhn.fhir.jpa.test.BaseJpaTest;
import ca.uhn.fhir.jpa.test.config.TestR5Config;
import ca.uhn.fhir.narrative.DefaultThymeleafNarrativeGenerator;
import ca.uhn.fhir.rest.api.EncodingEnum;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.rest.client.api.IGenericClient;
import ca.uhn.fhir.rest.client.interceptor.LoggingInterceptor;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.interceptor.CorsInterceptor;
import ca.uhn.fhir.rest.server.provider.ResourceProviderFactory;
import ca.uhn.fhir.test.utilities.ITestDataBuilder;
import ca.uhn.fhir.test.utilities.server.RestfulServerConfigurerExtension;
import ca.uhn.fhir.test.utilities.server.RestfulServerExtension;
import ca.uhn.fhir.util.VersionEnum;
import jakarta.persistence.EntityManagerFactory;
import org.apache.commons.lang3.StringUtils;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r5.model.Bundle;
import org.hl7.fhir.r5.model.IdType;
import org.hl7.fhir.r5.model.IntegerType;
import org.hl7.fhir.r5.model.Parameters;
import org.hl7.fhir.r5.model.Patient;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.data.envers.repository.support.EnversRevisionRepositoryFactoryBean;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.transaction.PlatformTransactionManager;
import org.springframework.web.cors.CorsConfiguration;
import javax.sql.DataSource;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Properties;
import java.util.Set;
import static ca.uhn.fhir.jpa.model.util.JpaConstants.OPERATION_EVERYTHING;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@ExtendWith(SpringExtension.class)
@EnableJpaRepositories(repositoryFactoryBeanClass = EnversRevisionRepositoryFactoryBean.class)
@ContextConfiguration(classes = {BaseDatabaseVerificationIT.TestConfig.class})
public abstract class BaseDatabaseVerificationIT {
public abstract class BaseDatabaseVerificationIT extends BaseJpaTest implements ITestDataBuilder {
private static final Logger ourLog = LoggerFactory.getLogger(BaseDatabaseVerificationIT.class);
private static final String MIGRATION_TABLENAME = "MIGRATIONS";
@ -50,7 +92,34 @@ public abstract class BaseDatabaseVerificationIT {
JpaEmbeddedDatabase myJpaEmbeddedDatabase;
@Autowired
IFhirResourceDao<Patient> myPatientDao;
IFhirResourceDaoPatient<Patient> myPatientDao;
@Autowired
private FhirContext myFhirContext;
@Autowired
private DaoRegistry myDaoRegistry;
@Autowired
private PlatformTransactionManager myTxManager;
@Autowired
protected ResourceProviderFactory myResourceProviders;
@Autowired
private DatabaseBackedPagingProvider myPagingProvider;
@RegisterExtension
protected RestfulServerExtension myServer = new RestfulServerExtension(FhirContext.forR5Cached());
@RegisterExtension
protected RestfulServerConfigurerExtension myServerConfigurer = new RestfulServerConfigurerExtension(() -> myServer)
.withServerBeforeAll(s -> {
s.registerProviders(myResourceProviders.createProviders());
s.setDefaultResponseEncoding(EncodingEnum.JSON);
s.setDefaultPrettyPrint(false);
s.setPagingProvider(myPagingProvider);
});
@ParameterizedTest
@ -80,6 +149,34 @@ public abstract class BaseDatabaseVerificationIT {
}
@Test
public void testEverything() {
Set<String> expectedIds = new HashSet<>();
expectedIds.add(createPatient(withId("A"), withActiveTrue()).toUnqualifiedVersionless().getValue());
for (int i = 0; i < 25; i++) {
expectedIds.add(createObservation(withSubject("Patient/A")).toUnqualifiedVersionless().getValue());
}
IGenericClient client = myServer.getFhirClient();
Bundle outcome = client
.operation()
.onInstanceVersion(new IdType("Patient/A"))
.named(OPERATION_EVERYTHING)
.withNoParameters(Parameters.class)
.returnResourceType(Bundle.class)
.execute();
List<String> values = toUnqualifiedVersionlessIdValues(outcome);
while (outcome.getLink("next") != null) {
outcome = client.loadPage().next(outcome).execute();
values.addAll(toUnqualifiedVersionlessIdValues(outcome));
}
assertThat(values.toString(), values, containsInAnyOrder(expectedIds.toArray(new String[0])));
}
@Configuration
public static class TestConfig extends TestR5Config {
@ -142,6 +239,25 @@ public abstract class BaseDatabaseVerificationIT {
}
}
@Override
public IIdType doCreateResource(IBaseResource theResource) {
return myDaoRegistry.getResourceDao(myFhirContext.getResourceType(theResource)).create(theResource, new SystemRequestDetails()).getId();
}
@Override
public IIdType doUpdateResource(IBaseResource theResource) {
return myDaoRegistry.getResourceDao(myFhirContext.getResourceType(theResource)).update(theResource, new SystemRequestDetails()).getId();
}
@Override
public FhirContext getFhirContext() {
return myFhirContext;
}
@Override
protected PlatformTransactionManager getTxManager() {
return myTxManager;
}
}

View File

@ -1,11 +1,11 @@
package ca.uhn.fhir.jpa.dao.r5.database;
import ca.uhn.fhir.jpa.embedded.MsSqlEmbeddedDatabase;
import ca.uhn.fhir.jpa.model.dialect.HapiFhirPostgresDialect;
import ca.uhn.fhir.jpa.model.dialect.HapiFhirSQLServerDialect;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.transaction.PlatformTransactionManager;
@ContextConfiguration(classes = {
DatabaseVerificationWithMsSqlIT.TestConfig.class