diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index 60635b0f763..25b1479685e 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1319,17 +1319,18 @@ public abstract class BaseHapiFhirDao implements IDao, mySearchParamWithInlineReferencesExtractor.populateFromResource(newParams, this, theUpdateTime, theEntity, theResource, existingParams); changed = populateResourceIntoEntity(theRequest, theResource, theEntity, true); + if (changed.isChanged()) { + theEntity.setUpdated(theUpdateTime); + if (theResource instanceof IResource) { + theEntity.setLanguage(((IResource) theResource).getLanguage().getValue()); + } else { + theEntity.setLanguage(((IAnyResource) theResource).getLanguageElement().getValue()); + } - theEntity.setUpdated(theUpdateTime); - if (theResource instanceof IResource) { - theEntity.setLanguage(((IResource) theResource).getLanguage().getValue()); - } else { - theEntity.setLanguage(((IAnyResource) theResource).getLanguageElement().getValue()); + newParams.setParamsOn(theEntity); + theEntity.setIndexStatus(INDEX_STATUS_INDEXED); + populateFullTextFields(myContext, theResource, theEntity); } - - newParams.setParamsOn(theEntity); - theEntity.setIndexStatus(INDEX_STATUS_INDEXED); - populateFullTextFields(myContext, theResource, theEntity); } else { changed = populateResourceIntoEntity(theRequest, theResource, theEntity, false); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java index 78c8087ee9e..468d03905e3 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/util/CircularQueueCaptureQueriesListener.java @@ -144,6 +144,17 @@ public class CircularQueueCaptureQueriesListener extends BaseCaptureQueriesListe return getQueriesForCurrentThreadStartingWith("delete"); } + /** + * Log all captured UPDATE queries + */ + public void logUpdateQueriesForCurrentThread() { + List queries = getUpdateQueriesForCurrentThread() + .stream() + .map(CircularQueueCaptureQueriesListener::formatQueryAsSql) + .collect(Collectors.toList()); + ourLog.info("Select Queries:\n{}", String.join("\n", queries)); + } + /** * Log all captured SELECT queries */ diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java index 42a84d3862b..8c64289115b 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/config/TestR4Config.java @@ -4,6 +4,7 @@ import ca.uhn.fhir.jpa.util.CircularQueueCaptureQueriesListener; import ca.uhn.fhir.rest.server.interceptor.RequestValidatingInterceptor; import ca.uhn.fhir.validation.ResultSeverityEnum; import net.ttddyy.dsproxy.listener.SingleQueryCountHolder; +import net.ttddyy.dsproxy.listener.logging.SLF4JLogLevel; import net.ttddyy.dsproxy.support.ProxyDataSourceBuilder; import org.apache.commons.dbcp2.BasicDataSource; import org.springframework.context.annotation.Bean; @@ -102,7 +103,7 @@ public class TestR4Config extends BaseJavaConfigR4 { DataSource dataSource = ProxyDataSourceBuilder .create(retVal) -// .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") + .logQueryBySlf4j(SLF4JLogLevel.INFO, "SQL") // .logSlowQueryBySlf4j(10, TimeUnit.SECONDS) // .countQuery(new ThreadQueryCountHolder()) .beforeQuery(new BlockLargeNumbersOfParamsListener()) diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java index 9301cbd1c59..5702eed11f3 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4UpdateTest.java @@ -1,6 +1,8 @@ package ca.uhn.fhir.jpa.dao.r4; import ca.uhn.fhir.jpa.dao.DaoConfig; +import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable; +import ca.uhn.fhir.jpa.model.entity.ResourceTable; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.jpa.util.TestUtil; import ca.uhn.fhir.model.primitive.InstantDt; @@ -85,6 +87,58 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test { } + + @Test + public void testUpdateNotModifiedDoesNotAffectDates() { + IIdType id = runInTransaction(() -> { + Patient p = new Patient(); + p.addIdentifier().setSystem("urn:system").setValue("2"); + return myPatientDao.create(p).getId().toUnqualified(); + }); + + String createTime = runInTransaction(()->{ + List allResources = myResourceTableDao.findAll(); + assertEquals(1, allResources.size()); + ResourceTable resourceTable = allResources.get(0); + + List allHistory = myResourceHistoryTableDao.findAll(); + assertEquals(1, allHistory.size()); + ResourceHistoryTable historyTable = allHistory.get(0); + + assertEquals(resourceTable.getUpdated().getValueAsString(), historyTable.getUpdated().getValueAsString()); + return resourceTable.getUpdated().getValueAsString(); + }); + + myCaptureQueriesListener.clear(); + runInTransaction(()->{ + Patient p = new Patient(); + p.setId(id.getIdPart()); + p.addIdentifier().setSystem("urn:system").setValue("2"); + myPatientDao.update(p); + }); + myCaptureQueriesListener.logSelectQueriesForCurrentThread(); + // TODO: it'd be nice if this was lower + assertEquals(6, myCaptureQueriesListener.getSelectQueriesForCurrentThread().size()); + myCaptureQueriesListener.logUpdateQueriesForCurrentThread(); + assertEquals(0, myCaptureQueriesListener.getUpdateQueriesForCurrentThread().size()); + assertThat(myCaptureQueriesListener.getInsertQueriesForCurrentThread(), empty()); + assertThat(myCaptureQueriesListener.getDeleteQueriesForCurrentThread(), empty()); + + runInTransaction(()->{ + List allResources = myResourceTableDao.findAll(); + assertEquals(1, allResources.size()); + ResourceTable resourceTable = allResources.get(0); + + List allHistory = myResourceHistoryTableDao.findAll(); + assertEquals(1, allHistory.size()); + ResourceHistoryTable historyTable = allHistory.get(0); + + assertEquals(createTime, historyTable.getUpdated().getValueAsString()); + assertEquals(createTime, resourceTable.getUpdated().getValueAsString()); + }); + + } + @Test public void testDuplicateProfilesIgnored() { String name = "testDuplicateProfilesIgnored"; diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 94fa7bedd80..51d5b3a6c99 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -132,6 +132,11 @@ The second bug is that one case wasn't properly handled: when a resourceId with no version is provided. This executed the case where only resource type is provided. + + When updating a resource in the JPA server, if the contents have not actually changed + the resource version is not updated and no new version is created. In this situation, + the update time was modified however. It will no longer be updated. +