Merge branch 'master' of github.com:jamesagnew/hapi-fhir

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java

@@ -1167,7 +1167,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 	public SearchBuilder newSearchBuilder() {
 		SearchBuilder builder = new SearchBuilder(
 			getContext(), myEntityManager, myFulltextSearchSvc, this, myResourceIndexedSearchParamUriDao,
-			myForcedIdDao,	myTerminologySvc, mySerarchParamRegistry, myResourceTagDao, myResourceViewDao);
+			myForcedIdDao, myTerminologySvc, mySerarchParamRegistry, myResourceTagDao, myResourceViewDao);
 		return builder;
 	}
 
@@ -1301,20 +1301,24 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 				}
 			}
 
-			// Don't keep duplicate tags
+			Set<ResourceTag> allTagsNew = getAllTagDefinitions(theEntity);
 			Set<TagDefinition> allDefsPresent = new HashSet<>();
-			theEntity.getTags().removeIf(theResourceTag -> !allDefsPresent.add(theResourceTag.getTag()));
+			allTagsNew.forEach(tag -> {
 
-			// Remove any tags that have been removed
+				// Don't keep duplicate tags
-			for (ResourceTag next : allTagsOld) {
+				if (!allDefsPresent.add(tag.getTag())) {
-				if (!allDefs.contains(next)) {
+					theEntity.getTags().remove(tag);
-					if (shouldDroppedTagBeRemovedOnUpdate(theRequest, next)) {
+				}
-						theEntity.getTags().remove(next);
+
+				// Drop any tags that have been removed
+				if (!allDefs.contains(tag)) {
+					if (shouldDroppedTagBeRemovedOnUpdate(theRequest, tag)) {
+						theEntity.getTags().remove(tag);
 					}
 				}
-			}
 
-			Set<ResourceTag> allTagsNew = getAllTagDefinitions(theEntity);
+			});
+
 			if (!allTagsOld.equals(allTagsNew)) {
 				changed = true;
 			}
@@ -1473,6 +1477,15 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 		return retVal;
 	}
 
+	/**
+	 * Subclasses may override to provide behaviour. Called when a pre-existing resource has been updated in the database
+	 *
+	 * @param theEntity The resource
+	 */
+	protected void postDelete(ResourceTable theEntity) {
+		// nothing
+	}
+
 	/**
 	 * Subclasses may override to provide behaviour. Called when a resource has been inserted into the database for the first time.
 	 *
@@ -1483,15 +1496,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 		// nothing
 	}
 
-	/**
-	 * Subclasses may override to provide behaviour. Called when a pre-existing resource has been updated in the database
-	 *
-	 * @param theEntity   The resource
-	 */
-	protected void postDelete(ResourceTable theEntity) {
-		// nothing
-	}
-
 	/**
 	 * Subclasses may override to provide behaviour. Called when a pre-existing resource has been updated in the database
 	 *
@@ -1638,7 +1642,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 			resourceEncoding = history.getEncoding();
 			myTagList = history.getTags();
 		} else if (theEntity instanceof ResourceTable) {
-			ResourceTable resource = (ResourceTable)theEntity;
+			ResourceTable resource = (ResourceTable) theEntity;
 			ResourceHistoryTable history = myResourceHistoryTableDao.findForIdAndVersion(theEntity.getId(), theEntity.getVersion());
 			if (history == null) {
 				return null;
@@ -1648,7 +1652,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 			myTagList = resource.getTags();
 		} else if (theEntity instanceof ResourceSearchView) {
 			// This is the search View
-			ResourceSearchView myView = (ResourceSearchView)theEntity;
+			ResourceSearchView myView = (ResourceSearchView) theEntity;
 			resourceBytes = myView.getResource();
 			resourceEncoding = myView.getEncoding();
 			if (theTagList == null)
@@ -2046,6 +2050,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 			postPersist(theEntity, (T) theResource);
 
 		} else if (theEntity.getDeleted() != null) {
+			theEntity = myEntityManager.merge(theEntity);
 
 			postDelete(theEntity);
 
@@ -2191,12 +2196,11 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
 
 		} // if thePerformIndexing
 
-		theEntity = myEntityManager.merge(theEntity);
-
 		if (theResource != null) {
 			populateResourceIdFromEntity(theEntity, theResource);
 		}
 
+
 		return theEntity;
 	}
 

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java

@@ -395,16 +395,6 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 					"This server cannot create an entity with a user-specified numeric ID - Client should not specify an ID when creating a new resource, or should include at least one letter in the ID to force a client-defined ID");
 			}
 			createForcedIdIfNeeded(entity, theResource.getIdElement());
-
-			if (entity.getForcedId() != null) {
-				try {
-					translateForcedIdToPid(getResourceName(), theResource.getIdElement().getIdPart());
-					throw new UnprocessableEntityException(getContext().getLocalizer().getMessage(BaseHapiFhirResourceDao.class, "duplicateCreateForcedId", theResource.getIdElement().getIdPart()));
-				} catch (ResourceNotFoundException e) {
-					// good, this ID doesn't exist so we can create it
-				}
-			}
-
 		}
 
 		// Notify interceptors
@@ -1211,7 +1201,9 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
 			}
 		} else {
 			/*
-			 * Note: resourcdeId will not be null or empty here, because we check it and reject requests in BaseOutcomeReturningMethodBindingWithResourceParam
+			 * Note: resourceId will not be null or empty here, because we
+			 * check it and reject requests in
+			 * BaseOutcomeReturningMethodBindingWithResourceParam
 			 */
 			resourceId = theResource.getIdElement();
 

hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ForcedId.java

@@ -27,7 +27,6 @@ import javax.persistence.*;
 @Entity()
 @Table(name = "HFJ_FORCED_ID", uniqueConstraints = {
 	@UniqueConstraint(name = "IDX_FORCEDID_RESID", columnNames = {"RESOURCE_PID"}),
-	@UniqueConstraint(name = "IDX_FORCEDID_TYPE_RESID", columnNames = {"RESOURCE_TYPE", "RESOURCE_PID"}),
 	@UniqueConstraint(name = "IDX_FORCEDID_TYPE_FID", columnNames = {"RESOURCE_TYPE", "FORCED_ID"})
 }, indexes = {
 	/*

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu2Config.java

@@ -12,6 +12,7 @@ import org.slf4j.LoggerFactory;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Lazy;
+import org.springframework.core.env.Environment;
 import org.springframework.orm.jpa.JpaTransactionManager;
 import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
@@ -43,11 +44,6 @@ public class TestDstu2Config extends BaseJavaConfigDstu2 {
 	private Exception myLastStackTrace;
 	private String myLastStackTraceThreadName;
 
-	@Bean(name="maxDatabaseThreadsForTest")
-	public Integer getMaxThread(){
-		return ourMaxThreads;
-	}
-
 	@Bean()
 	public DaoConfig daoConfig() {
 		return new DaoConfig();
@@ -131,6 +127,11 @@ public class TestDstu2Config extends BaseJavaConfigDstu2 {
 		return retVal;
 	}
 
+	@Bean(name = "maxDatabaseThreadsForTest")
+	public Integer getMaxThread() {
+		return ourMaxThreads;
+	}
+
 	private Properties jpaProperties() {
 		Properties extraProperties = new Properties();
 		extraProperties.put("hibernate.format_sql", "true");
@@ -165,4 +166,9 @@ public class TestDstu2Config extends BaseJavaConfigDstu2 {
 		return retVal;
 	}
 
+	@Bean
+	public UnregisterScheduledProcessor unregisterScheduledProcessor(Environment theEnv) {
+		return new UnregisterScheduledProcessor(theEnv);
+	}
+
 }

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestDstu3Config.java

@@ -6,14 +6,9 @@ import ca.uhn.fhir.jpa.subscription.email.IEmailSender;
 import ca.uhn.fhir.jpa.subscription.email.JavaMailEmailSender;
 import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor;
 import ca.uhn.fhir.validation.ResultSeverityEnum;
-import net.ttddyy.dsproxy.listener.logging.SLF4JLogLevel;
 import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder;
 import org.apache.commons.dbcp2.BasicDataSource;
 import org.hibernate.jpa.HibernatePersistenceProvider;
-import org.springframework.beans.BeansException;
-import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
-import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
-import org.springframework.beans.factory.support.DefaultListableBeanFactory;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Lazy;
@@ -22,13 +17,11 @@ import org.springframework.context.support.PropertySourcesPlaceholderConfigurer;
 import org.springframework.core.env.Environment;
 import org.springframework.orm.jpa.JpaTransactionManager;
 import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean;
-import org.springframework.scheduling.annotation.ScheduledAnnotationBeanPostProcessor;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
 
 import javax.persistence.EntityManagerFactory;
 import javax.sql.DataSource;
 import java.sql.Connection;
-import java.sql.SQLException;
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
@@ -194,23 +187,4 @@ public class TestDstu3Config extends BaseJavaConfigDstu3 {
 	}
 
 
-	public class UnregisterScheduledProcessor implements BeanFactoryPostProcessor {
-
-		private final Environment myEnvironment;
-
-		public UnregisterScheduledProcessor(Environment theEnv) {
-			myEnvironment = theEnv;
-		}
-
-		@Override
-		public void postProcessBeanFactory(final ConfigurableListableBeanFactory beanFactory) throws BeansException {
-			String schedulingDisabled = myEnvironment.getProperty("scheduling_disabled");
-			if ("true".equals(schedulingDisabled)) {
-				for (String beanName : beanFactory.getBeanNamesForType(ScheduledAnnotationBeanPostProcessor.class)) {
-					((DefaultListableBeanFactory) beanFactory).removeBeanDefinition(beanName);
-				}
-			}
-		}
-	}
-
 }

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java

@@ -7,15 +7,13 @@ import net.ttddyy.dsproxy.listener.ThreadQueryCountHolder;
 import net.ttddyy.dsproxy.listener.logging.SLF4JLogLevel;
 import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder;
 import org.apache.commons.dbcp2.BasicDataSource;
-import org.hibernate.jpa.HibernatePersistenceProvider;
 import org.hibernate.query.criteria.LiteralHandlingMode;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.Lazy;
-import org.springframework.orm.hibernate5.HibernateExceptionTranslator;
+import org.springframework.core.env.Environment;
 import org.springframework.orm.jpa.JpaTransactionManager;
 import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean;
-import org.springframework.orm.jpa.vendor.HibernateJpaDialect;
 import org.springframework.transaction.annotation.EnableTransactionManagement;
 
 import javax.persistence.EntityManagerFactory;
@@ -163,6 +161,11 @@ public class TestR4Config extends BaseJavaConfigR4 {
 		return retVal;
 	}
 
+	@Bean
+	public UnregisterScheduledProcessor unregisterScheduledProcessor(Environment theEnv) {
+		return new UnregisterScheduledProcessor(theEnv);
+	}
+
 	public static int getMaxThreads() {
 		return ourMaxThreads;
 	}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/UnregisterScheduledProcessor.java

@@ -0,0 +1,39 @@
+package ca.uhn.fhir.jpa.config;
+
+import org.springframework.beans.BeansException;
+import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.beans.factory.support.DefaultListableBeanFactory;
+import org.springframework.core.env.Environment;
+import org.springframework.scheduling.annotation.ScheduledAnnotationBeanPostProcessor;
+import org.springframework.scheduling.concurrent.ExecutorConfigurationSupport;
+
+/**
+ * This bean postprocessor disables all scheduled tasks. It is intended
+ * only to be used in unit tests in circumstances where scheduled
+ * tasks cause issues.
+ */
+public class UnregisterScheduledProcessor implements BeanFactoryPostProcessor {
+
+	private final Environment myEnvironment;
+
+	public UnregisterScheduledProcessor(Environment theEnv) {
+		myEnvironment = theEnv;
+	}
+
+	@Override
+	public void postProcessBeanFactory(final ConfigurableListableBeanFactory beanFactory) throws BeansException {
+		String schedulingDisabled = myEnvironment.getProperty("scheduling_disabled");
+		if ("true".equals(schedulingDisabled)) {
+			for (String beanName : beanFactory.getBeanNamesForType(ScheduledAnnotationBeanPostProcessor.class)) {
+				((DefaultListableBeanFactory) beanFactory).removeBeanDefinition(beanName);
+			}
+
+			for (String beanName : beanFactory.getBeanNamesForType(ExecutorConfigurationSupport.class)) {
+				ExecutorConfigurationSupport executorConfigSupport = ((DefaultListableBeanFactory) beanFactory).getBean(beanName, ExecutorConfigurationSupport.class);
+				executorConfigSupport.shutdown();
+			}
+		}
+
+	}
+}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4QueryCountTest.java

@@ -0,0 +1,109 @@
+package ca.uhn.fhir.jpa.dao.r4;
+
+import ca.uhn.fhir.jpa.dao.DaoConfig;
+import ca.uhn.fhir.util.TestUtil;
+import net.ttddyy.dsproxy.QueryCountHolder;
+import org.hl7.fhir.instance.model.api.IIdType;
+import org.hl7.fhir.r4.model.DateTimeType;
+import org.hl7.fhir.r4.model.Patient;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Test;
+import org.springframework.test.context.TestPropertySource;
+
+import static org.junit.Assert.*;
+
+@TestPropertySource(properties = {
+	"scheduling_disabled=true"
+})
+public class FhirResourceDaoR4QueryCountTest extends BaseJpaR4Test {
+	private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4QueryCountTest.class);
+
+	@After
+	public void afterResetDao() {
+		myDaoConfig.setResourceMetaCountHardLimit(new DaoConfig().getResourceMetaCountHardLimit());
+		myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields());
+	}
+
+	@Test
+	public void testCreateClientAssignedId() {
+		myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
+
+		QueryCountHolder.clear();
+		ourLog.info("** Starting Update Non-Existing resource with client assigned ID");
+		Patient p = new Patient();
+		p.setId("A");
+		p.getPhotoFirstRep().setCreationElement(new DateTimeType("2011")); // non-indexed field
+		myPatientDao.update(p).getId().toUnqualifiedVersionless();
+
+		assertEquals(1, QueryCountHolder.getGrandTotal().getSelect());
+		assertEquals(4, QueryCountHolder.getGrandTotal().getInsert());
+		assertEquals(0, QueryCountHolder.getGrandTotal().getDelete());
+		// Because of the forced ID's bidirectional link HFJ_RESOURCE <-> HFJ_FORCED_ID
+		assertEquals(1, QueryCountHolder.getGrandTotal().getUpdate());
+		runInTransaction(() -> {
+			assertEquals(1, myResourceTableDao.count());
+			assertEquals(1, myResourceHistoryTableDao.count());
+			assertEquals(1, myForcedIdDao.count());
+			assertEquals(1, myResourceIndexedSearchParamTokenDao.count());
+		});
+
+		// Ok how about an update
+
+		QueryCountHolder.clear();
+		ourLog.info("** Starting Update Existing resource with client assigned ID");
+		p = new Patient();
+		p.setId("A");
+		p.getPhotoFirstRep().setCreationElement(new DateTimeType("2012")); // non-indexed field
+		myPatientDao.update(p).getId().toUnqualifiedVersionless();
+
+		assertEquals(5, QueryCountHolder.getGrandTotal().getSelect());
+		assertEquals(1, QueryCountHolder.getGrandTotal().getInsert());
+		assertEquals(0, QueryCountHolder.getGrandTotal().getDelete());
+		assertEquals(1, QueryCountHolder.getGrandTotal().getUpdate());
+		runInTransaction(() -> {
+			assertEquals(1, myResourceTableDao.count());
+			assertEquals(2, myResourceHistoryTableDao.count());
+			assertEquals(1, myForcedIdDao.count());
+			assertEquals(1, myResourceIndexedSearchParamTokenDao.count());
+		});
+
+	}
+
+
+		@Test
+	public void testOneRowPerUpdate() {
+		myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
+
+		QueryCountHolder.clear();
+		Patient p = new Patient();
+		p.getPhotoFirstRep().setCreationElement(new DateTimeType("2011")); // non-indexed field
+		IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
+
+		assertEquals(3, QueryCountHolder.getGrandTotal().getInsert());
+		runInTransaction(() -> {
+			assertEquals(1, myResourceTableDao.count());
+			assertEquals(1, myResourceHistoryTableDao.count());
+		});
+
+		QueryCountHolder.clear();
+		p = new Patient();
+		p.setId(id);
+		p.getPhotoFirstRep().setCreationElement(new DateTimeType("2012")); // non-indexed field
+		myPatientDao.update(p).getId().toUnqualifiedVersionless();
+
+		assertEquals(1, QueryCountHolder.getGrandTotal().getInsert());
+		runInTransaction(() -> {
+			assertEquals(1, myResourceTableDao.count());
+			assertEquals(2, myResourceHistoryTableDao.count());
+		});
+
+	}
+
+
+	@AfterClass
+	public static void afterClassClearContext() {
+		TestUtil.clearAllStaticFieldsForUnitTest();
+	}
+
+}

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTagSnapshotTest.java

@@ -39,9 +39,7 @@ public class FhirResourceDaoR4UpdateTagSnapshotTest extends BaseJpaR4Test {
 		myPatientDao.update(p, mySrd);
 
 		p = myPatientDao.read(new IdType("A"), mySrd);
-		// It would be nice if this didn't trigger a version update but
+		assertEquals("1", p.getIdElement().getVersionIdPart());
-		// i guess it's not so bad that it does
-		assertEquals("2", p.getIdElement().getVersionIdPart());
 		assertEquals(true, p.getActive());
 		assertEquals(1, p.getMeta().getTag().size());
 	}
@@ -86,9 +84,7 @@ public class FhirResourceDaoR4UpdateTagSnapshotTest extends BaseJpaR4Test {
 		myPatientDao.update(p, mySrd);
 
 		p = myPatientDao.read(new IdType("A"), mySrd);
-		// It would be nice if this didn't trigger a version update but
+		assertEquals("1", p.getIdElement().getVersionIdPart());
-		// i guess it's not so bad that it does
-		assertEquals("2", p.getIdElement().getVersionIdPart());
 		assertEquals(true, p.getActive());
 		assertEquals(1, p.getMeta().getTag().size());
 		assertEquals("urn:foo", p.getMeta().getTag().get(0).getSystem());
@@ -136,9 +132,7 @@ public class FhirResourceDaoR4UpdateTagSnapshotTest extends BaseJpaR4Test {
 		p = myPatientDao.read(new IdType("A"), mySrd);
 		assertEquals(true, p.getActive());
 		assertEquals(0, p.getMeta().getTag().size());
-		// It would be nice if this didn't trigger a version update but
+		assertEquals("1", p.getIdElement().getVersionIdPart());
-		// i guess it's not so bad that it does
-		assertEquals("2", p.getIdElement().getVersionIdPart());
 	}
 
 	@AfterClass

hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java

@@ -39,35 +39,6 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
 		myDaoConfig.setIndexMissingFields(new DaoConfig().getIndexMissingFields());
 	}
 
-	@Test
-	public void testOneRowPerUpdate(){
-		myDaoConfig.setIndexMissingFields(DaoConfig.IndexEnabledEnum.DISABLED);
-
-		QueryCountHolder.clear();
-		Patient p = new Patient();
-		p.getPhotoFirstRep().setCreationElement(new DateTimeType("2011")); // non-indexed field
-		IIdType id = myPatientDao.create(p).getId().toUnqualifiedVersionless();
-
-		assertEquals(3, QueryCountHolder.getGrandTotal().getInsert());
-		runInTransaction(()->{
-			assertEquals(1, myResourceTableDao.count());
-			assertEquals(1, myResourceHistoryTableDao.count());
-		});
-
-		QueryCountHolder.clear();
-		p = new Patient();
-		p.setId(id);
-		p.getPhotoFirstRep().setCreationElement(new DateTimeType("2012")); // non-indexed field
-		myPatientDao.update(p).getId().toUnqualifiedVersionless();
-
-		assertEquals(1, QueryCountHolder.getGrandTotal().getInsert());
-		runInTransaction(()->{
-			assertEquals(1, myResourceTableDao.count());
-			assertEquals(2, myResourceHistoryTableDao.count());
-		});
-
-	}
-
 	@Test
 	public void testCreateAndUpdateWithoutRequest() {
 		String methodName = "testUpdateByUrl";