From 77cbeb33c41c742c5404811b5bdf2db5e32fb55f Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 2 Jan 2025 18:39:25 -0500 Subject: [PATCH 1/5] Drop tag definition constraint --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 3 + .../jpa/dao/JpaStorageResourceParser.java | 10 ++ .../tasks/HapiFhirJpaMigrationTasks.java | 10 ++ .../ca/uhn/fhir/jpa/model/entity/BaseTag.java | 14 ++- .../jpa/dao/r4/FhirResourceDaoR4TagsTest.java | 102 +++++++++++++++--- 5 files changed, 121 insertions(+), 18 deletions(-) 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 b7627bf88be5..aede003ad5af 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 @@ -766,6 +766,9 @@ public BaseHasResource readEntity(IIdType theValueId, RequestDetails theRequest) * @return Returns true if the tag should be removed */ protected boolean shouldDroppedTagBeRemovedOnUpdate(RequestDetails theRequest, ResourceTag theTag) { + if (theTag.getTag() == null) { + return true; + } Set metaSnapshotModeTokens = null; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java index 125bf6f4f7f4..6d0243351cea 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java @@ -490,7 +490,17 @@ private R populateResourceMetadataRi( res.getMeta().getTag().clear(); res.getMeta().getProfile().clear(); res.getMeta().getSecurity().clear(); + + boolean haveWarnedForMissingTag = false; for (BaseTag next : theTagList) { + if (next.getTag() == null) { + if (!haveWarnedForMissingTag) { + ourLog.warn("Tag definition HFJ_TAG_DEF#{} is missing, returned Resource.meta may not be complete", next.getTagId()); + haveWarnedForMissingTag = true; + } + continue; + } + switch (next.getTag().getTagType()) { case PROFILE: res.getMeta().addProfile(next.getTag().getCode()); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 29c6aba1567d..7aca59e6cbc5 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -379,6 +379,16 @@ protected void init780() { .nullable() .withType(ColumnTypeEnum.TINYINT) .heavyweightSkipByDefault(); + + /* + * These two constraints were the last two that we had that used + * hibernate-generated names. Yay! + */ + version.onTable("HFJ_RES_TAG") + .dropForeignKey("20250102.10", "FKbfcjbaftmiwr3rxkwsy23vneo", "HFJ_TAG_DEF"); + version.onTable("HFJ_HISTORY_TAG") + .dropForeignKey("20250102.20", "FKtderym7awj6q8iq5c51xv4ndw", "HFJ_TAG_DEF"); + } protected void init780_afterPartitionChanges() { diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java index 5c502fd35a32..a45c8166e6fb 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java @@ -20,9 +20,13 @@ package ca.uhn.fhir.jpa.model.entity; import jakarta.persistence.Column; +import jakarta.persistence.ConstraintMode; +import jakarta.persistence.ForeignKey; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; import jakarta.persistence.MappedSuperclass; +import org.hibernate.annotations.NotFound; +import org.hibernate.annotations.NotFoundAction; import java.io.Serializable; @@ -31,9 +35,15 @@ public abstract class BaseTag extends BasePartitionable implements Serializable private static final long serialVersionUID = 1L; - // many baseTags -> one tag definition + /** + * Every tag has a reference to the tag definition. Note that this field + * must not have a FK constraint! In this case, Postgres (and maybe others) + * are horribly slow writing to the table if there's an FK constraint. + * See https://pganalyze.com/blog/5mins-postgres-multiXact-ids-foreign-keys-performance + */ @ManyToOne(cascade = {}) - @JoinColumn(name = "TAG_ID", nullable = false) + @JoinColumn(name = "TAG_ID", nullable = false, foreignKey = @ForeignKey(ConstraintMode.NO_CONSTRAINT)) + @NotFound(action = NotFoundAction.IGNORE) private TagDefinition myTag; @Column(name = "TAG_ID", insertable = false, updatable = false) diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java index f67f5af08ebe..4fa956204e77 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java @@ -2,12 +2,17 @@ import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.jpa.api.config.JpaStorageSettings; +import ca.uhn.fhir.jpa.dao.JpaStorageResourceParser; import ca.uhn.fhir.jpa.provider.BaseResourceProviderR4Test; import ca.uhn.fhir.jpa.searchparam.SearchParameterMap; import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.gclient.TokenClientParam; import ca.uhn.fhir.util.BundleBuilder; +import ca.uhn.test.util.LogbackTestExtension; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.spi.ILoggingEvent; +import jakarta.annotation.Nonnull; import org.hl7.fhir.instance.model.api.IIdType; import org.hl7.fhir.r4.model.Bundle; import org.hl7.fhir.r4.model.Enumerations; @@ -17,8 +22,10 @@ import org.hl7.fhir.r4.model.SearchParameter; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; -import jakarta.annotation.Nonnull; import java.util.List; import java.util.stream.Collectors; @@ -32,6 +39,9 @@ public class FhirResourceDaoR4TagsTest extends BaseResourceProviderR4Test { private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(FhirResourceDaoR4TagsTest.class); + @RegisterExtension + private LogbackTestExtension myLogbackTestExtension = new LogbackTestExtension(JpaStorageResourceParser.class, Level.WARN); + @Override @AfterEach public final void after() throws Exception { @@ -182,6 +192,66 @@ public void testDeleteResourceWithTags_NonVersionedTags_InTransaction() { } + /** + * We have removed the FK constraint from {@link ca.uhn.fhir.jpa.model.entity.ResourceTag} + * to {@link ca.uhn.fhir.jpa.model.entity.TagDefinition} so it's possible that tag + * definitions get deleted. This test makes sure we act sanely when that happens. + */ + @ParameterizedTest + @ValueSource(strings = {"read", "vread", "search", "history", "update"}) + public void testTagDefinitionDeleted(String theOperation) { + // Setup + Patient patient = new Patient(); + patient.setId("Patient/A"); + patient.getMeta().addProfile("http://profile0"); + patient.getMeta().addProfile("http://profile1"); + patient.getMeta().addTag("http://tag", "tag0", "display0"); + patient.getMeta().addTag("http://tag", "tag1", "display1"); + patient.getMeta().addSecurity("http://security", "security0", "display0"); + patient.getMeta().addSecurity("http://security", "security1", "display1"); + myPatientDao.update(patient, mySrd); + + // Test + runInTransaction(() -> { + assertEquals(6, myTagDefinitionDao.count()); + myTagDefinitionDao.deleteAll(); + assertEquals(0, myTagDefinitionDao.count()); + }); + Patient actualPatient; + switch (theOperation) { + case "read": + actualPatient = myPatientDao.read(new IdType("Patient/A"), mySrd); + break; + case "vread": + actualPatient = myPatientDao.read(new IdType("Patient/A/_history/1"), mySrd); + break; + case "search": + actualPatient = (Patient) myPatientDao.search(SearchParameterMap.newSynchronous(), mySrd).getResources(0, 1).get(0); + break; + case "history": + actualPatient = (Patient) mySystemDao.history(null, null, null, mySrd).getResources(0, 1).get(0); + break; + case "update": + Patient updatePatient = new Patient(); + updatePatient.setId("Patient/A"); + updatePatient.setActive(true); + actualPatient = (Patient) myPatientDao.update(updatePatient, mySrd).getResource(); + break; + default: + throw new IllegalArgumentException("Unknown operation: " + theOperation); + } + + // Verify + + assertEquals(0, actualPatient.getMeta().getProfile().size()); + assertEquals(0, actualPatient.getMeta().getTag().size()); + assertEquals(0, actualPatient.getMeta().getSecurity().size()); + + List logEvents = myLogbackTestExtension.getLogEvents(t -> t.getMessage().contains("Tag definition HFJ_TAG_DEF#{} is missing")); + assertThat(logEvents).as(() -> myLogbackTestExtension.getLogEvents().toString()).hasSize(1); + } + + /** * Make sure tags are preserved */ @@ -510,21 +580,6 @@ public void testInlineTags_Search_Security() { validatePatientSearchResultsForInlineTags(outcome); } - @Nonnull - public static SearchParameter createSecuritySearchParameter(FhirContext fhirContext) { - SearchParameter searchParameter = new SearchParameter(); - searchParameter.setId("SearchParameter/resource-security"); - for (String next : fhirContext.getResourceTypes().stream().sorted().collect(Collectors.toList())) { - searchParameter.addBase(next); - } - searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE); - searchParameter.setType(Enumerations.SearchParamType.TOKEN); - searchParameter.setCode("_security"); - searchParameter.setName("Security"); - searchParameter.setExpression("meta.security"); - return searchParameter; - } - private void validatePatientSearchResultsForInlineTags(Bundle outcome) { Patient patient; patient = (Patient) outcome.getEntry().get(0).getResource(); @@ -599,6 +654,21 @@ private void initializeVersioned() { assertEquals("2", myPatientDao.update(patient, mySrd).getId().getVersionIdPart()); } + @Nonnull + public static SearchParameter createSecuritySearchParameter(FhirContext fhirContext) { + SearchParameter searchParameter = new SearchParameter(); + searchParameter.setId("SearchParameter/resource-security"); + for (String next : fhirContext.getResourceTypes().stream().sorted().collect(Collectors.toList())) { + searchParameter.addBase(next); + } + searchParameter.setStatus(Enumerations.PublicationStatus.ACTIVE); + searchParameter.setType(Enumerations.SearchParamType.TOKEN); + searchParameter.setCode("_security"); + searchParameter.setName("Security"); + searchParameter.setExpression("meta.security"); + return searchParameter; + } + @Nonnull static List toTags(Patient patient) { return toTags(patient.getMeta()); From 1b39807d2b6638b1246c8fa38f33ba07fd4ae75b Mon Sep 17 00:00:00 2001 From: James Agnew Date: Thu, 2 Jan 2025 18:44:43 -0500 Subject: [PATCH 2/5] Add changelog --- .../7_8_0/6582-drop-tag-fk-constraint.yaml | 4 ++++ .../jpa/dao/r4/FhirResourceDaoR4TagsTest.java | 21 +++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6582-drop-tag-fk-constraint.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6582-drop-tag-fk-constraint.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6582-drop-tag-fk-constraint.yaml new file mode 100644 index 000000000000..e866502b2ee9 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_8_0/6582-drop-tag-fk-constraint.yaml @@ -0,0 +1,4 @@ +--- +type: perf +issue: 6582 +title: "Under heavy load, a foreign key constraint in the Tag Definition table (used for Tags, Security Labels, and Profile Definitions) can cause serious slowdowns when writing large numbers of resources (particularly if many resources contain the same tags/labels, or if the resources are being written individually or in smaller batches). This has been corrected." diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java index 4fa956204e77..49bfea138789 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; import java.util.List; @@ -198,9 +199,22 @@ public void testDeleteResourceWithTags_NonVersionedTags_InTransaction() { * definitions get deleted. This test makes sure we act sanely when that happens. */ @ParameterizedTest - @ValueSource(strings = {"read", "vread", "search", "history", "update"}) - public void testTagDefinitionDeleted(String theOperation) { + @CsvSource(value = { + "NONVERSIONED, read", + "NONVERSIONED, vread", + "NONVERSIONED, search", + "NONVERSIONED, history", + "NONVERSIONED, update", + "VERSIONED, read", + "VERSIONED, vread", + "VERSIONED, search", + "VERSIONED, history", + "VERSIONED, update" + }) + public void testTagDefinitionDeleted(JpaStorageSettings.TagStorageModeEnum theTagStorageMode, String theOperation) { // Setup + myStorageSettings.setTagStorageMode(theTagStorageMode); + Patient patient = new Patient(); patient.setId("Patient/A"); patient.getMeta().addProfile("http://profile0"); @@ -211,6 +225,9 @@ public void testTagDefinitionDeleted(String theOperation) { patient.getMeta().addSecurity("http://security", "security1", "display1"); myPatientDao.update(patient, mySrd); + patient.setActive(false); + myPatientDao.update(patient, mySrd); + // Test runInTransaction(() -> { assertEquals(6, myTagDefinitionDao.count()); From db60adf6fda8629e61dbeacb435b8b360c5d2efd Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 3 Jan 2025 05:55:49 -0500 Subject: [PATCH 3/5] Add tests --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 9 ++++---- .../ca/uhn/fhir/jpa/model/entity/BaseTag.java | 7 +++++++ .../jpa/model/entity/ResourceHistoryTag.java | 4 ++-- .../fhir/jpa/model/entity/ResourceTag.java | 2 +- .../jpa/dao/r4/FhirResourceDaoR4TagsTest.java | 21 ++++++++++--------- 5 files changed, 26 insertions(+), 17 deletions(-) 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 aede003ad5af..d428715ef234 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 @@ -671,17 +671,18 @@ private boolean updateTags( allResourceTagsNewAndOldFromTheEntity.forEach(tag -> { - // Don't keep duplicate tags - if (!allTagDefinitionsPresent.add(tag.getTag())) { + // Don't keep duplicate tags or tags with a missing definition + TagDefinition tagDefinition = tag.getTag(); + if (tagDefinition == null || !allTagDefinitionsPresent.add(tagDefinition)) { theEntity.getTags().remove(tag); } // Drop any tags that have been removed - if (!allResourceTagsFromTheResource.contains(tag)) { + if (tagDefinition != null && !allResourceTagsFromTheResource.contains(tag)) { if (shouldDroppedTagBeRemovedOnUpdate(theRequest, tag)) { theEntity.getTags().remove(tag); } else if (HapiExtensions.EXT_SUBSCRIPTION_MATCHING_STRATEGY.equals( - tag.getTag().getSystem())) { + tagDefinition.getSystem())) { theEntity.getTags().remove(tag); } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java index a45c8166e6fb..d945be797f19 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/BaseTag.java @@ -19,6 +19,7 @@ */ package ca.uhn.fhir.jpa.model.entity; +import jakarta.annotation.Nullable; import jakarta.persistence.Column; import jakarta.persistence.ConstraintMode; import jakarta.persistence.ForeignKey; @@ -53,6 +54,12 @@ public Long getTagId() { return myTagId; } + /** + * This can be null if the tag definition has been deleted. This + * should never happen unless someone has manually messed with + * the database, but it could happen. + */ + @Nullable public TagDefinition getTag() { return myTag; } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceHistoryTag.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceHistoryTag.java index 3467a68afe5b..64ddda7eba9f 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceHistoryTag.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceHistoryTag.java @@ -154,12 +154,12 @@ public JpaPid getResourcePid() { public String toString() { ToStringBuilder b = new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE); b.append("id", getId()); - if (getPartitionId() != null) { + if (getPartitionId().getPartitionId() != null) { b.append("partId", getPartitionId().getPartitionId()); } b.append("versionId", myResourceHistoryPid); b.append("resId", getResourceId()); - b.append("tag", getTag().getId()); + b.append("tag", getTagId()); return b.build(); } } diff --git a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTag.java b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTag.java index bda0dd2a84f9..29ff08da8281 100644 --- a/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTag.java +++ b/hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceTag.java @@ -175,7 +175,7 @@ public String toString() { b.append("partition", getPartitionId().getPartitionId()); } b.append("resId", getResourceId()); - b.append("tag", getTag().getId()); + b.append("tag", getTagId()); return b.build(); } } diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java index 49bfea138789..127c02f18871 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/dao/r4/FhirResourceDaoR4TagsTest.java @@ -200,16 +200,16 @@ public void testDeleteResourceWithTags_NonVersionedTags_InTransaction() { */ @ParameterizedTest @CsvSource(value = { - "NONVERSIONED, read", - "NONVERSIONED, vread", - "NONVERSIONED, search", - "NONVERSIONED, history", - "NONVERSIONED, update", - "VERSIONED, read", - "VERSIONED, vread", - "VERSIONED, search", - "VERSIONED, history", - "VERSIONED, update" + "NON_VERSIONED, read", + "NON_VERSIONED, vread", + "NON_VERSIONED, search", + "NON_VERSIONED, history", + "NON_VERSIONED, update", + "VERSIONED, read", + "VERSIONED, vread", + "VERSIONED, search", + "VERSIONED, history", + "VERSIONED, update" }) public void testTagDefinitionDeleted(JpaStorageSettings.TagStorageModeEnum theTagStorageMode, String theOperation) { // Setup @@ -229,6 +229,7 @@ public void testTagDefinitionDeleted(JpaStorageSettings.TagStorageModeEnum theTa myPatientDao.update(patient, mySrd); // Test + runInTransaction(() -> { assertEquals(6, myTagDefinitionDao.count()); myTagDefinitionDao.deleteAll(); From c21646c6fa8d8915add7d320c926dc25a1303909 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 3 Jan 2025 06:00:54 -0500 Subject: [PATCH 4/5] Add tests --- .../src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 3 +-- .../java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java | 4 +++- .../fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java | 7 ++----- pom.xml | 5 +++++ 4 files changed, 11 insertions(+), 8 deletions(-) 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 d428715ef234..974c316924c8 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 @@ -681,8 +681,7 @@ private boolean updateTags( if (tagDefinition != null && !allResourceTagsFromTheResource.contains(tag)) { if (shouldDroppedTagBeRemovedOnUpdate(theRequest, tag)) { theEntity.getTags().remove(tag); - } else if (HapiExtensions.EXT_SUBSCRIPTION_MATCHING_STRATEGY.equals( - tagDefinition.getSystem())) { + } else if (HapiExtensions.EXT_SUBSCRIPTION_MATCHING_STRATEGY.equals(tagDefinition.getSystem())) { theEntity.getTags().remove(tag); } } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java index 6d0243351cea..2dcaead52381 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/JpaStorageResourceParser.java @@ -495,7 +495,9 @@ private R populateResourceMetadataRi( for (BaseTag next : theTagList) { if (next.getTag() == null) { if (!haveWarnedForMissingTag) { - ourLog.warn("Tag definition HFJ_TAG_DEF#{} is missing, returned Resource.meta may not be complete", next.getTagId()); + ourLog.warn( + "Tag definition HFJ_TAG_DEF#{} is missing, returned Resource.meta may not be complete", + next.getTagId()); haveWarnedForMissingTag = true; } continue; diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java index 7aca59e6cbc5..194271cae167 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/migrate/tasks/HapiFhirJpaMigrationTasks.java @@ -384,11 +384,8 @@ protected void init780() { * These two constraints were the last two that we had that used * hibernate-generated names. Yay! */ - version.onTable("HFJ_RES_TAG") - .dropForeignKey("20250102.10", "FKbfcjbaftmiwr3rxkwsy23vneo", "HFJ_TAG_DEF"); - version.onTable("HFJ_HISTORY_TAG") - .dropForeignKey("20250102.20", "FKtderym7awj6q8iq5c51xv4ndw", "HFJ_TAG_DEF"); - + version.onTable("HFJ_RES_TAG").dropForeignKey("20250102.10", "FKbfcjbaftmiwr3rxkwsy23vneo", "HFJ_TAG_DEF"); + version.onTable("HFJ_HISTORY_TAG").dropForeignKey("20250102.20", "FKtderym7awj6q8iq5c51xv4ndw", "HFJ_TAG_DEF"); } protected void init780_afterPartitionChanges() { diff --git a/pom.xml b/pom.xml index ba100ec13a25..d98e7703af08 100644 --- a/pom.xml +++ b/pom.xml @@ -2528,6 +2528,11 @@ maven-dependency-plugin 3.8.1 + + org.apache.maven.plugins + maven-deploy-plugin + 3.1.3 + org.sonatype.plugins nexus-staging-maven-plugin From a2a29319156331b981b493aee18e427d282250b9 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 3 Jan 2025 08:15:11 -0500 Subject: [PATCH 5/5] Test fix --- .../java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java | 9 +++++---- .../jpa/migrate/taskdef/DropForeignKeyTask.java | 13 ++++++++++--- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java index 2bbee486af56..79274cec5784 100644 --- a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/JdbcUtils.java @@ -276,7 +276,8 @@ public static ColumnType getColumnType( } /** - * Retrieve all index names + * Retrieve all index names. The returned names will be in upper case + * always. */ public static Set getForeignKeys( DriverTypeEnum.ConnectionProperties theConnectionProperties, @@ -588,9 +589,9 @@ public static boolean isColumnNullable( } } - private static String massageIdentifier(DatabaseMetaData theMetadata, String theCatalog) throws SQLException { - String retVal = theCatalog; - if (theCatalog == null) { + public static String massageIdentifier(DatabaseMetaData theMetadata, String theIdentifier) throws SQLException { + String retVal = theIdentifier; + if (theIdentifier == null) { return null; } else if (theMetadata.storesLowerCaseIdentifiers()) { retVal = retVal.toLowerCase(); diff --git a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java index 40fd601ba619..909a547067d5 100644 --- a/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java +++ b/hapi-fhir-sql-migrate/src/main/java/ca/uhn/fhir/jpa/migrate/taskdef/DropForeignKeyTask.java @@ -26,12 +26,15 @@ import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.builder.EqualsBuilder; import org.apache.commons.lang3.builder.HashCodeBuilder; +import org.intellij.lang.annotations.Language; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.sql.Connection; import java.sql.SQLException; import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.Set; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -92,14 +95,18 @@ public void validate() { public void doExecute() throws SQLException { Set existing = JdbcUtils.getForeignKeys(getConnectionProperties(), myParentTableName, getTableName()); - if (!existing.contains(myConstraintName)) { + if (!existing.contains(myConstraintName.toUpperCase(Locale.US))) { logInfo(ourLog, "Don't have constraint named {} - No action performed", myConstraintName); return; } - List sqls = generateSql(getTableName(), myConstraintName, getDriverType()); + List sqlStatements; + try (Connection connection = getConnectionProperties().getDataSource().getConnection()) { + String constraintName = JdbcUtils.massageIdentifier(connection.getMetaData(), myConstraintName); + sqlStatements = generateSql(getTableName(), constraintName, getDriverType()); + } - for (String next : sqls) { + for (@Language("SQL") String next : sqlStatements) { executeSql(getTableName(), next); } }