From 8f8c5c31f1c4971eb6fdf1f4879a46be5c739fd8 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Fri, 1 Jan 2021 14:29:10 -0500 Subject: [PATCH] Add validation rules interceptor (#2264) * Start work on validation rules interceptor * Work on interceptor * Add tests * Work on validation interceptor * Work on interceptor * Test fix * Documentation tweaks --- .../executor/InterceptorService.java | 3 +- .../BaseServerResponseException.java | 5 +- .../java/ca/uhn/fhir/util/BundleBuilder.java | 26 +- .../java/ca/uhn/fhir/util/ReflectionUtil.java | 61 +++- .../main/java/ca/uhn/fhir/util/UrlUtil.java | 25 ++ .../ca/uhn/fhir/i18n/hapi-messages.properties | 3 + .../executor/InterceptorServiceTest.java | 69 ++++ .../java/ca/uhn/fhir/util/UrlUtilTest.java | 10 + hapi-fhir-docs/pom.xml | 5 + ...amples.java => BundleBuilderExamples.java} | 6 +- ...positoryValidatingInterceptorExamples.java | 103 ++++++ .../2264-allow-protected-hook-methods.yaml | 5 + .../5_3_0/2264-rename-transactionbuiler.yaml | 5 + ...264-repository-validation-interceptor.yaml | 7 + .../ca/uhn/hapi/fhir/docs/files.properties | 3 +- .../built_in_server_interceptors.md | 9 + ...ansaction_builder.md => bundle_builder.md} | 14 +- .../repository_validating_interceptor.md | 85 +++++ .../ca/uhn/fhir/jpa/config/BaseConfig.java | 8 + .../fhir/jpa/config/BaseConfigDstu3Plus.java | 15 +- .../fhir/jpa/dao/BaseHapiFhirResourceDao.java | 81 +++-- .../interceptor/validation/BaseTypedRule.java | 49 +++ .../validation/IRepositoryValidatingRule.java | 88 +++++ .../jpa/interceptor/validation/IRuleRoot.java | 39 +++ .../RepositoryValidatingInterceptor.java | 130 +++++++ .../RepositoryValidatingRuleBuilder.java | 211 ++++++++++++ .../validation/RequireValidationRule.java | 71 ++++ .../validation/RuleDisallowProfile.java | 60 ++++ .../RuleRequireProfileDeclaration.java | 56 ++++ .../validation/JpaFhirInstanceValidator.java | 114 ------- .../validation/ValidatorResourceFetcher.java | 111 ++++++ ...RepositoryValidatingInterceptorR4Test.java | 316 ++++++++++++++++++ ...scriptionDeliveringRestHookSubscriber.java | 2 +- .../RequestValidatingInterceptor.java | 69 ++-- .../ca/uhn/fhir/util/BundleBuilderTest.java | 4 +- .../validator/FhirInstanceValidator.java | 61 +--- .../VersionSpecificWorkerContextWrapper.java | 58 ++++ 37 files changed, 1702 insertions(+), 285 deletions(-) rename hapi-fhir-docs/src/main/java/ca/uhn/hapi/fhir/docs/{TransactionBuilderExamples.java => BundleBuilderExamples.java} (96%) create mode 100644 hapi-fhir-docs/src/main/java/ca/uhn/hapi/fhir/docs/RepositoryValidatingInterceptorExamples.java create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2264-allow-protected-hook-methods.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2264-rename-transactionbuiler.yaml create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2264-repository-validation-interceptor.yaml rename hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/model/{transaction_builder.md => bundle_builder.md} (60%) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/docs/validation/repository_validating_interceptor.md create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/BaseTypedRule.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/IRepositoryValidatingRule.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/IRuleRoot.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptor.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingRuleBuilder.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RequireValidationRule.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RuleDisallowProfile.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/interceptor/validation/RuleRequireProfileDeclaration.java delete mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/validation/JpaFhirInstanceValidator.java create mode 100644 hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/validation/ValidatorResourceFetcher.java create mode 100644 hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/interceptor/validation/RepositoryValidatingInterceptorR4Test.java diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java index a6ff71a222b..c61e1f54560 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/interceptor/executor/InterceptorService.java @@ -28,6 +28,7 @@ import ca.uhn.fhir.interceptor.api.IInterceptorService; import ca.uhn.fhir.interceptor.api.Interceptor; import ca.uhn.fhir.interceptor.api.Pointcut; import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; +import ca.uhn.fhir.util.ReflectionUtil; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ListMultimap; @@ -567,7 +568,7 @@ public class InterceptorService implements IInterceptorService, IInterceptorBroa */ private static List scanInterceptorForHookMethods(Object theInterceptor, int theTypeOrder) { ArrayList retVal = new ArrayList<>(); - for (Method nextMethod : theInterceptor.getClass().getMethods()) { + for (Method nextMethod : ReflectionUtil.getDeclaredMethods(theInterceptor.getClass(), true)) { Optional hook = findAnnotation(nextMethod, Hook.class); if (hook.isPresent()) { diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/BaseServerResponseException.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/BaseServerResponseException.java index 0c46602e57a..0e0ace20367 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/BaseServerResponseException.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/server/exceptions/BaseServerResponseException.java @@ -78,7 +78,10 @@ public abstract class BaseServerResponseException extends RuntimeException { * @param theStatusCode The HTTP status code corresponding to this problem * @param theMessage The message */ - public BaseServerResponseException(int theStatusCode, String theMessage) { + public /** + * Interceptor hook method. This method should not be called directly. + */ + BaseServerResponseException(int theStatusCode, String theMessage) { super(theMessage); myStatusCode = theStatusCode; myBaseOperationOutcome = null; diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java index 402f421a370..eee8ec9047f 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/BundleBuilder.java @@ -31,6 +31,8 @@ import org.hl7.fhir.instance.model.api.IBaseBundle; import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; +import java.util.Objects; + /** * This class can be used to build a Bundle resource to be used as a FHIR transaction. Convenience methods provide * support for setting various bundle fields and working with bundle parts such as metadata and entry @@ -72,8 +74,6 @@ public class BundleBuilder { myBundleDef = myContext.getResourceDefinition("Bundle"); myBundle = (IBaseBundle) myBundleDef.newInstance(); - setBundleField("type", "transaction"); - myEntryChild = myBundleDef.getChildByName("entry"); myEntryDef = myEntryChild.getChildByName("entry"); @@ -144,11 +144,14 @@ public class BundleBuilder { } /** - * Adds an entry containing an update (PUT) request + * Adds an entry containing an update (PUT) request. + * Also sets the Bundle.type value to "transaction" if it is not already set. * * @param theResource The resource to update */ - public UpdateBuilder addUpdateEntry(IBaseResource theResource) { + public UpdateBuilder addTransactionUpdateEntry(IBaseResource theResource) { + setBundleField("type", "transaction"); + IBase request = addEntryAndReturnRequest(theResource); // Bundle.entry.request.url @@ -165,11 +168,14 @@ public class BundleBuilder { } /** - * Adds an entry containing an create (POST) request + * Adds an entry containing an create (POST) request. + * Also sets the Bundle.type value to "transaction" if it is not already set. * * @param theResource The resource to create */ public CreateBuilder addCreateEntry(IBaseResource theResource) { + setBundleField("type", "transaction"); + IBase request = addEntryAndReturnRequest(theResource); String resourceType = myContext.getResourceType(theResource); @@ -212,6 +218,11 @@ public class BundleBuilder { return (IBaseBackboneElement) searchInstance; } + /** + * + * @param theResource + * @return + */ public IBase addEntryAndReturnRequest(IBaseResource theResource) { Validate.notNull(theResource, "theResource must not be null"); @@ -310,7 +321,7 @@ public class BundleBuilder { return retVal; } - public class UpdateBuilder { + public static class UpdateBuilder { private final IPrimitiveType myUrl; @@ -338,7 +349,8 @@ public class BundleBuilder { * Make this create a Conditional Create */ public void conditional(String theConditionalUrl) { - IPrimitiveType ifNoneExist = (IPrimitiveType) myContext.getElementDefinition("string").newInstance(); + BaseRuntimeElementDefinition stringDefinition = Objects.requireNonNull(myContext.getElementDefinition("string")); + IPrimitiveType ifNoneExist = (IPrimitiveType) stringDefinition.newInstance(); ifNoneExist.setValueAsString(theConditionalUrl); myEntryRequestIfNoneExistChild.getMutator().setValue(myRequest, ifNoneExist); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java index df364b9f143..26e5532ab3a 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/ReflectionUtil.java @@ -32,7 +32,9 @@ import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.lang.reflect.WildcardType; import java.util.ArrayList; +import java.util.Arrays; import java.util.Comparator; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; @@ -45,25 +47,64 @@ public class ReflectionUtil { private static final ConcurrentHashMap ourFhirServerVersions = new ConcurrentHashMap<>(); private static final org.slf4j.Logger ourLog = org.slf4j.LoggerFactory.getLogger(ReflectionUtil.class); + /** + * Non instantiable + */ + private ReflectionUtil() { + super(); + } + + /** + * Returns all methods declared against {@literal theClazz}. This method returns a predictable order, which is + * sorted by method name and then by parameters. + *

+ * This method does not include superclass methods (see {@link #getDeclaredMethods(Class, boolean)} if you + * want to include those. + *

+ */ + public static List getDeclaredMethods(Class theClazz) { + return getDeclaredMethods(theClazz, false); + } + /** * Returns all methods declared against {@literal theClazz}. This method returns a predictable order, which is * sorted by method name and then by parameters. */ - public static List getDeclaredMethods(Class theClazz) { - HashSet foundMethods = new LinkedHashSet<>(); + public static List getDeclaredMethods(Class theClazz, boolean theIncludeMethodsFromSuperclasses) { + HashMap foundMethods = new HashMap<>(); + + populateDeclaredMethodsMap(theClazz, foundMethods, theIncludeMethodsFromSuperclasses); + + List retVal = new ArrayList<>(foundMethods.values()); + retVal.sort((Comparator.comparing(ReflectionUtil::describeMethodInSortFriendlyWay))); + return retVal; + } + + private static void populateDeclaredMethodsMap(Class theClazz, HashMap foundMethods, boolean theIncludeMethodsFromSuperclasses) { Method[] declaredMethods = theClazz.getDeclaredMethods(); for (Method next : declaredMethods) { - try { - Method method = theClazz.getMethod(next.getName(), next.getParameterTypes()); - foundMethods.add(method); - } catch (NoSuchMethodException | SecurityException e) { - foundMethods.add(next); + + if (Modifier.isAbstract(next.getModifiers()) || + Modifier.isStatic(next.getModifiers()) || + Modifier.isPrivate(next.getModifiers())) { + continue; + } + + String description = next.getName() + Arrays.asList(next.getParameterTypes()); + + if (!foundMethods.containsKey(description)) { + try { + Method method = theClazz.getMethod(next.getName(), next.getParameterTypes()); + foundMethods.put(description, method); + } catch (NoSuchMethodException | SecurityException e) { + foundMethods.put(description, next); + } } } - List retVal = new ArrayList<>(foundMethods); - retVal.sort((Comparator.comparing(ReflectionUtil::describeMethodInSortFriendlyWay))); - return retVal; + if (theIncludeMethodsFromSuperclasses && !theClazz.getSuperclass().equals(Object.class)) { + populateDeclaredMethodsMap(theClazz.getSuperclass(), foundMethods, theIncludeMethodsFromSuperclasses); + } } /** diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java index b31d27d4f95..8f7031365e8 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/UrlUtil.java @@ -8,6 +8,7 @@ import ca.uhn.fhir.rest.api.Constants; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import com.google.common.escape.Escaper; import com.google.common.net.PercentEscaper; +import org.apache.commons.lang3.StringUtils; import org.apache.http.NameValuePair; import org.apache.http.client.utils.URLEncodedUtils; import org.apache.http.message.BasicNameValuePair; @@ -16,6 +17,8 @@ import org.hl7.fhir.instance.model.api.IPrimitiveType; import javax.annotation.Nonnull; import java.io.UnsupportedEncodingException; import java.net.MalformedURLException; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLDecoder; import java.util.ArrayList; @@ -29,6 +32,7 @@ import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.defaultIfBlank; import static org.apache.commons.lang3.StringUtils.defaultString; +import static org.apache.commons.lang3.StringUtils.endsWith; import static org.apache.commons.lang3.StringUtils.isBlank; /* @@ -300,6 +304,27 @@ public class UrlUtil { return toQueryStringMap(map); } + /** + * Normalizes canonical URLs for comparison. Trailing "/" is stripped, + * and any version identifiers or fragment hash is removed + */ + public static String normalizeCanonicalUrlForComparison(String theUrl) { + String retVal; + try { + retVal = new URI(theUrl).normalize().toString(); + } catch (URISyntaxException e) { + retVal = theUrl; + } + while (endsWith(retVal, "/")) { + retVal = retVal.substring(0, retVal.length() - 1); + } + int hashOrPipeIndex = StringUtils.indexOfAny(retVal, '#', '|'); + if (hashOrPipeIndex != -1) { + retVal = retVal.substring(0, hashOrPipeIndex); + } + return retVal; + } + /** * Parse a URL in one of the following forms: *