Improve error message on unique constraint violation (#2182)

* Improve error message on unique constraint violation

* Add changelog

* Test fixes

* Test fix

hapi-fhir-base/src/main/resources/ca/uhn/fhir/i18n/hapi-messages.properties

@@ -80,7 +80,7 @@ ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.invalidMatchUrlMultipleMatches=Invalid match
 ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.transactionOperationWithMultipleMatchFailure=Failed to {0} resource with match URL "{1}" because this search matched {2} resources
 ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.transactionOperationFailedNoId=Failed to {0} resource in transaction because no ID was provided
 ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.transactionOperationFailedUnknownId=Failed to {0} resource in transaction because no resource could be found with ID {1}
-ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.uniqueIndexConflictFailure=Can not create resource of type {0} as it would create a duplicate index matching query: {1} (existing index belongs to {2})
+ca.uhn.fhir.jpa.dao.BaseHapiFhirDao.uniqueIndexConflictFailure=Can not create resource of type {0} as it would create a duplicate unique index matching query: {1} (existing index belongs to {2}, new unique index created by {3})
 
 ca.uhn.fhir.jpa.dao.BaseHapiFhirSystemDao.transactionContainsMultipleWithDuplicateId=Transaction bundle contains multiple resources with ID: {0}
 ca.uhn.fhir.jpa.dao.BaseHapiFhirSystemDao.transactionEntryHasInvalidVerb=Transaction bundle entry has missing or invalid HTTP Verb specified in Bundle.entry({1}).request.method. Found value: "{0}"

hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2182-improve-unique-constraint-error-message.yaml

@@ -0,0 +1,5 @@
+---
+type: add
+issue: 2182
+title: "When a unique index SearchParametr violation is blocked, the error message will now include the ID of the relevant
+   SearchParameter, in order to make troubleshooting easier."

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

@@ -206,7 +206,7 @@ public class SearchParamWithInlineReferencesExtractor {
 			for (String nextQueryString : queryStringsToPopulate) {
 				if (isNotBlank(nextQueryString)) {
 					ourLog.trace("Adding composite unique SP: {}", nextQueryString);
-					theParams.myCompositeStringUniques.add(new ResourceIndexedCompositeStringUnique(theEntity, nextQueryString));
+					theParams.myCompositeStringUniques.add(new ResourceIndexedCompositeStringUnique(theEntity, nextQueryString, next.getId()));
 				}
 			}
 		}
@@ -289,7 +289,13 @@ public class SearchParamWithInlineReferencesExtractor {
 				if (myDaoConfig.isUniqueIndexesCheckedBeforeSave()) {
 					ResourceIndexedCompositeStringUnique existing = myResourceIndexedCompositeStringUniqueDao.findByQueryString(next.getIndexString());
 					if (existing != null) {
-						String msg = myContext.getLocalizer().getMessage(BaseHapiFhirDao.class, "uniqueIndexConflictFailure", theEntity.getResourceType(), next.getIndexString(), existing.getResource().getIdDt().toUnqualifiedVersionless().getValue());
+
+						String searchParameterId = "(unknown)";
+						if (next.getSearchParameterId() != null) {
+							searchParameterId = next.getSearchParameterId().toUnqualifiedVersionless().getValue();
+						}
+
+						String msg = myContext.getLocalizer().getMessage(BaseHapiFhirDao.class, "uniqueIndexConflictFailure", theEntity.getResourceType(), next.getIndexString(), existing.getResource().getIdDt().toUnqualifiedVersionless().getValue(), searchParameterId);
 						throw new PreconditionFailedException(msg);
 					}
 				}

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

@@ -139,7 +139,7 @@ public class FhirResourceDaoR4ConcurrentWriteTest extends BaseJpaR4Test {
 					myPatientDao.create(p);
 				} catch (PreconditionFailedException e) {
 					// expected - This is as a result of the unique SP
-					assertThat(e.getMessage(), containsString("duplicate index matching query: Patient?gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"));
+					assertThat(e.getMessage(), containsString("duplicate unique index matching query: Patient?gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale"));
 				} catch (ResourceVersionConflictException e) {
 					// expected - This is as a result of the unique SP
 					assertThat(e.getMessage(), containsString("would have resulted in a duplicate value for a unique index"));

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

@@ -803,7 +803,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test {
 			myPatientDao.create(pt1).getId().toUnqualifiedVersionless();
 			fail();
 		} catch (PreconditionFailedException e) {
-			assertEquals("Can not create resource of type Patient as it would create a duplicate index matching query: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale (existing index belongs to Patient/" + id1.getIdPart() + ")", e.getMessage());
+			assertEquals("Can not create resource of type Patient as it would create a duplicate unique index matching query: Patient?birthdate=2011-01-01&gender=http%3A%2F%2Fhl7.org%2Ffhir%2Fadministrative-gender%7Cmale (existing index belongs to Patient/" + id1.getIdPart() + ", new unique index created by SearchParameter/patient-gender-birthdate)", e.getMessage());
 		}
 	}
 
@@ -1510,7 +1510,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test {
 			myPatientDao.create(pt1).getId().toUnqualifiedVersionless();
 			fail();
 		} catch (PreconditionFailedException e) {
-			// good
+			assertThat(e.getMessage(), containsString("new unique index created by SearchParameter/patient-gender-birthdate"));
 		}
 
 		Patient pt2 = new Patient();
@@ -1525,7 +1525,7 @@ public class FhirResourceDaoR4UniqueSearchParamTest extends BaseJpaR4Test {
 			myPatientDao.update(pt2);
 			fail();
 		} catch (PreconditionFailedException e) {
-			// good
+			assertThat(e.getMessage(), containsString("new unique index created by SearchParameter/patient-gender-birthdate"));
 		}
 
 	}

hapi-fhir-jpaserver-model/src/main/java/ca/uhn/fhir/jpa/model/entity/ResourceIndexedCompositeStringUnique.java

@@ -26,6 +26,7 @@ import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
+import org.hl7.fhir.instance.model.api.IIdType;
 
 import javax.persistence.*;
 
@@ -59,6 +60,8 @@ public class ResourceIndexedCompositeStringUnique extends BasePartitionable impl
 	@SuppressWarnings("unused")
 	@Column(name = PartitionablePartitionId.PARTITION_ID, insertable = false, updatable = false, nullable = true)
 	private Integer myPartitionIdValue;
+	@Transient
+	private IIdType mySearchParameterId;
 
 	/**
 	 * Constructor
@@ -70,10 +73,11 @@ public class ResourceIndexedCompositeStringUnique extends BasePartitionable impl
 	/**
 	 * Constructor
 	 */
-	public ResourceIndexedCompositeStringUnique(ResourceTable theResource, String theIndexString) {
+	public ResourceIndexedCompositeStringUnique(ResourceTable theResource, String theIndexString, IIdType theSearchParameterId) {
 		setResource(theResource);
 		setIndexString(theIndexString);
 		setPartitionId(theResource.getPartitionId());
+		setSearchParameterId(theSearchParameterId);
 	}
 
 	@Override
@@ -131,4 +135,18 @@ public class ResourceIndexedCompositeStringUnique extends BasePartitionable impl
 			.append("partition", getPartitionId())
 			.toString();
 	}
+
+	/**
+	 * Note: This field is not persisted, so it will only be populated for new indexes
+	 */
+	public void setSearchParameterId(IIdType theSearchParameterId) {
+		mySearchParameterId = theSearchParameterId;
+	}
+
+	/**
+	 * Note: This field is not persisted, so it will only be populated for new indexes
+	 */
+	public IIdType getSearchParameterId() {
+		return mySearchParameterId;
+	}
 }