From 82bf8cdc9d777e4ac92a5bfd813056c2e789f852 Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Tue, 27 Apr 2021 09:29:08 -0400 Subject: [PATCH] Code review updates --- .../java/ca/uhn/fhir/util/TerserUtil.java | 25 ++++++------------- .../address/impl/LoquateAddressValidator.java | 20 +++++---------- .../fields/FieldValidatingInterceptor.java | 1 - 3 files changed, 13 insertions(+), 33 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java index fe2de45066e..02cf84537f9 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java @@ -25,6 +25,7 @@ import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition; import ca.uhn.fhir.context.BaseRuntimeElementDefinition; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.context.RuntimeResourceDefinition; +import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.tuple.Triple; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseResource; @@ -172,9 +173,7 @@ public final class TerserUtil { RuntimeResourceDefinition definition = theFhirContext.getResourceDefinition(theFrom); BaseRuntimeChildDefinition childDefinition = definition.getChildByName(theField); - if (childDefinition == null) { - throw new IllegalArgumentException(String.format("Unable to find child definition %s in %s", theField, theFrom)); - } + Validate.notNull(childDefinition); List theFromFieldValues = childDefinition.getAccessor().getValues(theFrom); List theToFieldValues = childDefinition.getAccessor().getValues(theTo); @@ -226,9 +225,7 @@ public final class TerserUtil { } final Method method = getMethod(theItem1, EQUALS_DEEP); - if (method == null) { - throw new IllegalArgumentException(String.format("Instance %s do not provide %s method", theItem1, EQUALS_DEEP)); - } + Validate.notNull(method); return equals(theItem1, theItem2, method); } @@ -315,9 +312,7 @@ public final class TerserUtil { */ public static void replaceField(FhirContext theFhirContext, String theFieldName, IBaseResource theFrom, IBaseResource theTo) { RuntimeResourceDefinition definition = theFhirContext.getResourceDefinition(theFrom); - if (definition == null) { - throw new IllegalArgumentException(String.format("Field %s does not exist in %s", theFieldName, theFrom)); - } + Validate.notNull(definition); replaceField(theFrom, theTo, theFhirContext.getResourceDefinition(theFrom).getChildByName(theFieldName)); } @@ -343,9 +338,7 @@ public final class TerserUtil { public static void clearField(FhirContext theFhirContext, String theFieldName, IBase theBase) { BaseRuntimeElementDefinition definition = theFhirContext.getElementDefinition(theBase.getClass()); BaseRuntimeChildDefinition childDefinition = definition.getChildByName(theFieldName); - if (childDefinition == null) { - throw new IllegalStateException(String.format("Field %s does not exist", theFieldName)); - } + Validate.notNull(childDefinition); childDefinition.getAccessor().getValues(theBase).clear(); } @@ -528,9 +521,7 @@ public final class TerserUtil { private static BaseRuntimeChildDefinition getBaseRuntimeChildDefinition(FhirContext theFhirContext, String theFieldName, IBaseResource theFrom) { RuntimeResourceDefinition definition = theFhirContext.getResourceDefinition(theFrom); BaseRuntimeChildDefinition childDefinition = definition.getChildByName(theFieldName); - if (childDefinition == null) { - throw new IllegalStateException(String.format("Field %s does not exist", theFieldName)); - } + Validate.notNull(childDefinition); return childDefinition; } @@ -593,9 +584,7 @@ public final class TerserUtil { */ public static T newElement(FhirContext theFhirContext, String theElementType, Object theConstructorParam) { BaseRuntimeElementDefinition def = theFhirContext.getElementDefinition(theElementType); - if (def == null) { - throw new IllegalArgumentException(String.format("Unable to find element type definition for %s", theElementType)); - } + Validate.notNull(def); return (T) def.newInstance(theConstructorParam); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/address/impl/LoquateAddressValidator.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/address/impl/LoquateAddressValidator.java index 303c26a2884..65ec8fb2207 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/address/impl/LoquateAddressValidator.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/address/impl/LoquateAddressValidator.java @@ -32,6 +32,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Validate; import org.apache.http.entity.ContentType; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseExtension; @@ -68,28 +69,19 @@ public class LoquateAddressValidator extends BaseRestfulValidator { public LoquateAddressValidator(Properties theProperties) { super(theProperties); - if (!theProperties.containsKey(PROPERTY_SERVICE_KEY)) { - if (!theProperties.containsKey(PROPERTY_SERVICE_ENDPOINT)) { - throw new IllegalArgumentException("Expected service key or custom service endpoint in the configuration, but got " + theProperties); - } - } + Validate.isTrue(theProperties.containsKey(PROPERTY_SERVICE_KEY) || !theProperties.containsKey(PROPERTY_SERVICE_ENDPOINT), + "Expected service key or custom service endpoint in the configuration, but got " + theProperties); } @Override protected AddressValidationResult getValidationResult(AddressValidationResult theResult, JsonNode response, FhirContext theFhirContext) { - if (!response.isArray() || response.size() < 1) { - throw new AddressValidationException("Invalid response - expected to get an array of validated addresses"); - } + Validate.isTrue(response.isArray() && response.size() >= 1, "Invalid response - expected to get an array of validated addresses"); JsonNode firstMatch = response.get(0); - if (!firstMatch.has("Matches")) { - throw new AddressValidationException("Invalid response - matches are unavailable"); - } + Validate.isTrue(firstMatch.has("Matches"), "Invalid response - matches are unavailable"); JsonNode matches = firstMatch.get("Matches"); - if (!matches.isArray()) { - throw new AddressValidationException("Invalid response - expected to get a validated match in the response"); - } + Validate.isTrue(matches.isArray(), "Invalid response - expected to get a validated match in the response"); JsonNode match = matches.get(0); return toAddressValidationResult(theResult, match, theFhirContext); diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/fields/FieldValidatingInterceptor.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/fields/FieldValidatingInterceptor.java index 7d5f596e258..0d4eb9e4a4b 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/fields/FieldValidatingInterceptor.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/validation/fields/FieldValidatingInterceptor.java @@ -115,7 +115,6 @@ public class FieldValidatingInterceptor { private void setValidationStatus(FhirContext ctx, IBase theBase, boolean isValid) { ExtensionUtil.clearExtensionsByUrl(theBase, getValidationExtensionUrl()); - IBaseExtension validationResultExtension = ExtensionUtil.addExtension(theBase, getValidationExtensionUrl()); ExtensionUtil.setExtension(ctx, theBase, getValidationExtensionUrl(), "boolean", !isValid); }