From d1e3436cea6c98dccf57049c19b49a7e0f084403 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 3 Feb 2022 19:04:44 +0100 Subject: [PATCH 01/36] Original idea as is during our discussion with Ritesh. --- .../om/request/validation/ContextAware.java | 6 +++ .../validation/RequestFeatureValidator.java | 6 +++ .../validation/ResponseFeatureValidator.java | 9 ++++ .../request/validation/ValidationContext.java | 13 +++++ .../validation/ValidationDescriptor.java | 17 +++++++ .../request/validation/ValidatorRegistry.java | 50 +++++++++++++++++++ .../om/request/validation/ValidatorType.java | 8 +++ 7 files changed, 109 insertions(+) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java new file mode 100644 index 000000000000..23d954b3288e --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java @@ -0,0 +1,6 @@ +package org.apache.hadoop.ozone.om.request.validation; + +public interface ContextAware { + + void setContext(ValidationContext context); +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java new file mode 100644 index 000000000000..2f9da94bd5b1 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java @@ -0,0 +1,6 @@ +package org.apache.hadoop.ozone.om.request.validation; + +public interface RequestFeatureValidator { + + REQTYPE rejectOrAdjust(REQTYPE request) throws Exception; +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java new file mode 100644 index 000000000000..a0e7a9f3b8aa --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java @@ -0,0 +1,9 @@ +package org.apache.hadoop.ozone.om.request.validation; + +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; + +public interface ResponseFeatureValidator { + + OzoneManagerProtocolProtos.OMResponse validate( + OzoneManagerProtocolProtos.OMRequest request, OzoneManagerProtocolProtos.OMResponse response) throws Exception; +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java new file mode 100644 index 000000000000..24bc29351f6e --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java @@ -0,0 +1,13 @@ +package org.apache.hadoop.ozone.om.request.validation; + +import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; +import org.apache.hadoop.util.ComparableVersion; + +public interface ValidationContext { + + LayoutVersionManager versionManager(); + + ComparableVersion serverVersion(); + + int clientVersion(); +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java new file mode 100644 index 000000000000..73256c7033ee --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java @@ -0,0 +1,17 @@ +package org.apache.hadoop.ozone.om.request.validation; + +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface ValidationDescriptor { + + ValidatorType type() default ValidatorType.GENERIC_VALIDATOR; + + Type requestType(); +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java new file mode 100644 index 000000000000..3d75eba0d568 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -0,0 +1,50 @@ +package org.apache.hadoop.ozone.om.request.validation; + +import org.reflections.Reflections; +import org.reflections.scanners.SubTypesScanner; +import org.reflections.scanners.TypeAnnotationsScanner; +import org.reflections.util.ClasspathHelper; +import org.reflections.util.ConfigurationBuilder; + +import java.util.List; +import java.util.Set; + +public class ValidatorRegistry { + + private List> preFinalizationRequestValidators; + private List> oldClientRequestValidators; + private List> newClientRequestValidators; + private List> genericRequestValidators; + + private List> preFinalizationResponseValidators; + private List> oldClientResponseValidators; + private List> newClientResponseValidators; + private List> genericResponseValidators; + + public ValidatorRegistry(String validatorPackage) { + initMaps(validatorPackage); + } + + private void initMaps(String validatorPackage) { + Reflections reflections = new Reflections(new ConfigurationBuilder() + .setUrls(ClasspathHelper.forPackage(validatorPackage)) + .setScanners(new TypeAnnotationsScanner(), new SubTypesScanner()) + .setExpandSuperTypes(false) + .useParallelExecutor() + ); + Set> describedValidators = + reflections.getTypesAnnotatedWith(ValidationDescriptor.class); + + initRequestValidators(); + initResponseValidators(); + } + + private void initRequestValidators() { + + } + + private void initResponseValidators() { + + } + +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java new file mode 100644 index 000000000000..ee1541ea4397 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java @@ -0,0 +1,8 @@ +package org.apache.hadoop.ozone.om.request.validation; + +public enum ValidatorType { + PRE_FINALIZATION_VALIDATOR, + OLDER_CLIENT_REQUESTS_VALIDATOR, + NEWER_CLIENT_REQUESTS_VALIDATOR, + GENERIC_VALIDATOR +} From 8c29ad1a6037ba3ec402014be49de8fa94113aa3 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 4 Feb 2022 01:22:53 +0100 Subject: [PATCH 02/36] Switch to the annotating validator methods instead of interfaces and such, add initial structure to the registry, and initial lookup, and execution of validator methods. --- .../om/request/validation/ContextAware.java | 6 - .../validation/RequestFeatureValidator.java | 20 ++- .../validation/RequestProcessingPhase.java | 6 + .../validation/ResponseFeatureValidator.java | 9 -- .../validation/ValidationCondition.java | 8 ++ .../request/validation/ValidationContext.java | 25 +++- .../validation/ValidationDescriptor.java | 17 --- .../request/validation/ValidatorRegistry.java | 127 +++++++++++++++--- .../om/request/validation/ValidatorType.java | 8 -- ...ManagerProtocolServerSideTranslatorPB.java | 107 ++++++++++++++- 10 files changed, 270 insertions(+), 63 deletions(-) delete mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java delete mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java delete mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java delete mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java deleted file mode 100644 index 23d954b3288e..000000000000 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ContextAware.java +++ /dev/null @@ -1,6 +0,0 @@ -package org.apache.hadoop.ozone.om.request.validation; - -public interface ContextAware { - - void setContext(ValidationContext context); -} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java index 2f9da94bd5b1..9e0c01ff565d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java @@ -1,6 +1,22 @@ package org.apache.hadoop.ozone.om.request.validation; -public interface RequestFeatureValidator { +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.METHOD) +public @interface RequestFeatureValidator { + + ValidationCondition[] conditions(); + + RequestProcessingPhase processingPhase(); + + Type requestType(); + + boolean contextAware() default false; - REQTYPE rejectOrAdjust(REQTYPE request) throws Exception; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java new file mode 100644 index 000000000000..81a530189e87 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java @@ -0,0 +1,6 @@ +package org.apache.hadoop.ozone.om.request.validation; + +public enum RequestProcessingPhase { + PRE_PROCESS, + POST_PROCESS +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java deleted file mode 100644 index a0e7a9f3b8aa..000000000000 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ResponseFeatureValidator.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.apache.hadoop.ozone.om.request.validation; - -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; - -public interface ResponseFeatureValidator { - - OzoneManagerProtocolProtos.OMResponse validate( - OzoneManagerProtocolProtos.OMRequest request, OzoneManagerProtocolProtos.OMResponse response) throws Exception; -} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java new file mode 100644 index 000000000000..cd722bfc8058 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -0,0 +1,8 @@ +package org.apache.hadoop.ozone.om.request.validation; + +public enum ValidationCondition { + CLUSTER_IS_PRE_FINALIZED, + OLDER_CLIENT_REQUESTS, + NEWER_CLIENT_REQUESTS, + UNCONDITIONAL; +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java index 24bc29351f6e..d2058d4abea3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java @@ -1,13 +1,34 @@ package org.apache.hadoop.ozone.om.request.validation; import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; -import org.apache.hadoop.util.ComparableVersion; public interface ValidationContext { LayoutVersionManager versionManager(); - ComparableVersion serverVersion(); + int serverVersion(); int clientVersion(); + + static ValidationContext of( + LayoutVersionManager versionManager, + int serverVersion, + int clientVersion) { + return new ValidationContext() { + @Override + public LayoutVersionManager versionManager() { + return versionManager; + } + + @Override + public int serverVersion() { + return serverVersion; + } + + @Override + public int clientVersion() { + return clientVersion; + } + }; + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java deleted file mode 100644 index 73256c7033ee..000000000000 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationDescriptor.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.apache.hadoop.ozone.om.request.validation; - -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) -public @interface ValidationDescriptor { - - ValidatorType type() default ValidatorType.GENERIC_VALIDATOR; - - Type requestType(); -} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index 3d75eba0d568..8aae163851ac 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -1,50 +1,141 @@ package org.apache.hadoop.ozone.om.request.validation; +import org.apache.commons.lang3.tuple.Pair; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; +import org.jetbrains.annotations.NotNull; import org.reflections.Reflections; -import org.reflections.scanners.SubTypesScanner; -import org.reflections.scanners.TypeAnnotationsScanner; +import org.reflections.scanners.MethodAnnotationsScanner; import org.reflections.util.ClasspathHelper; import org.reflections.util.ConfigurationBuilder; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumMap; +import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Set; public class ValidatorRegistry { - private List> preFinalizationRequestValidators; - private List> oldClientRequestValidators; - private List> newClientRequestValidators; - private List> genericRequestValidators; + EnumMap, List>>> + validators = new EnumMap<>(ValidationCondition.class); - private List> preFinalizationResponseValidators; - private List> oldClientResponseValidators; - private List> newClientResponseValidators; - private List> genericResponseValidators; + public static void main(String[] args) { + new ValidatorRegistry("org.apache.hadoop.ozone"); + } public ValidatorRegistry(String validatorPackage) { initMaps(validatorPackage); + System.out.println(validators.entrySet()); + } + + public List validationsFor( + List conditions, + Type requestType, + RequestProcessingPhase phase) { + + if (conditions.isEmpty() || validators.isEmpty()) { + return Collections.emptyList(); + } + + List returnValue = + validationsFor(conditions.get(0), requestType, phase); + + for (int i=1; i < conditions.size(); i++) { + returnValue.addAll(validationsFor(conditions.get(i), requestType, phase)); + } + return returnValue; + } + + private List validationsFor( + ValidationCondition condition, + Type requestType, + RequestProcessingPhase phase) { + + List returnValue = new LinkedList<>(); + + EnumMap, List>> requestTypeMap = + validators.get(condition); + if (requestTypeMap == null || requestTypeMap.isEmpty()) { + returnValue = Collections.emptyList(); + } + + Pair, List> phases = requestTypeMap.get(requestType); + if (phases == null || + (phases.getLeft().isEmpty() && phases.getRight().isEmpty())) { + returnValue = Collections.emptyList(); + } + + if (phase.equals(RequestProcessingPhase.PRE_PROCESS)) { + returnValue = phases.getLeft(); + } else if (phase.equals(RequestProcessingPhase.POST_PROCESS)) { + returnValue = phases.getRight(); + } + return returnValue; } private void initMaps(String validatorPackage) { Reflections reflections = new Reflections(new ConfigurationBuilder() .setUrls(ClasspathHelper.forPackage(validatorPackage)) - .setScanners(new TypeAnnotationsScanner(), new SubTypesScanner()) - .setExpandSuperTypes(false) + .setScanners(new MethodAnnotationsScanner()) .useParallelExecutor() ); - Set> describedValidators = - reflections.getTypesAnnotatedWith(ValidationDescriptor.class); - initRequestValidators(); - initResponseValidators(); + Set describedValidators = + reflections.getMethodsAnnotatedWith(RequestFeatureValidator.class); + + for (Method m : describedValidators) { + RequestFeatureValidator descriptor = + m.getAnnotation(RequestFeatureValidator.class); + + for (ValidationCondition condition : descriptor.conditions()) { + EnumMap, List>> requestTypeMap = + getAndInitialize(condition, newTypeMap(), validators); + Pair, List> phases = getAndInitialize( + descriptor.requestType(), newListPair(), requestTypeMap); + List validationMethods = null; + if (isPreProcessValidator(descriptor)) { + validationMethods = phases.getLeft(); + } else if (isPostProcessValidator(descriptor)) { + validationMethods = phases.getRight(); + } + validationMethods.add(m); + } + } } - private void initRequestValidators() { + @NotNull + private EnumMap, List>> newTypeMap() { + return new EnumMap<>(Type.class); + } + + V getAndInitialize(K key, V defaultValue, Map from) { + if (defaultValue == null) { + throw new NullPointerException( + "Entry can not be initialized with null value."); + } + V inMapValue = from.get(key); + if (inMapValue == null || !from.containsKey(key)) { + from.put(key, defaultValue); + return defaultValue; + } + return inMapValue; + } + private boolean isPreProcessValidator(RequestFeatureValidator descriptor) { + return descriptor.processingPhase() + .equals(RequestProcessingPhase.PRE_PROCESS); } - private void initResponseValidators() { + private boolean isPostProcessValidator(RequestFeatureValidator descriptor) { + return descriptor.processingPhase() + .equals(RequestProcessingPhase.POST_PROCESS); + } + private Pair, List> newListPair() { + return Pair.of(new ArrayList<>(), new ArrayList<>()); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java deleted file mode 100644 index ee1541ea4397..000000000000 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorType.java +++ /dev/null @@ -1,8 +0,0 @@ -package org.apache.hadoop.ozone.om.request.validation; - -public enum ValidatorType { - PRE_FINALIZATION_VALIDATOR, - OLDER_CLIENT_REQUESTS_VALIDATOR, - NEWER_CLIENT_REQUESTS_VALIDATOR, - GENERIC_VALIDATOR -} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index ef8206937ec8..4d6f75099fc3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -22,6 +22,10 @@ import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.PrepareStatus; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.LinkedList; +import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicLong; @@ -38,6 +42,11 @@ import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer.RaftServerStatus; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; import org.apache.hadoop.ozone.om.request.OMClientRequest; +import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; +import org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase; +import org.apache.hadoop.ozone.om.request.validation.ValidationCondition; +import org.apache.hadoop.ozone.om.request.validation.ValidationContext; +import org.apache.hadoop.ozone.om.request.validation.ValidatorRegistry; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -60,6 +69,9 @@ public class OzoneManagerProtocolServerSideTranslatorPB implements OzoneManagerProtocolPB { private static final Logger LOG = LoggerFactory .getLogger(OzoneManagerProtocolServerSideTranslatorPB.class); + private static final String OM_REQUESTS_PACKAGE = + "org.apache.hadoop.ozone"; + private final OzoneManagerRatisServer omRatisServer; private final RequestHandler handler; private final boolean isRatisEnabled; @@ -68,6 +80,7 @@ public class OzoneManagerProtocolServerSideTranslatorPB implements private final AtomicLong transactionIndex; private final OzoneProtocolMessageDispatcher dispatcher; + private final ValidatorRegistry validatorRegistry; /** * Constructs an instance of the server handler. @@ -110,6 +123,8 @@ public OzoneManagerProtocolServerSideTranslatorPB( this.omRatisServer = ratisServer; dispatcher = new OzoneProtocolMessageDispatcher<>("OzoneProtocol", metrics, LOG, OMPBHelper::processForDebug, OMPBHelper::processForDebug); + // TODO: make this injectable for testing... + validatorRegistry = new ValidatorRegistry(OM_REQUESTS_PACKAGE); } /** @@ -120,9 +135,99 @@ public OzoneManagerProtocolServerSideTranslatorPB( @Override public OMResponse submitRequest(RpcController controller, OMRequest request) throws ServiceException { + List conditions = getConditions(request); + ValidationContext context = ValidationContext.of( + ozoneManager.getVersionManager(), 0, request.getVersion()); - return dispatcher.processRequest(request, this::processRequest, + OMRequest validatedRequest = preValidate(conditions, context, request); + + OMResponse response = + dispatcher.processRequest(validatedRequest, this::processRequest, request.getCmdType(), request.getTraceID()); + + return postValidate(conditions, context, request, response); + } + + private OMRequest preValidate( + List conditions, + ValidationContext context, + OMRequest originalRequest) throws ServiceException { + + List validations = validatorRegistry.validationsFor( + conditions, + originalRequest.getCmdType(), + RequestProcessingPhase.PRE_PROCESS); + + + OMRequest validatedRequest = originalRequest.toBuilder().build(); + for (Method method : validations) { + // TODO: restrict the parameter list and return type of such annotated + // methods either at compile time, or via a test + try { + if (method + .getAnnotation(RequestFeatureValidator.class).contextAware()) { + validatedRequest = + (OMRequest) method.invoke(null, validatedRequest, context); + } else { + validatedRequest = + (OMRequest) method.invoke(null, validatedRequest); + } + } catch (IllegalAccessException | InvocationTargetException e) { + throw new ServiceException(e); + } + } + return validatedRequest; + } + + private OMResponse postValidate( + List conditions, + ValidationContext context, + OMRequest originalRequest, + OMResponse originalResponse) throws ServiceException { + + List validations = validatorRegistry.validationsFor( + conditions, + originalRequest.getCmdType(), + RequestProcessingPhase.POST_PROCESS); + + OMResponse validatedResponse = originalResponse.toBuilder().build(); + for (Method method : validations) { + // TODO: restrict the parameter list and return type of such annotated + // methods either at compile time, or via a test + try { + if (method.getAnnotation(RequestFeatureValidator.class).contextAware()) { + validatedResponse = (OMResponse) method + .invoke(null, originalRequest, validatedResponse, context); + } else { + validatedResponse = (OMResponse) method + .invoke(null, originalRequest, validatedResponse); + } + } catch (IllegalAccessException | InvocationTargetException e) { + throw new ServiceException(e); + } + } + return validatedResponse; + } + + private List getConditions(OMRequest request) { + List conditions = new LinkedList<>(); + conditions.add(ValidationCondition.UNCONDITIONAL); + int serverProtocolVersion = getServerProtocolVersion(); + int clientProtocolVersion = request.getVersion(); + if (serverProtocolVersion < clientProtocolVersion) { + conditions.add(ValidationCondition.NEWER_CLIENT_REQUESTS); + } else if (serverProtocolVersion > clientProtocolVersion) { + conditions.add(ValidationCondition.OLDER_CLIENT_REQUESTS); + } + if (ozoneManager.getVersionManager().needsFinalization()) { + conditions.add(ValidationCondition.CLUSTER_IS_PRE_FINALIZED); + } + return conditions; + } + + //TODO fidn out versioning and where it comes from... + private int getServerProtocolVersion() { + return 0; } private OMResponse processRequest(OMRequest request) throws From 6d400d3c1f26235f6e5a8cb527e5733648bf90c4 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 4 Feb 2022 14:14:19 +0100 Subject: [PATCH 03/36] Added some TODO and Javadoc. Fixed findbugs, and started to eliminate checkstyle and rat warnings. --- .../validation/RequestFeatureValidator.java | 85 +++++++++++++++++++ .../validation/RequestProcessingPhase.java | 22 +++++ .../validation/ValidationCondition.java | 16 ++++ .../request/validation/ValidationContext.java | 16 ++++ .../request/validation/ValidatorRegistry.java | 28 ++++-- .../om/request/validation/package-info.java | 62 ++++++++++++++ ...ManagerProtocolServerSideTranslatorPB.java | 5 +- 7 files changed, 226 insertions(+), 8 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/package-info.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java index 9e0c01ff565d..d4427294188c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.hadoop.ozone.om.request.validation; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; @@ -7,16 +23,85 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +/** + * An annotation to mark methods that does certain request validations. + * + * The methods annotated with this annotation are collected by the + * {@link ValidatorRegistry} class during the initialization of the server. + * + * The conditions specify the specific use case in which the validator should be + * applied to the request. See {@link ValidationCondition} for more details + * on the specific conditions. + * The validator method should be applied to just one specific request type + * to help keep these methods simple and straightforward. If you want to use + * the same validation for different request types, use inheritance, and + * annotate the override method that just calls super. + * Note that the aim is to have these validators together with the request + * processing code, so the handling of these specific situations are easy to + * find. + * + * The annotated methods have to have a fixed signature. + * A {@link RequestProcessingPhase#PRE_PROCESS} phase method is running before + * the request is processed by the regular code. + * Its signature has to be the following: + * - it has to be static and idempotent + * - it has to have one or two parameters + * - if not contextAware, the only parameter it should have is an + * {@link + * org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest} + * - if contextAware, a second parameter of type {@link ValidationContext} has + * to be there in the argument list. + * - the method has to return the modified request, or throw a ServiceException + * in case the request is considered to be invalid + * - the method does not need to care about preserving the request it gets, + * the original request is captured and saved by the calling environment. + * + * A {@link RequestProcessingPhase#POST_PROCESS} phase method is running once + * the + * {@link + * org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse} + * is calculated for a given request. + * Its signature has to be the following: + * - it has to be static and idempotent + * - it has 2 or 3 parameters + * - similalry to the pre-processing validators, first parameter is the + * OMRequest, the second parameter is the OMResponse, and the third optional + * parameter is a ValidationContext if the method marked to be context aware. + * - the method has to return the modified response of throw a ServiceException + * if the request is considered invalid based on the response. + * - the method gets the request object that was supplied for the general + * request processing code, not the original request, while it gets a copy + * of the original response object provided by the general request processing + * code. + */ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.METHOD) public @interface RequestFeatureValidator { + /** + * Runtime conditions in which a validator should run. + * @return a list of conditions when the validator should be applied + */ ValidationCondition[] conditions(); + /** + * Defines if the validation has to run before or after the general request + * processing. + * @return if this is a pre or post processing validator + */ RequestProcessingPhase processingPhase(); + /** + * The type of the request handled by this validator method. + * @return the requestType to whihc the validator shoudl be applied + */ Type requestType(); + /** + * Tells whether the validator requires a {@link ValidationContext} or it + * does not. + * @return if the method requires the context. + */ boolean contextAware() default false; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java index 81a530189e87..672156842914 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestProcessingPhase.java @@ -1,5 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.hadoop.ozone.om.request.validation; +/** + * Processing phase defines when a request validator should run. + * + * There are two hooking point at the moment, before and after the generic + * request processing code. + */ public enum RequestProcessingPhase { PRE_PROCESS, POST_PROCESS diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java index cd722bfc8058..fb0420b4fdec 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.hadoop.ozone.om.request.validation; public enum ValidationCondition { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java index d2058d4abea3..bf9a3c1523a9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.hadoop.ozone.om.request.validation; import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index 8aae163851ac..52a34feaa17d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.hadoop.ozone.om.request.validation; import org.apache.commons.lang3.tuple.Pair; @@ -54,20 +70,20 @@ private List validationsFor( Type requestType, RequestProcessingPhase phase) { - List returnValue = new LinkedList<>(); EnumMap, List>> requestTypeMap = validators.get(condition); if (requestTypeMap == null || requestTypeMap.isEmpty()) { - returnValue = Collections.emptyList(); + return Collections.emptyList(); } Pair, List> phases = requestTypeMap.get(requestType); if (phases == null || (phases.getLeft().isEmpty() && phases.getRight().isEmpty())) { - returnValue = Collections.emptyList(); + return Collections.emptyList(); } + List returnValue = new LinkedList<>(); if (phase.equals(RequestProcessingPhase.PRE_PROCESS)) { returnValue = phases.getLeft(); } else if (phase.equals(RequestProcessingPhase.POST_PROCESS)) { @@ -95,13 +111,11 @@ private void initMaps(String validatorPackage) { getAndInitialize(condition, newTypeMap(), validators); Pair, List> phases = getAndInitialize( descriptor.requestType(), newListPair(), requestTypeMap); - List validationMethods = null; if (isPreProcessValidator(descriptor)) { - validationMethods = phases.getLeft(); + phases.getLeft().add(m); } else if (isPostProcessValidator(descriptor)) { - validationMethods = phases.getRight(); + phases.getRight().add(m); } - validationMethods.add(m); } } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/package-info.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/package-info.java new file mode 100644 index 000000000000..e82bab7307a0 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/package-info.java @@ -0,0 +1,62 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +/** + * Request's feature validation handling. + * + * This package holds facilities to add new situation specific behaviour to + * request handling without cluttering the basic logic of the request handler + * code. + * + * Typical use case scenarios, that we had in mind during the design: + * - during an upgrade, in the pre-finalized state certain request types are + * to be rejected based on provided properties of the request not based on the + * request type + * - a client connects to the server but uses an older version of the protocol + * - a client connects to the server but uses a newer version of the protocol + * - the code can handle certain checks that have to run all the time, but at + * first we do not see a general use case that we would pull in immediately. + * These are the current + * {@link org.apache.hadoop.ozone.om.request.validation.ValidationCondition}s + * but this list might be extended later on if we see other use cases. + * + * The system uses a reflection based discovery to find methods that are + * annotated with the + * {@link org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator} + * annotation. + * This annotation is used to specify the condition in which a certain validator + * has to be used, the request type to which the validation should be applied, + * and the request processing phase in which we apply the validation. + * + * One validator can be applied in multiple + * {@link org.apache.hadoop.ozone.om.request.validation.ValidationCondition} + * but a validator has to handle strictly just one + * {@link org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type + * }. + * The main reason to avoid validating multiple request types with the same + * validator, is that these validators have to be simple methods without state + * any complex validation has to happen in the reql request handling. + * In these validators we need to ensure that in the given condition the request + * is rejected with a proper message, or rewritten to the proper format if for + * example we want to handle an old request with a new server, but we need some + * additional values set to something default, while in the meantime we want to + * add meaning to a null value from newer clients. + * + * In general, it is a good practice to have the request handling code, and the + * validations tied together in one class. + */ +package org.apache.hadoop.ozone.om.request.validation; \ No newline at end of file diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 4d6f75099fc3..f7e1bb4b5822 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -145,9 +145,12 @@ public OMResponse submitRequest(RpcController controller, dispatcher.processRequest(validatedRequest, this::processRequest, request.getCmdType(), request.getTraceID()); - return postValidate(conditions, context, request, response); + return postValidate(conditions, context, validatedRequest, response); } + //TODO: move this code out from here, and just call from a validation util + // class or instead of gettign the validation from the registry we + // might even use that to run the validations... private OMRequest preValidate( List conditions, ValidationContext context, From dddbf3845411a52ddf987570ba6005e7999b3539 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 4 Feb 2022 16:22:20 +0100 Subject: [PATCH 04/36] Add remaining API docs --- .../validation/ValidationCondition.java | 26 ++++++++++ .../request/validation/ValidationContext.java | 26 ++++++++++ .../request/validation/ValidatorRegistry.java | 47 +++++++++++++++++-- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java index fb0420b4fdec..49ab6ee6056d 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -16,9 +16,35 @@ */ package org.apache.hadoop.ozone.om.request.validation; +/** + * Defines conditions for which validators can be assigned to. + * + * These conditions describe a situation where special request handling might + * be necessary. In these cases we do not override the actual request handling + * code, but based on certain request properties we might reject a request + * early, or we might modify the request, or the response received/sent from/to + * the client. + */ public enum ValidationCondition { + /** + * Classifies validations that has to run after an upgrade until the cluster + * is in a pre-finalized state. + */ CLUSTER_IS_PRE_FINALIZED, + /** + * Classifies validations that has to run, when the client uses an older + * protocol version than the server. + */ OLDER_CLIENT_REQUESTS, + /** + * Classifies validations that has to run, when the client uses a newer + * protocol version than the server. + */ NEWER_CLIENT_REQUESTS, + /** + * Classifies validations that has to run for every request. + * If you plan to use this, please justify why the validation code should not + * be part of the actual request handling code. + */ UNCONDITIONAL; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java index bf9a3c1523a9..5334feceb16e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java @@ -18,14 +18,40 @@ import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; +/** + * A context that contains useful information for request validator instances. + */ public interface ValidationContext { + /** + * Gets the {@link LayoutVersionManager} of the service, so that a pre + * finalization validation can check if the layout version it belongs to + * is finalized already or not. + * + * @return the {@link LayoutVersionManager} of the service + */ LayoutVersionManager versionManager(); + /** + * Provides the protocol version of the server side running. + * @return the server side protocol version + */ int serverVersion(); + /** + * Provides the protocol version of the clients side sending the request. + * @return the client side protocol version + */ int clientVersion(); + /** + * Creates a context object based on the given parameters. + * + * @param versionManager the {@link LayoutVersionManager} of the service + * @param serverVersion the server side protocol version + * @param clientVersion the client side protocol version + * @return the {@link ValidationContext} specified by the parameters. + */ static ValidationContext of( LayoutVersionManager versionManager, int serverVersion, diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index 52a34feaa17d..419a77812cf0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -33,20 +33,39 @@ import java.util.Map; import java.util.Set; +/** + * Registry that loads and stores the request validators to be applied by + * a service. + */ public class ValidatorRegistry { EnumMap, List>>> validators = new EnumMap<>(ValidationCondition.class); - public static void main(String[] args) { - new ValidatorRegistry("org.apache.hadoop.ozone"); - } - + /** + * Creates a {@link ValidatorRegistry} instance that discovers validation + * methods in the provided package and its sub-packages. + * A validation method is recognized by the {@link RequestFeatureValidator} + * annotation that contains important information about how and when to use + * the validator. + * @param validatorPackage the main package inside which validatiors should + * be discovered. + */ public ValidatorRegistry(String validatorPackage) { initMaps(validatorPackage); System.out.println(validators.entrySet()); } + /** + * Get the validators that has to be run in the given list of + * {@link ValidationCondition}s, for the given requestType and + * {@link RequestProcessingPhase}. + * + * @param conditions conditions that are present for the request + * @param requestType the type of the protocol message + * @param phase the request processing phase + * @return the list of validation methods that has to run. + */ public List validationsFor( List conditions, Type requestType, @@ -65,6 +84,14 @@ public List validationsFor( return returnValue; } + /** + * Grabs validations for one particular condition. + * + * @param condition conditions that are present for the request + * @param requestType the type of the protocol message + * @param phase the request processing phase + * @return the list of validation methods that has to run. + */ private List validationsFor( ValidationCondition condition, Type requestType, @@ -92,6 +119,17 @@ private List validationsFor( return returnValue; } + /** + * Initializes the internal request validator store. + * The requests are stored in the following structure: + * - An EnumMap with the {@link ValidationCondition} as the key, and in which + * - values are an EnumMap with the request type as the key, and in which + * - values are Pair of lists, in which + * - left side is the pre-processing validations list + * - right side is the post-processing validations list + * @param validatorPackage the package in which the methods annotated with + * {@link RequestFeatureValidator} are gathered. + */ private void initMaps(String validatorPackage) { Reflections reflections = new Reflections(new ConfigurationBuilder() .setUrls(ClasspathHelper.forPackage(validatorPackage)) @@ -120,7 +158,6 @@ private void initMaps(String validatorPackage) { } } - @NotNull private EnumMap, List>> newTypeMap() { return new EnumMap<>(Type.class); } From 8b9ef2b2c49cd4f14ef0cbc24d18d79c04dbaaf1 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 4 Feb 2022 16:36:19 +0100 Subject: [PATCH 05/36] Fix checkstyle --- .../ozone/om/request/validation/ValidatorRegistry.java | 7 ++++--- .../OzoneManagerProtocolServerSideTranslatorPB.java | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index 419a77812cf0..1c5345bbdc6b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -18,7 +18,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; -import org.jetbrains.annotations.NotNull; import org.reflections.Reflections; import org.reflections.scanners.MethodAnnotationsScanner; import org.reflections.util.ClasspathHelper; @@ -39,7 +38,9 @@ */ public class ValidatorRegistry { - EnumMap, List>>> + private + EnumMap< + ValidationCondition, EnumMap, List>>> validators = new EnumMap<>(ValidationCondition.class); /** @@ -162,7 +163,7 @@ private EnumMap, List>> newTypeMap() { return new EnumMap<>(Type.class); } - V getAndInitialize(K key, V defaultValue, Map from) { + V getAndInitialize(K key, V defaultValue, Map from) { if (defaultValue == null) { throw new NullPointerException( "Entry can not be initialized with null value."); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index f7e1bb4b5822..1b6b1a690e23 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -198,7 +198,8 @@ private OMResponse postValidate( // TODO: restrict the parameter list and return type of such annotated // methods either at compile time, or via a test try { - if (method.getAnnotation(RequestFeatureValidator.class).contextAware()) { + if (method + .getAnnotation(RequestFeatureValidator.class).contextAware()) { validatedResponse = (OMResponse) method .invoke(null, originalRequest, validatedResponse, context); } else { From b392734b05b0b1e75e11e245928941dde4839e5d Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Tue, 8 Feb 2022 01:19:03 +0100 Subject: [PATCH 06/36] Reorganize code, to simplify the integration point in the server side translator code. --- .../validation/RequestValidations.java | 98 ++++++++++++++++ .../validation/ValidationCondition.java | 2 +- .../request/validation/ValidationContext.java | 15 +-- .../request/validation/ValidatorRegistry.java | 21 +++- ...ManagerProtocolServerSideTranslatorPB.java | 111 ++---------------- 5 files changed, 130 insertions(+), 117 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java new file mode 100644 index 000000000000..02e4f947454f --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -0,0 +1,98 @@ +package org.apache.hadoop.ozone.om.request.validation; + +import com.google.protobuf.ServiceException; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.LinkedList; +import java.util.List; + +import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.POST_PROCESS; +import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.PRE_PROCESS; + +public class RequestValidations { + + private static final String DEFAULT_PACKAGE = "org.apache.hadoop.ozone"; + + private String validationsPackageName = DEFAULT_PACKAGE; + private ValidationContext context = ValidationContext.of(null, -1); + private ValidatorRegistry registry = ValidatorRegistry.emptyRegistry(); + + public RequestValidations fromPackage(String packageName) { + validationsPackageName = packageName; + return this; + } + + public RequestValidations withinContext(ValidationContext context) { + this.context = context; + return this; + } + + public synchronized RequestValidations load() { + registry = new ValidatorRegistry(validationsPackageName); + return this; + } + + public OMRequest validateRequest(OMRequest request) throws ServiceException { + List validations = registry.validationsFor( + conditions(request), request.getCmdType(), PRE_PROCESS); + + OMRequest validatedRequest = request.toBuilder().build(); + try { + for (Method m : validations) { + if (m.getParameterCount() == 2) { // context aware + return (OMRequest) m.invoke(null, validatedRequest, context); + } else { + return (OMRequest) m.invoke(null, validatedRequest); + } + } + } catch (IllegalAccessException | InvocationTargetException e) { + throw new ServiceException(e); + } + // this should not happen, as parameter count is enforced by tests, + // but for sanity return the request as is. + return validatedRequest; + } + + public OMResponse validateResponse(OMRequest request, OMResponse response) + throws ServiceException { + List validations = registry.validationsFor( + conditions(request), request.getCmdType(), POST_PROCESS); + + OMResponse validatedResponse = response.toBuilder().build(); + try { + for (Method m : validations) { + if (m.getParameterCount() == 3) { // context aware post processor + return (OMResponse) m.invoke(null, request, response, context); + } else { + return (OMResponse) m.invoke(null, request, response); + } + } + } catch (InvocationTargetException | IllegalAccessException e) { + throw new ServiceException(e); + } + // this should not happen, as parameter count is enforced by tests, + // but for sanity return the response as is. + return validatedResponse; + } + + private List conditions(OMRequest request) { + List conditions = new LinkedList<>(); + conditions.add(ValidationCondition.UNCONDITIONAL); + if (context.serverVersion() != request.getVersion()) { + if (context.serverVersion() < request.getVersion()) { + conditions.add(ValidationCondition.NEWER_CLIENT_REQUESTS); + } else { + conditions.add(ValidationCondition.OLDER_CLIENT_REQUESTS); + } + } + if (context.versionManager() != null + && context.versionManager().needsFinalization()) { + conditions.add(ValidationCondition.CLUSTER_NEEDS_FINALIZATION); + } + return conditions; + } + +} diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java index 49ab6ee6056d..fb61a4af5fba 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -30,7 +30,7 @@ public enum ValidationCondition { * Classifies validations that has to run after an upgrade until the cluster * is in a pre-finalized state. */ - CLUSTER_IS_PRE_FINALIZED, + CLUSTER_NEEDS_FINALIZATION, /** * Classifies validations that has to run, when the client uses an older * protocol version than the server. diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java index 5334feceb16e..ce8cecf79c57 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java @@ -38,24 +38,16 @@ public interface ValidationContext { */ int serverVersion(); - /** - * Provides the protocol version of the clients side sending the request. - * @return the client side protocol version - */ - int clientVersion(); - /** * Creates a context object based on the given parameters. * * @param versionManager the {@link LayoutVersionManager} of the service * @param serverVersion the server side protocol version - * @param clientVersion the client side protocol version * @return the {@link ValidationContext} specified by the parameters. */ static ValidationContext of( LayoutVersionManager versionManager, - int serverVersion, - int clientVersion) { + int serverVersion) { return new ValidationContext() { @Override public LayoutVersionManager versionManager() { @@ -66,11 +58,6 @@ public LayoutVersionManager versionManager() { public int serverVersion() { return serverVersion; } - - @Override - public int clientVersion() { - return clientVersion; - } }; } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index 1c5345bbdc6b..da00ad26421b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -52,7 +52,7 @@ public class ValidatorRegistry { * @param validatorPackage the main package inside which validatiors should * be discovered. */ - public ValidatorRegistry(String validatorPackage) { + ValidatorRegistry(String validatorPackage) { initMaps(validatorPackage); System.out.println(validators.entrySet()); } @@ -67,7 +67,7 @@ public ValidatorRegistry(String validatorPackage) { * @param phase the request processing phase * @return the list of validation methods that has to run. */ - public List validationsFor( + List validationsFor( List conditions, Type requestType, RequestProcessingPhase phase) { @@ -131,7 +131,7 @@ private List validationsFor( * @param validatorPackage the package in which the methods annotated with * {@link RequestFeatureValidator} are gathered. */ - private void initMaps(String validatorPackage) { + void initMaps(String validatorPackage) { Reflections reflections = new Reflections(new ConfigurationBuilder() .setUrls(ClasspathHelper.forPackage(validatorPackage)) .setScanners(new MethodAnnotationsScanner()) @@ -163,7 +163,7 @@ private EnumMap, List>> newTypeMap() { return new EnumMap<>(Type.class); } - V getAndInitialize(K key, V defaultValue, Map from) { + private V getAndInitialize(K key, V defaultValue, Map from) { if (defaultValue == null) { throw new NullPointerException( "Entry can not be initialized with null value."); @@ -190,4 +190,17 @@ private Pair, List> newListPair() { return Pair.of(new ArrayList<>(), new ArrayList<>()); } + static ValidatorRegistry emptyRegistry() { + return new ValidatorRegistry("") { + @Override + List validationsFor(List conditions, + Type requestType, RequestProcessingPhase phase) { + return Collections.emptyList(); + } + + @Override + void initMaps(String validatorPackage) {} + }; + } + } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 1b6b1a690e23..affe732e40a3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -22,10 +22,6 @@ import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.PrepareStatus; import java.io.IOException; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.util.LinkedList; -import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicLong; @@ -42,11 +38,8 @@ import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer.RaftServerStatus; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; import org.apache.hadoop.ozone.om.request.OMClientRequest; -import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; -import org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase; -import org.apache.hadoop.ozone.om.request.validation.ValidationCondition; +import org.apache.hadoop.ozone.om.request.validation.RequestValidations; import org.apache.hadoop.ozone.om.request.validation.ValidationContext; -import org.apache.hadoop.ozone.om.request.validation.ValidatorRegistry; import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -80,7 +73,7 @@ public class OzoneManagerProtocolServerSideTranslatorPB implements private final AtomicLong transactionIndex; private final OzoneProtocolMessageDispatcher dispatcher; - private final ValidatorRegistry validatorRegistry; + private final RequestValidations requestValidations; /** * Constructs an instance of the server handler. @@ -124,7 +117,14 @@ public OzoneManagerProtocolServerSideTranslatorPB( dispatcher = new OzoneProtocolMessageDispatcher<>("OzoneProtocol", metrics, LOG, OMPBHelper::processForDebug, OMPBHelper::processForDebug); // TODO: make this injectable for testing... - validatorRegistry = new ValidatorRegistry(OM_REQUESTS_PACKAGE); + requestValidations = + new RequestValidations() + .fromPackage(OM_REQUESTS_PACKAGE) + .withinContext( + ValidationContext.of( + ozoneManager.getVersionManager(), + getServerProtocolVersion())) + .load(); } /** @@ -135,101 +135,16 @@ public OzoneManagerProtocolServerSideTranslatorPB( @Override public OMResponse submitRequest(RpcController controller, OMRequest request) throws ServiceException { - List conditions = getConditions(request); - ValidationContext context = ValidationContext.of( - ozoneManager.getVersionManager(), 0, request.getVersion()); - - OMRequest validatedRequest = preValidate(conditions, context, request); + OMRequest validatedRequest = requestValidations.validateRequest(request); OMResponse response = dispatcher.processRequest(validatedRequest, this::processRequest, request.getCmdType(), request.getTraceID()); - return postValidate(conditions, context, validatedRequest, response); - } - - //TODO: move this code out from here, and just call from a validation util - // class or instead of gettign the validation from the registry we - // might even use that to run the validations... - private OMRequest preValidate( - List conditions, - ValidationContext context, - OMRequest originalRequest) throws ServiceException { - - List validations = validatorRegistry.validationsFor( - conditions, - originalRequest.getCmdType(), - RequestProcessingPhase.PRE_PROCESS); - - - OMRequest validatedRequest = originalRequest.toBuilder().build(); - for (Method method : validations) { - // TODO: restrict the parameter list and return type of such annotated - // methods either at compile time, or via a test - try { - if (method - .getAnnotation(RequestFeatureValidator.class).contextAware()) { - validatedRequest = - (OMRequest) method.invoke(null, validatedRequest, context); - } else { - validatedRequest = - (OMRequest) method.invoke(null, validatedRequest); - } - } catch (IllegalAccessException | InvocationTargetException e) { - throw new ServiceException(e); - } - } - return validatedRequest; - } - - private OMResponse postValidate( - List conditions, - ValidationContext context, - OMRequest originalRequest, - OMResponse originalResponse) throws ServiceException { - - List validations = validatorRegistry.validationsFor( - conditions, - originalRequest.getCmdType(), - RequestProcessingPhase.POST_PROCESS); - - OMResponse validatedResponse = originalResponse.toBuilder().build(); - for (Method method : validations) { - // TODO: restrict the parameter list and return type of such annotated - // methods either at compile time, or via a test - try { - if (method - .getAnnotation(RequestFeatureValidator.class).contextAware()) { - validatedResponse = (OMResponse) method - .invoke(null, originalRequest, validatedResponse, context); - } else { - validatedResponse = (OMResponse) method - .invoke(null, originalRequest, validatedResponse); - } - } catch (IllegalAccessException | InvocationTargetException e) { - throw new ServiceException(e); - } - } - return validatedResponse; - } - - private List getConditions(OMRequest request) { - List conditions = new LinkedList<>(); - conditions.add(ValidationCondition.UNCONDITIONAL); - int serverProtocolVersion = getServerProtocolVersion(); - int clientProtocolVersion = request.getVersion(); - if (serverProtocolVersion < clientProtocolVersion) { - conditions.add(ValidationCondition.NEWER_CLIENT_REQUESTS); - } else if (serverProtocolVersion > clientProtocolVersion) { - conditions.add(ValidationCondition.OLDER_CLIENT_REQUESTS); - } - if (ozoneManager.getVersionManager().needsFinalization()) { - conditions.add(ValidationCondition.CLUSTER_IS_PRE_FINALIZED); - } - return conditions; + return requestValidations.validateResponse(request, response); } - //TODO fidn out versioning and where it comes from... + //TODO find out versioning and where it comes from... private int getServerProtocolVersion() { return 0; } From 209f626c938dcba4594d76e8aac0c622b387b1be Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 9 Feb 2022 12:49:29 +0100 Subject: [PATCH 07/36] first trials with apt... --- dev-support/annotations/pom.xml | 52 +++++++++++++++++++ .../RequestFeatureValidatorProcessor.java | 37 +++++++++++++ .../javax.annotation.processing.Processor | 1 + ...ManagerProtocolServerSideTranslatorPB.java | 10 ++++ pom.xml | 17 +++++- 5 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 dev-support/annotations/pom.xml create mode 100644 dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java create mode 100644 dev-support/annotations/src/main/resources/META-INF/services/javax.annotation.processing.Processor diff --git a/dev-support/annotations/pom.xml b/dev-support/annotations/pom.xml new file mode 100644 index 000000000000..8f300dd0837a --- /dev/null +++ b/dev-support/annotations/pom.xml @@ -0,0 +1,52 @@ + + 4.0.0 + + org.apache.ozone + ozone-annotation-processing + 1.3.0-SNAPSHOT + Apache Ozone annotation processing tools for validating custom + annotations at compile time. + Apache Ozone Annotation Processing + jar + + + UTF-8 + UTF-8 + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.9.0 + + 1.8 + 1.8 + + + + + + + \ No newline at end of file diff --git a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java new file mode 100644 index 000000000000..98afbd7855b3 --- /dev/null +++ b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java @@ -0,0 +1,37 @@ +package org.apache.ozone.annotations; + +import javax.annotation.processing.AbstractProcessor; +import javax.annotation.processing.RoundEnvironment; +import javax.annotation.processing.SupportedAnnotationTypes; +import javax.annotation.processing.SupportedSourceVersion; +import javax.lang.model.SourceVersion; +import javax.lang.model.element.Element; +import javax.lang.model.element.TypeElement; +import javax.lang.model.type.ExecutableType; +import javax.tools.Diagnostic; +import java.util.Set; + +@SupportedAnnotationTypes( + "org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator") +@SupportedSourceVersion(SourceVersion.RELEASE_8) +public class RequestFeatureValidatorProcessor extends AbstractProcessor { + + @Override + public boolean process(Set annotations, + RoundEnvironment roundEnv) { + + for (TypeElement annotation : annotations) { + Set annotatedElements + = roundEnv.getElementsAnnotatedWith(annotation); + for(Element element : annotatedElements) { + ExecutableType elem = ((ExecutableType) element.asType()); + if (elem.getParameterTypes().size()!=3) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "testing error...", element); + } + } + } + + return true; + } +} diff --git a/dev-support/annotations/src/main/resources/META-INF/services/javax.annotation.processing.Processor b/dev-support/annotations/src/main/resources/META-INF/services/javax.annotation.processing.Processor new file mode 100644 index 000000000000..3db1611cb36a --- /dev/null +++ b/dev-support/annotations/src/main/resources/META-INF/services/javax.annotation.processing.Processor @@ -0,0 +1 @@ +org.apache.ozone.annotations.RequestFeatureValidatorProcessor \ No newline at end of file diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index affe732e40a3..1e06a1bf8af8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -38,9 +38,13 @@ import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer.RaftServerStatus; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; import org.apache.hadoop.ozone.om.request.OMClientRequest; +import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; +import org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase; import org.apache.hadoop.ozone.om.request.validation.RequestValidations; +import org.apache.hadoop.ozone.om.request.validation.ValidationCondition; import org.apache.hadoop.ozone.om.request.validation.ValidationContext; import org.apache.hadoop.ozone.om.response.OMClientResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -133,6 +137,12 @@ public OzoneManagerProtocolServerSideTranslatorPB( * translator for OM protocol. */ @Override + @RequestFeatureValidator( + conditions = {ValidationCondition.NEWER_CLIENT_REQUESTS}, + contextAware = true, + requestType = OzoneManagerProtocolProtos.Type.CreateVolume, + processingPhase = RequestProcessingPhase.PRE_PROCESS + ) public OMResponse submitRequest(RpcController controller, OMRequest request) throws ServiceException { OMRequest validatedRequest = requestValidations.validateRequest(request); diff --git a/pom.xml b/pom.xml index d8a6252e4f77..0f99cbcee3a6 100644 --- a/pom.xml +++ b/pom.xml @@ -24,6 +24,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs pom + dev-support/annotations hadoop-hdds hadoop-ozone @@ -213,7 +214,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs ${maven-surefire-plugin.version} 2.5 - 3.1 + 3.9.0 2.5.1 3.1.0 3.2.4 @@ -1620,6 +1621,13 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs + + + org.apache.ozone + ozone-annotation-processing + ${ozone.version} + + @@ -1697,6 +1705,13 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs ${javac.version} ${javac.version} false + From d0d48402b41a17d1514b254a5c003a7d5e17c2b5 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 10 Feb 2022 23:30:16 +0100 Subject: [PATCH 08/36] Add an annotation processor to validate methods that are annotated to be a validator. (First draft version surely fails checkstyle and such have to be refactored and cleaned up, commiting for demo purposes.) --- dev-support/annotations/pom.xml | 87 +++++------ .../RequestFeatureValidatorProcessor.java | 142 +++++++++++++++++- hadoop-hdds/interface-client/pom.xml | 5 + ...ManagerProtocolServerSideTranslatorPB.java | 17 ++- pom.xml | 9 +- 5 files changed, 197 insertions(+), 63 deletions(-) diff --git a/dev-support/annotations/pom.xml b/dev-support/annotations/pom.xml index 8f300dd0837a..51ef06d7da80 100644 --- a/dev-support/annotations/pom.xml +++ b/dev-support/annotations/pom.xml @@ -2,51 +2,52 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> - 4.0.0 + 4.0.0 - org.apache.ozone - ozone-annotation-processing - 1.3.0-SNAPSHOT - Apache Ozone annotation processing tools for validating custom - annotations at compile time. - Apache Ozone Annotation Processing - jar + org.apache.ozone + ozone-annotation-processing + 1.3.0-SNAPSHOT + Apache Ozone annotation processing tools for validating custom + annotations at compile time. + + Apache Ozone Annotation Processing + jar - - UTF-8 - UTF-8 - + + UTF-8 + UTF-8 + - - - - org.apache.maven.plugins - maven-compiler-plugin - 3.9.0 - - 1.8 - 1.8 - - - - - + + + + org.apache.maven.plugins + maven-compiler-plugin + 3.9.0 + + 1.8 + 1.8 + + + + default-compile + + -proc:none + + org/apache/ozone/annotations/** + + + + + compile-project + compile + + compile + + + + + + \ No newline at end of file diff --git a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java index 98afbd7855b3..ea771c1a876b 100644 --- a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java +++ b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java @@ -5,10 +5,21 @@ import javax.annotation.processing.SupportedAnnotationTypes; import javax.annotation.processing.SupportedSourceVersion; import javax.lang.model.SourceVersion; +import javax.lang.model.element.AnnotationMirror; +import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.Modifier; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; import javax.lang.model.type.ExecutableType; +import javax.lang.model.type.TypeKind; +import javax.lang.model.type.TypeMirror; +import javax.lang.model.util.SimpleAnnotationValueVisitor8; import javax.tools.Diagnostic; +import java.util.List; +import java.util.Map.Entry; import java.util.Set; @SupportedAnnotationTypes( @@ -19,19 +30,138 @@ public class RequestFeatureValidatorProcessor extends AbstractProcessor { @Override public boolean process(Set annotations, RoundEnvironment roundEnv) { - + System.out.println("I am here!!!!"); for (TypeElement annotation : annotations) { Set annotatedElements = roundEnv.getElementsAnnotatedWith(annotation); - for(Element element : annotatedElements) { - ExecutableType elem = ((ExecutableType) element.asType()); - if (elem.getParameterTypes().size()!=3) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "testing error...", element); + for(Element elem : annotatedElements) { + for (AnnotationMirror methodAnnotation : elem.getAnnotationMirrors()){ + System.out.println("simpleName: " + methodAnnotation.getAnnotationType().asElement().getSimpleName()); + if (methodAnnotation.getAnnotationType().asElement().getSimpleName() + .contentEquals("RequestFeatureValidator")){ + int expectedParamCount = -1; + boolean hasContext = false; + boolean isPreprocessor = false; + for (Entry + entry : methodAnnotation.getElementValues().entrySet()) { + if (entry.getKey().getSimpleName().contentEquals("conditions") + && !entry.getValue().accept(new ConditionValidator(), null)) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Condition is empty..."); + } + if (entry.getKey().getSimpleName().contentEquals("contextAware")) { + System.out.println("HAS CONTEXT!"); + hasContext = true; + } + if (entry.getKey().getSimpleName().contentEquals("processingPhase")) { + String procPhase = entry.getValue().accept(new ProcessingPhaseVisitor(), null); + if (procPhase.equals("PRE_PROCESS")) { + isPreprocessor = true; + expectedParamCount = 1; + } else if (procPhase.equals("POST_PROCESS")){ + isPreprocessor = false; + expectedParamCount = 2; + } + } + } + if (hasContext) { + expectedParamCount++; + } + if (elem.getKind() != ElementKind.METHOD) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Annotated element is not a method..."); + } + if (!elem.getModifiers().contains(Modifier.STATIC)) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Validator method has to be static..."); + } + if (isPreprocessor && !((ExecutableElement) elem).getReturnType().toString() + .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest")) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Validator method has to return OMRequest..."); + } + if (!isPreprocessor && !((ExecutableElement) elem).getReturnType().toString() + .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse")) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Validator method has to return OMResponse..."); + } + List paramTypes = + ((ExecutableType) elem.asType()).getParameterTypes(); + int realParamCount = + paramTypes.size(); + if (realParamCount != expectedParamCount) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Unexpected parameter count. Expected: " + expectedParamCount + + "; found: " + realParamCount); + } + paramTypes.forEach(t -> { + if (!t.getKind().equals(TypeKind.DECLARED)) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Unexpected parameter type, it has to be a declared type." + + " Found: " + t); + } + }); + int contextOrder = -1; + if (isPreprocessor) { + contextOrder = 1; + } else { + contextOrder = 2; + if (paramTypes.size()>=2 && !paramTypes.get(1).toString() + .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse")) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Validator method second param has to be OMResponse..."); + } + } + if (paramTypes.size()>=1 && !paramTypes.get(0).toString() + .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest")) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Validator method first param has to be OMRequest..."); + } + if (paramTypes.size()>=contextOrder+1 && !paramTypes.get(contextOrder).toString() + .equals("org.apache.hadoop.ozone.om.request.validation.ValidationContext")) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, + "Validator method last param has to be ValidationContext..."); + } + } } } } return true; } + + class ConditionValidator + extends SimpleAnnotationValueVisitor8 { + + ConditionValidator() { + super(Boolean.TRUE); + } + + @Override + public Boolean visitArray(List vals, + Void unused) { + if (vals.isEmpty()) { + return Boolean.FALSE; + } + return Boolean.TRUE; + } + } + + class ProcessingPhaseVisitor + extends SimpleAnnotationValueVisitor8 { + + ProcessingPhaseVisitor() { + super("UNKNOWN"); + } + + @Override + public String visitEnumConstant(VariableElement c, Void unused) { + if (c.getSimpleName().contentEquals("PRE_PROCESS")) { + return "PRE_PROCESS"; + } else if (c.getSimpleName().contentEquals("POST_PROCESS")) { + return "POST_PROCESS"; + } + throw new IllegalStateException("Method processing phase is unknown..."); + } + } } diff --git a/hadoop-hdds/interface-client/pom.xml b/hadoop-hdds/interface-client/pom.xml index 4b97b4539c26..eed1807e6e1e 100644 --- a/hadoop-hdds/interface-client/pom.xml +++ b/hadoop-hdds/interface-client/pom.xml @@ -47,6 +47,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> javax.annotation javax.annotation-api + + com.google.code.findbugs + jsr305 + compile + diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 1e06a1bf8af8..57ccf02d339b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.PrepareStatus; import java.io.IOException; +import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicLong; @@ -131,18 +132,22 @@ public OzoneManagerProtocolServerSideTranslatorPB( .load(); } + @RequestFeatureValidator( + conditions = {ValidationCondition.NEWER_CLIENT_REQUESTS}, + contextAware = true, + requestType = OzoneManagerProtocolProtos.Type.CreateVolume, + processingPhase = RequestProcessingPhase.POST_PROCESS + ) + public static OMResponse fooValidation(OMRequest first, OMResponse second, ValidationContext third) { + return null; + } + /** * Submit requests to Ratis server for OM HA implementation. * TODO: Once HA is implemented fully, we should have only one server side * translator for OM protocol. */ @Override - @RequestFeatureValidator( - conditions = {ValidationCondition.NEWER_CLIENT_REQUESTS}, - contextAware = true, - requestType = OzoneManagerProtocolProtos.Type.CreateVolume, - processingPhase = RequestProcessingPhase.PRE_PROCESS - ) public OMResponse submitRequest(RpcController controller, OMRequest request) throws ServiceException { OMRequest validatedRequest = requestValidations.validateRequest(request); diff --git a/pom.xml b/pom.xml index 0f99cbcee3a6..9b87595896d7 100644 --- a/pom.xml +++ b/pom.xml @@ -214,7 +214,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs ${maven-surefire-plugin.version} 2.5 - 3.9.0 + 3.1 2.5.1 3.1.0 3.2.4 @@ -1705,13 +1705,6 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs ${javac.version} ${javac.version} false - From 2a152e7e7bfa16f9eee3822a5c4824462cc407cb Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 11 Feb 2022 02:15:54 +0100 Subject: [PATCH 09/36] Remove contextAware property from the annotation, and from the annotation processor. --- .../RequestFeatureValidatorProcessor.java | 13 ++-------- .../validation/RequestFeatureValidator.java | 24 +++++++------------ ...ManagerProtocolServerSideTranslatorPB.java | 14 +---------- 3 files changed, 11 insertions(+), 40 deletions(-) diff --git a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java index ea771c1a876b..2c8d9ff72da6 100644 --- a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java +++ b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java @@ -40,7 +40,6 @@ public boolean process(Set annotations, if (methodAnnotation.getAnnotationType().asElement().getSimpleName() .contentEquals("RequestFeatureValidator")){ int expectedParamCount = -1; - boolean hasContext = false; boolean isPreprocessor = false; for (Entry entry : methodAnnotation.getElementValues().entrySet()) { @@ -49,24 +48,17 @@ public boolean process(Set annotations, processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, "Condition is empty..."); } - if (entry.getKey().getSimpleName().contentEquals("contextAware")) { - System.out.println("HAS CONTEXT!"); - hasContext = true; - } if (entry.getKey().getSimpleName().contentEquals("processingPhase")) { String procPhase = entry.getValue().accept(new ProcessingPhaseVisitor(), null); if (procPhase.equals("PRE_PROCESS")) { isPreprocessor = true; - expectedParamCount = 1; + expectedParamCount = 2; } else if (procPhase.equals("POST_PROCESS")){ isPreprocessor = false; - expectedParamCount = 2; + expectedParamCount = 3; } } } - if (hasContext) { - expectedParamCount++; - } if (elem.getKind() != ElementKind.METHOD) { processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, "Annotated element is not a method..."); @@ -126,7 +118,6 @@ public boolean process(Set annotations, } } } - return true; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java index d4427294188c..1bb00c9a960a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java @@ -45,12 +45,11 @@ * the request is processed by the regular code. * Its signature has to be the following: * - it has to be static and idempotent - * - it has to have one or two parameters - * - if not contextAware, the only parameter it should have is an + * - it has to have two parameters + * - the first parameter it is an * {@link * org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest} - * - if contextAware, a second parameter of type {@link ValidationContext} has - * to be there in the argument list. + * - the second parameter of type {@link ValidationContext} * - the method has to return the modified request, or throw a ServiceException * in case the request is considered to be invalid * - the method does not need to care about preserving the request it gets, @@ -63,12 +62,12 @@ * is calculated for a given request. * Its signature has to be the following: * - it has to be static and idempotent - * - it has 2 or 3 parameters + * - it has three parameters * - similalry to the pre-processing validators, first parameter is the - * OMRequest, the second parameter is the OMResponse, and the third optional - * parameter is a ValidationContext if the method marked to be context aware. - * - the method has to return the modified response of throw a ServiceException - * if the request is considered invalid based on the response. + * OMRequest, the second parameter is the OMResponse, and the third + * parameter is a ValidationContext. + * - the method has to return the modified OMResponse or throw a + * ServiceException if the request is considered invalid based on response. * - the method gets the request object that was supplied for the general * request processing code, not the original request, while it gets a copy * of the original response object provided by the general request processing @@ -97,11 +96,4 @@ */ Type requestType(); - /** - * Tells whether the validator requires a {@link ValidationContext} or it - * does not. - * @return if the method requires the context. - */ - boolean contextAware() default false; - } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 57ccf02d339b..97fea69c4612 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -132,20 +132,8 @@ public OzoneManagerProtocolServerSideTranslatorPB( .load(); } - @RequestFeatureValidator( - conditions = {ValidationCondition.NEWER_CLIENT_REQUESTS}, - contextAware = true, - requestType = OzoneManagerProtocolProtos.Type.CreateVolume, - processingPhase = RequestProcessingPhase.POST_PROCESS - ) - public static OMResponse fooValidation(OMRequest first, OMResponse second, ValidationContext third) { - return null; - } - /** - * Submit requests to Ratis server for OM HA implementation. - * TODO: Once HA is implemented fully, we should have only one server side - * translator for OM protocol. + * Submit mutating requests to Ratis server in OM, and process read requests. */ @Override public OMResponse submitRequest(RpcController controller, From 98db10175ac03ec01f894ae4d692091a28248aaa Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 11 Feb 2022 14:21:23 +0100 Subject: [PATCH 10/36] Make the annotation processor code more readable and structured. Fix checkstyle and rat. --- .../RequestFeatureValidatorProcessor.java | 304 ++++++++++++------ .../validation/RequestValidations.java | 24 +- .../request/validation/ValidatorRegistry.java | 4 +- ...ManagerProtocolServerSideTranslatorPB.java | 5 - 4 files changed, 235 insertions(+), 102 deletions(-) diff --git a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java index 2c8d9ff72da6..3d93ed50195e 100644 --- a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java +++ b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java @@ -7,6 +7,7 @@ import javax.lang.model.SourceVersion; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.AnnotationValue; +import javax.lang.model.element.AnnotationValueVisitor; import javax.lang.model.element.Element; import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; @@ -27,101 +28,212 @@ @SupportedSourceVersion(SourceVersion.RELEASE_8) public class RequestFeatureValidatorProcessor extends AbstractProcessor { + public static final String ERROR_CONDITION_IS_EMPTY = + "RequestFeatureValidator has an empty condition list. Please define the" + + " ValidationCondition in which the validator has to be applied."; + public static final String ERROR_ANNOTATED_ELEMENT_IS_NOT_A_METHOD = + "RequestFeatureValidator annotation is not applied to a method."; + public static final String ERROR_VALIDATOR_METHOD_HAS_TO_BE_STATIC = + "Only static methods can be annotated with the RequestFeatureValidator" + + " annotation."; + public static final String ERROR_UNEXPECTED_PARAMETER_TYPE = + "Unexpected parameter type, it has to be a declared type. Found: %s"; + public static final String ERROR_UNEXPECTED_PARAMETER_COUNT = + "Unexpected parameter count. Expected: %d; found: %d."; + public static final String ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMREQUEST = + "Pre-processing validator methods annotated with RequestFeatureValidator" + + " annotation has to return an OMRequest object."; + public static final String ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMRESPONSE = + "Post-processing validator methods annotated with RequestFeatureValidator" + + " annotation has to return an OMResponse object."; + public static final String ERROR_FIRST_PARAM_HAS_TO_BE_OMREQUEST = + "First parameter of a RequestFeatureValidator method has to be an" + + " OMRequest object."; + public static final String ERROR_LAST_PARAM_HAS_TO_BE_VALIDATION_CONTEXT = + "Last parameter of a RequestFeatureValidator method has to be" + + " ValidationContext object."; + public static final String ERROR_SECOND_PARAM_HAS_TO_BE_OMRESPONSE = + "Second parameter of a RequestFeatureValidator method has to be an" + + " OMResponse object."; + + public static final String OM_REQUEST_CLASS_NAME = + "org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos" + + ".OMRequest"; + public static final String OM_RESPONSE_CLASS_NAME = + "org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos" + + ".OMResponse"; + public static final String VALIDATION_CONTEXT_CLASS_NAME = + "org.apache.hadoop.ozone.om.request.validation.ValidationContext"; + + public static final String ANNOTATION_SIMPLE_NAME = "RequestFeatureValidator"; + public static final String ANNOTATION_CONDITIONS_PROPERTY_NAME = "conditions"; + public static final String ANNOTATION_PROCESSING_PHASE_PROPERTY_NAME = + "processingPhase"; + + public static final String PROCESSING_PHASE_PRE_PROCESS = "PRE_PROCESS"; + public static final String PROCESSING_PHASE_POST_PROCESS = "POST_PROCESS"; + public static final String ERROR_NO_PROCESSING_PHASE_DEFINED = + "RequestFeatureValidator has an invalid ProcessingPhase defined."; + @Override public boolean process(Set annotations, RoundEnvironment roundEnv) { - System.out.println("I am here!!!!"); for (TypeElement annotation : annotations) { - Set annotatedElements - = roundEnv.getElementsAnnotatedWith(annotation); - for(Element elem : annotatedElements) { - for (AnnotationMirror methodAnnotation : elem.getAnnotationMirrors()){ - System.out.println("simpleName: " + methodAnnotation.getAnnotationType().asElement().getSimpleName()); - if (methodAnnotation.getAnnotationType().asElement().getSimpleName() - .contentEquals("RequestFeatureValidator")){ - int expectedParamCount = -1; - boolean isPreprocessor = false; - for (Entry - entry : methodAnnotation.getElementValues().entrySet()) { - if (entry.getKey().getSimpleName().contentEquals("conditions") - && !entry.getValue().accept(new ConditionValidator(), null)) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Condition is empty..."); - } - if (entry.getKey().getSimpleName().contentEquals("processingPhase")) { - String procPhase = entry.getValue().accept(new ProcessingPhaseVisitor(), null); - if (procPhase.equals("PRE_PROCESS")) { - isPreprocessor = true; - expectedParamCount = 2; - } else if (procPhase.equals("POST_PROCESS")){ - isPreprocessor = false; - expectedParamCount = 3; - } - } - } - if (elem.getKind() != ElementKind.METHOD) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Annotated element is not a method..."); - } - if (!elem.getModifiers().contains(Modifier.STATIC)) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Validator method has to be static..."); - } - if (isPreprocessor && !((ExecutableElement) elem).getReturnType().toString() - .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest")) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Validator method has to return OMRequest..."); - } - if (!isPreprocessor && !((ExecutableElement) elem).getReturnType().toString() - .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse")) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Validator method has to return OMResponse..."); - } - List paramTypes = - ((ExecutableType) elem.asType()).getParameterTypes(); - int realParamCount = - paramTypes.size(); - if (realParamCount != expectedParamCount) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Unexpected parameter count. Expected: " + expectedParamCount - + "; found: " + realParamCount); - } - paramTypes.forEach(t -> { - if (!t.getKind().equals(TypeKind.DECLARED)) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Unexpected parameter type, it has to be a declared type." - + " Found: " + t); - } - }); - int contextOrder = -1; - if (isPreprocessor) { - contextOrder = 1; - } else { - contextOrder = 2; - if (paramTypes.size()>=2 && !paramTypes.get(1).toString() - .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse")) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Validator method second param has to be OMResponse..."); - } - } - if (paramTypes.size()>=1 && !paramTypes.get(0).toString() - .equals("org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest")) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Validator method first param has to be OMRequest..."); - } - if (paramTypes.size()>=contextOrder+1 && !paramTypes.get(contextOrder).toString() - .equals("org.apache.hadoop.ozone.om.request.validation.ValidationContext")) { - processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, - "Validator method last param has to be ValidationContext..."); - } - } - } + if (!annotation.getSimpleName().contentEquals(ANNOTATION_SIMPLE_NAME)) { + continue; + } + processElements(roundEnv.getElementsAnnotatedWith(annotation)); + } + return false; + } + + private void processElements(Set annotatedElements) { + for(Element elem : annotatedElements) { + for (AnnotationMirror methodAnnotation : elem.getAnnotationMirrors()){ + validateAnnotatedMethod(elem, methodAnnotation); + } + } + } + + private void validateAnnotatedMethod( + Element elem, AnnotationMirror methodAnnotation) { + boolean isPreprocessor = checkAndEvaluateAnnotation(methodAnnotation); + + checkMethodIsAnnotated(elem); + ensureAnnotatedMethodIsStatic(elem); + ensurePreProcessorReturnsOMReqest((ExecutableElement) elem, isPreprocessor); + ensurePostProcessorReturnsOMResponse( + (ExecutableElement) elem, isPreprocessor); + ensureMethodParameters(elem, isPreprocessor); + } + + private void ensureMethodParameters(Element elem, boolean isPreprocessor) { + List paramTypes = + ((ExecutableType) elem.asType()).getParameterTypes(); + ensureParameterCount(isPreprocessor, paramTypes); + ensureParametersHaveDeclaredTypes(paramTypes); + ensureParameterRequirements(paramTypes, 0, OM_REQUEST_CLASS_NAME, + ERROR_FIRST_PARAM_HAS_TO_BE_OMREQUEST); + if (!isPreprocessor) { + ensureParameterRequirements(paramTypes, 1, OM_RESPONSE_CLASS_NAME, + ERROR_SECOND_PARAM_HAS_TO_BE_OMRESPONSE); + } + int contextOrder = isPreprocessor ? 1 : 2; + ensureParameterRequirements(paramTypes, contextOrder, + VALIDATION_CONTEXT_CLASS_NAME, + ERROR_LAST_PARAM_HAS_TO_BE_VALIDATION_CONTEXT); + } + + private void ensureParametersHaveDeclaredTypes( + List paramTypes) { + paramTypes.forEach(t -> { + if (!t.getKind().equals(TypeKind.DECLARED)) { + emitErrorMsg(String.format( + ERROR_UNEXPECTED_PARAMETER_TYPE, t.getKind())); + } + }); + } + + private void ensureParameterCount(boolean isPreprocessor, + List paramTypes) { + int realParamCount = paramTypes.size(); + int expectedParamCount = isPreprocessor ? 2 : 3; + if (realParamCount != expectedParamCount) { + emitErrorMsg(String.format(ERROR_UNEXPECTED_PARAMETER_COUNT, + expectedParamCount, realParamCount)); + } + } + + private void ensureParameterRequirements( + List paramTypes, + int index, String validationContextClassName, + String errorLastParamHasToBeValidationContext) { + if (paramTypes.size() >= index+1 && + !paramTypes.get(index).toString().equals(validationContextClassName)) { + emitErrorMsg(errorLastParamHasToBeValidationContext); + } + } + + private void ensurePostProcessorReturnsOMResponse( + ExecutableElement elem, boolean isPreprocessor) { + if (!isPreprocessor && !elem.getReturnType().toString() + .equals(OM_RESPONSE_CLASS_NAME)) { + emitErrorMsg(ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMRESPONSE); + } + } + + private void ensurePreProcessorReturnsOMReqest( + ExecutableElement elem, boolean isPreprocessor) { + if (isPreprocessor && !elem.getReturnType().toString() + .equals(OM_REQUEST_CLASS_NAME)) { + emitErrorMsg(ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMREQUEST); + } + } + + private void ensureAnnotatedMethodIsStatic(Element elem) { + if (!elem.getModifiers().contains(Modifier.STATIC)) { + emitErrorMsg(ERROR_VALIDATOR_METHOD_HAS_TO_BE_STATIC); + } + } + + private void checkMethodIsAnnotated(Element elem) { + if (elem.getKind() != ElementKind.METHOD) { + emitErrorMsg(ERROR_ANNOTATED_ELEMENT_IS_NOT_A_METHOD); + } + } + + private boolean checkAndEvaluateAnnotation( + AnnotationMirror methodAnnotation) { + boolean isPreprocessor = false; + for (Entry + entry : methodAnnotation.getElementValues().entrySet()) { + + if (hasInvalidValidationCondition(entry)) { + emitErrorMsg(ERROR_CONDITION_IS_EMPTY); } + isPreprocessor = evaluateProcessingPhase(entry); } - return true; + return isPreprocessor; } - class ConditionValidator + private boolean evaluateProcessingPhase( + Entry entry) { + boolean isPreprocessor = false; + if (isProcessingPhaseValue(entry)) { + String procPhase = visit(entry, new ProcessingPhaseVisitor()); + if (procPhase.equals(PROCESSING_PHASE_PRE_PROCESS)) { + isPreprocessor = true; + } else if (procPhase.equals(PROCESSING_PHASE_POST_PROCESS)){ + isPreprocessor = false; + } + } + return isPreprocessor; + } + + private boolean isProcessingPhaseValue( + Entry entry) { + return isPropertyNamedAs(entry, ANNOTATION_PROCESSING_PHASE_PROPERTY_NAME); + } + + private boolean hasInvalidValidationCondition( + Entry entry) { + return isPropertyNamedAs(entry, ANNOTATION_CONDITIONS_PROPERTY_NAME) + && !visit(entry, new ConditionValidator()); + } + + private boolean isPropertyNamedAs( + Entry entry, + String simpleName) { + return entry.getKey().getSimpleName().contentEquals(simpleName); + } + + private T visit( + Entry entry, + AnnotationValueVisitor visitor) { + return entry.getValue().accept(visitor, null); + } + + private static class ConditionValidator extends SimpleAnnotationValueVisitor8 { ConditionValidator() { @@ -136,9 +248,10 @@ public Boolean visitArray(List vals, } return Boolean.TRUE; } + } - class ProcessingPhaseVisitor + private static class ProcessingPhaseVisitor extends SimpleAnnotationValueVisitor8 { ProcessingPhaseVisitor() { @@ -147,12 +260,17 @@ class ProcessingPhaseVisitor @Override public String visitEnumConstant(VariableElement c, Void unused) { - if (c.getSimpleName().contentEquals("PRE_PROCESS")) { - return "PRE_PROCESS"; - } else if (c.getSimpleName().contentEquals("POST_PROCESS")) { - return "POST_PROCESS"; + if (c.getSimpleName().contentEquals(PROCESSING_PHASE_PRE_PROCESS)) { + return PROCESSING_PHASE_PRE_PROCESS; + } else + if (c.getSimpleName().contentEquals(PROCESSING_PHASE_POST_PROCESS)) { + return PROCESSING_PHASE_POST_PROCESS; } - throw new IllegalStateException("Method processing phase is unknown..."); + throw new IllegalStateException(ERROR_NO_PROCESSING_PHASE_DEFINED); } } + + private void emitErrorMsg(String s) { + processingEnv.getMessager().printMessage(Diagnostic.Kind.ERROR, s); + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index 02e4f947454f..62f651847515 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.hadoop.ozone.om.request.validation; import com.google.protobuf.ServiceException; @@ -12,6 +28,10 @@ import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.POST_PROCESS; import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.PRE_PROCESS; +/** + * Main class to configure and set up and access the request/response + * validation framework. + */ public class RequestValidations { private static final String DEFAULT_PACKAGE = "org.apache.hadoop.ozone"; @@ -25,8 +45,8 @@ public RequestValidations fromPackage(String packageName) { return this; } - public RequestValidations withinContext(ValidationContext context) { - this.context = context; + public RequestValidations withinContext(ValidationContext validationContext) { + this.context = validationContext; return this; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index da00ad26421b..9dff165deea3 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -79,7 +79,7 @@ List validationsFor( List returnValue = validationsFor(conditions.get(0), requestType, phase); - for (int i=1; i < conditions.size(); i++) { + for (int i = 1; i < conditions.size(); i++) { returnValue.addAll(validationsFor(conditions.get(i), requestType, phase)); } return returnValue; @@ -199,7 +199,7 @@ List validationsFor(List conditions, } @Override - void initMaps(String validatorPackage) {} + void initMaps(String validatorPackage) { } }; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index 97fea69c4612..e731a3c54557 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -22,7 +22,6 @@ import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.PrepareStatus; import java.io.IOException; -import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicLong; @@ -39,13 +38,9 @@ import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer.RaftServerStatus; import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils; import org.apache.hadoop.ozone.om.request.OMClientRequest; -import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; -import org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase; import org.apache.hadoop.ozone.om.request.validation.RequestValidations; -import org.apache.hadoop.ozone.om.request.validation.ValidationCondition; import org.apache.hadoop.ozone.om.request.validation.ValidationContext; import org.apache.hadoop.ozone.om.response.OMClientResponse; -import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; From 926266b6531692bf84f75c235e1bd19806ec0a4d Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Sat, 12 Feb 2022 17:51:14 +0100 Subject: [PATCH 11/36] Change the validation logic, after removing conext awareness as an option. --- .../request/validation/RequestValidations.java | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index 62f651847515..49609e785dfe 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -62,17 +62,12 @@ public OMRequest validateRequest(OMRequest request) throws ServiceException { OMRequest validatedRequest = request.toBuilder().build(); try { for (Method m : validations) { - if (m.getParameterCount() == 2) { // context aware - return (OMRequest) m.invoke(null, validatedRequest, context); - } else { - return (OMRequest) m.invoke(null, validatedRequest); - } + validatedRequest = + (OMRequest) m.invoke(null, validatedRequest, context); } } catch (IllegalAccessException | InvocationTargetException e) { throw new ServiceException(e); } - // this should not happen, as parameter count is enforced by tests, - // but for sanity return the request as is. return validatedRequest; } @@ -84,17 +79,12 @@ public OMResponse validateResponse(OMRequest request, OMResponse response) OMResponse validatedResponse = response.toBuilder().build(); try { for (Method m : validations) { - if (m.getParameterCount() == 3) { // context aware post processor - return (OMResponse) m.invoke(null, request, response, context); - } else { - return (OMResponse) m.invoke(null, request, response); - } + validatedResponse = + (OMResponse) m.invoke(null, request, response, context); } } catch (InvocationTargetException | IllegalAccessException e) { throw new ServiceException(e); } - // this should not happen, as parameter count is enforced by tests, - // but for sanity return the response as is. return validatedResponse; } From 152917ddc873a043a156c04766cb2aa01bc778fc Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Tue, 15 Feb 2022 01:58:28 +0100 Subject: [PATCH 12/36] Add annotation processor tests, and fix some errors found. --- .../RequestFeatureValidatorProcessor.java | 33 +- hadoop-ozone/ozone-manager/pom.xml | 5 + .../request/validation/ValidatorRegistry.java | 1 + .../TestRequestFeatureValidator.java | 504 ++++++++++++++++++ pom.xml | 6 + 5 files changed, 525 insertions(+), 24 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java diff --git a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java index 3d93ed50195e..0d22d3fcb825 100644 --- a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java +++ b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java @@ -15,7 +15,6 @@ import javax.lang.model.element.TypeElement; import javax.lang.model.element.VariableElement; import javax.lang.model.type.ExecutableType; -import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.lang.model.util.SimpleAnnotationValueVisitor8; import javax.tools.Diagnostic; @@ -36,8 +35,6 @@ public class RequestFeatureValidatorProcessor extends AbstractProcessor { public static final String ERROR_VALIDATOR_METHOD_HAS_TO_BE_STATIC = "Only static methods can be annotated with the RequestFeatureValidator" + " annotation."; - public static final String ERROR_UNEXPECTED_PARAMETER_TYPE = - "Unexpected parameter type, it has to be a declared type. Found: %s"; public static final String ERROR_UNEXPECTED_PARAMETER_COUNT = "Unexpected parameter count. Expected: %d; found: %d."; public static final String ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMREQUEST = @@ -111,7 +108,6 @@ private void ensureMethodParameters(Element elem, boolean isPreprocessor) { List paramTypes = ((ExecutableType) elem.asType()).getParameterTypes(); ensureParameterCount(isPreprocessor, paramTypes); - ensureParametersHaveDeclaredTypes(paramTypes); ensureParameterRequirements(paramTypes, 0, OM_REQUEST_CLASS_NAME, ERROR_FIRST_PARAM_HAS_TO_BE_OMREQUEST); if (!isPreprocessor) { @@ -124,16 +120,6 @@ private void ensureMethodParameters(Element elem, boolean isPreprocessor) { ERROR_LAST_PARAM_HAS_TO_BE_VALIDATION_CONTEXT); } - private void ensureParametersHaveDeclaredTypes( - List paramTypes) { - paramTypes.forEach(t -> { - if (!t.getKind().equals(TypeKind.DECLARED)) { - emitErrorMsg(String.format( - ERROR_UNEXPECTED_PARAMETER_TYPE, t.getKind())); - } - }); - } - private void ensureParameterCount(boolean isPreprocessor, List paramTypes) { int realParamCount = paramTypes.size(); @@ -191,23 +177,22 @@ private boolean checkAndEvaluateAnnotation( if (hasInvalidValidationCondition(entry)) { emitErrorMsg(ERROR_CONDITION_IS_EMPTY); } - isPreprocessor = evaluateProcessingPhase(entry); + if (isProcessingPhaseValue(entry)) { + isPreprocessor = evaluateProcessingPhase(entry); + } } return isPreprocessor; } private boolean evaluateProcessingPhase( Entry entry) { - boolean isPreprocessor = false; - if (isProcessingPhaseValue(entry)) { - String procPhase = visit(entry, new ProcessingPhaseVisitor()); - if (procPhase.equals(PROCESSING_PHASE_PRE_PROCESS)) { - isPreprocessor = true; - } else if (procPhase.equals(PROCESSING_PHASE_POST_PROCESS)){ - isPreprocessor = false; - } + String procPhase = visit(entry, new ProcessingPhaseVisitor()); + if (procPhase.equals(PROCESSING_PHASE_PRE_PROCESS)) { + return true; + } else if (procPhase.equals(PROCESSING_PHASE_POST_PROCESS)){ + return false; } - return isPreprocessor; + return false; } private boolean isProcessingPhaseValue( diff --git a/hadoop-ozone/ozone-manager/pom.xml b/hadoop-ozone/ozone-manager/pom.xml index cea140b122c3..5e42414ed701 100644 --- a/hadoop-ozone/ozone-manager/pom.xml +++ b/hadoop-ozone/ozone-manager/pom.xml @@ -133,6 +133,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> test + + com.google.testing.compile + compile-testing + + org.reflections reflections diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index 9dff165deea3..beb68b4ea24f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -144,6 +144,7 @@ void initMaps(String validatorPackage) { for (Method m : describedValidators) { RequestFeatureValidator descriptor = m.getAnnotation(RequestFeatureValidator.class); + m.setAccessible(true); for (ValidationCondition condition : descriptor.conditions()) { EnumMap, List>> requestTypeMap = diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java new file mode 100644 index 000000000000..a5c227b58dd5 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java @@ -0,0 +1,504 @@ +package org.apache.hadoop.ozone.om.request.validations; + +import com.google.testing.compile.Compilation; +import com.google.testing.compile.JavaFileObjects; +import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; +import org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase; +import org.apache.hadoop.ozone.om.request.validation.ValidationCondition; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; +import org.apache.ozone.annotations.RequestFeatureValidatorProcessor; +import org.junit.Test; + +import java.lang.annotation.Annotation; +import java.lang.annotation.ElementType; +import java.lang.annotation.Target; +import java.util.ArrayList; +import java.util.List; + +import static com.google.testing.compile.CompilationSubject.assertThat; +import static com.google.testing.compile.Compiler.javac; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_CONDITION_IS_EMPTY; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_FIRST_PARAM_HAS_TO_BE_OMREQUEST; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_LAST_PARAM_HAS_TO_BE_VALIDATION_CONTEXT; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_SECOND_PARAM_HAS_TO_BE_OMRESPONSE; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_UNEXPECTED_PARAMETER_COUNT; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_VALIDATOR_METHOD_HAS_TO_BE_STATIC; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMREQUEST; +import static org.apache.ozone.annotations.RequestFeatureValidatorProcessor.ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMRESPONSE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; + +public class TestRequestFeatureValidator { + + private static final String CLASSNAME = "Validation"; + + @Test + public void testAnnotationCanOnlyBeAppliedOnMethods() { + Class c = RequestFeatureValidator.class; + for (Annotation a : c.getAnnotations()) { + if (a instanceof Target) { + assertEquals(1, ((Target) a).value().length); + assertSame(((Target) a).value()[0], ElementType.METHOD); + } + } + } + + @Test + public void testACorrectAnnotationSetupForPreProcessCompiles() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions("ServiceException")); + + assertThat(compile(source)).succeeded(); + } + + @Test + public void testACorrectAnnotationSetupForPostProcessCompiles() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), postProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMResponse"), + parameters("OMRequest rq", "OMResponse rp", "ValidationContext ctx"), + exceptions("ServiceException")); + + assertThat(compile(source)).succeeded(); + } + + @Test + public void testValidatorDoesNotNecessarilyThrowsExceptions() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)).succeeded(); + } + + @Test + public void testNonStaticValidatorDoesNotCompile() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_VALIDATOR_METHOD_HAS_TO_BE_STATIC); + } + + @Test + public void testValidatorMethodCanBeFinal() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static", "final"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)).succeeded(); + } + + @Test + public void testValidatorMethodCanBePrivate() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("private", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)).succeeded(); + } + + @Test + public void testValidatorMethodCanBeDefaultVisible() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)).succeeded(); + } + + @Test + public void testValidatorMethodCanBeProtected() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("protected", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)).succeeded(); + } + + @Test + public void testEmptyValidationConditionListDoesNotCompile() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(emptyConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)).hadErrorContaining(ERROR_CONDITION_IS_EMPTY); + } + + @Test + public void testNotEnoughParametersForPreProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining( + String.format(ERROR_UNEXPECTED_PARAMETER_COUNT, 2, 1)); + } + + @Test + public void testTooManyParametersForPreProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "OMResponse rp", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining( + String.format(ERROR_UNEXPECTED_PARAMETER_COUNT, 2, 3)); + } + + @Test + public void testNotEnoughParametersForPostProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), postProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMResponse"), + parameters("OMRequest rq", "OMResponse rp"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining( + String.format(ERROR_UNEXPECTED_PARAMETER_COUNT, 3, 2)); + } + + @Test + public void testTooManyParametersForPostProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), postProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMResponse"), + parameters("OMRequest rq", "OMResponse rp", "ValidationContext ctx", + "String name"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining( + String.format(ERROR_UNEXPECTED_PARAMETER_COUNT, 3, 4)); + } + + @Test + public void testWrongReturnValueForPreProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("String"), + parameters("OMRequest rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMREQUEST); + } + + @Test + public void testWrongReturnValueForPostProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), postProcess(), aReqType()), + modifiers("public", "static"), + returnValue("String"), + parameters("OMRequest rq", "OMResponse rp", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMRESPONSE); + } + + @Test + public void testWrongFirstArgumentForPreProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMRequest"), + parameters("String rq", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_FIRST_PARAM_HAS_TO_BE_OMREQUEST); + } + + @Test + public void testWrongFirstArgumentForPostProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), postProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMResponse"), + parameters("String rq", "OMResponse rp", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_FIRST_PARAM_HAS_TO_BE_OMREQUEST); + } + + @Test + public void testWrongSecondArgumentForPreProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), preProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMRequest"), + parameters("OMRequest rq", "String ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_LAST_PARAM_HAS_TO_BE_VALIDATION_CONTEXT); + } + + @Test + public void testWrongSecondArgumentForPostProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), postProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMResponse"), + parameters("OMRequest rq", "String rp", "ValidationContext ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_SECOND_PARAM_HAS_TO_BE_OMRESPONSE); + } + + @Test + public void testWrongThirdArgumentForPostProcess() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), postProcess(), aReqType()), + modifiers("public", "static"), + returnValue("OMResponse"), + parameters("OMRequest rq", "OMResponse rp", "String ctx"), + exceptions()); + + assertThat(compile(source)) + .hadErrorContaining(ERROR_LAST_PARAM_HAS_TO_BE_VALIDATION_CONTEXT); + } + + @Test + public void testInvalidProcessingPhase() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(someConditions(), "INVALID", aReqType()), + modifiers("public", "static"), + returnValue("OMResponse"), + parameters("OMRequest rq", "OMResponse rp", "ValidationContext ctx"), + exceptions("ServiceException")); + + assertThat(compile(source)).failed(); + } + + @Test + public void testMultipleErrorMessages() { + List source = generateSourceOfValidatorMethodWith( + annotationOf(emptyConditions(), postProcess(), aReqType()), + modifiers(), + returnValue("String"), + parameters("String rq", "int rp", "String ctx"), + exceptions()); + + Compilation compilation = compile(source); + assertThat(compilation).hadErrorContaining(ERROR_CONDITION_IS_EMPTY); + assertThat(compilation) + .hadErrorContaining(ERROR_VALIDATOR_METHOD_HAS_TO_BE_STATIC); + assertThat(compilation) + .hadErrorContaining(ERROR_VALIDATOR_METHOD_HAS_TO_RETURN_OMRESPONSE); + assertThat(compilation) + .hadErrorContaining(ERROR_FIRST_PARAM_HAS_TO_BE_OMREQUEST); + assertThat(compilation) + .hadErrorContaining(ERROR_SECOND_PARAM_HAS_TO_BE_OMRESPONSE); + assertThat(compilation) + .hadErrorContaining(ERROR_LAST_PARAM_HAS_TO_BE_VALIDATION_CONTEXT); + } + + private Compilation compile(List source) { + Compilation c = javac() + .withProcessors(new RequestFeatureValidatorProcessor()) + .compile(JavaFileObjects.forSourceLines(CLASSNAME, source)); + c.diagnostics().forEach(System.out::println); + return c; + } + + private ValidationCondition[] someConditions() { + return + new ValidationCondition[] {ValidationCondition.NEWER_CLIENT_REQUESTS}; + } + + private ValidationCondition[] emptyConditions() { + return new ValidationCondition[] {}; + } + + private RequestProcessingPhase preProcess() { + return RequestProcessingPhase.PRE_PROCESS; + } + + private RequestProcessingPhase postProcess() { + return RequestProcessingPhase.POST_PROCESS; + } + + private Type aReqType() { + return Type.CreateVolume; + } + + private String returnValue(String retVal) { + return retVal; + } + + private String[] parameters(String... params) { + return params; + } + + private String[] modifiers(String... modifiers) { + return modifiers; + } + + private String[] exceptions(String... exceptions) { + return exceptions; + } + + private List generateSourceOfValidatorMethodWith( + String annotation, + String[] modifiers, + String returnType, + String[] paramspecs, + String[] exceptions) { + List lines = new ArrayList<>(allImports()); + lines.add(""); + lines.add("public class " + CLASSNAME + " {"); + lines.add(""); + lines.add(" " + annotation); + StringBuilder signature = + buildMethodSignature(modifiers, returnType, paramspecs, exceptions); + lines.add(signature.toString()); + lines.add(" return null;"); + lines.add(" }"); + lines.add("}"); + lines.add(""); + lines.stream() + .filter(s -> !s.startsWith("import")) + .forEach(System.out::println); + return lines; + } + + private String annotationOf( + ValidationCondition[] conditions, + RequestProcessingPhase phase, + Type reqType) { + return annotationOf(conditions, phase.name(), reqType); + } + + private String annotationOf( + ValidationCondition[] conditions, + String phase, + Type reqType) { + StringBuilder annotation = new StringBuilder(); + annotation.append("@RequestFeatureValidator("); + StringBuilder conditionsArray = new StringBuilder(); + conditionsArray.append("conditions = { "); + if (conditions.length > 0) { + for (ValidationCondition condition : conditions) { + conditionsArray.append(condition.name()).append(", "); + } + annotation + .append(conditionsArray.substring(0, conditionsArray.length() - 2)); + } else { + annotation.append(conditionsArray); + } + annotation.append(" }"); + annotation.append(", processingPhase = ").append(phase); + annotation.append(", requestType = ").append(reqType.name()); + annotation.append(" )"); + return annotation.toString(); + } + + private List allImports() { + List imports = new ArrayList<>(); + imports.add("import org.apache.hadoop.ozone.om.request.validation" + + ".RequestFeatureValidator;"); + imports.add("import org.apache.hadoop.ozone.protocol.proto" + + ".OzoneManagerProtocolProtos.OMRequest;"); + imports.add("import org.apache.hadoop.ozone.protocol.proto" + + ".OzoneManagerProtocolProtos.OMResponse;"); + imports.add("import org.apache.hadoop.ozone.om.request.validation" + + ".ValidationContext;"); + imports.add("import com.google.protobuf.ServiceException;"); + for (ValidationCondition condition : ValidationCondition.values()) { + imports.add("import static org.apache.hadoop.ozone.om.request.validation" + + ".ValidationCondition." + condition.name() + ";"); + } + for (RequestProcessingPhase phase : RequestProcessingPhase.values()) { + imports.add("import static org.apache.hadoop.ozone.om.request.validation" + + ".RequestProcessingPhase." + phase.name() + ";"); + } + for (Type reqType : Type.values()) { + imports.add("import static org.apache.hadoop.ozone.protocol.proto" + + ".OzoneManagerProtocolProtos.Type." + reqType.name() + ";"); + } + return imports; + } + + private StringBuilder buildMethodSignature( + String[] modifiers, String returnType, + String[] paramspecs, String[] exceptions) { + StringBuilder signature = new StringBuilder(); + signature.append(" "); + for (String modifier : modifiers) { + signature.append(modifier).append(" "); + } + signature.append(returnType).append(" "); + signature.append("validatorMethod("); + signature.append(createParameterList(paramspecs)); + signature.append(") "); + signature.append(createThrowsClause(exceptions)); + return signature.append(" {"); + } + + private String createParameterList(String[] paramSpecs) { + if (paramSpecs == null || paramSpecs.length == 0) { + return ""; + } + StringBuilder parameters = new StringBuilder(); + for (String paramSpec : paramSpecs) { + parameters.append(paramSpec).append(", "); + } + return parameters.substring(0, parameters.length() - 2); + } + + private String createThrowsClause(String[] exceptions) { + StringBuilder throwsClause = new StringBuilder(); + if (exceptions != null && exceptions.length > 0) { + throwsClause.append(" throws "); + StringBuilder exceptionList = new StringBuilder(); + for (String exception : exceptions) { + exceptionList.append(exception).append(", "); + } + throwsClause + .append(exceptionList.substring(0, exceptionList.length() - 2)); + } + return throwsClause.toString(); + } +} diff --git a/pom.xml b/pom.xml index 9b87595896d7..ffb32464f393 100644 --- a/pom.xml +++ b/pom.xml @@ -1287,6 +1287,12 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs ${mockito2.version} test + + com.google.testing.compile + compile-testing + 0.19 + test + org.objenesis objenesis From 9c2fe1abe4be807e069f757f336b37af706c4f37 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Tue, 15 Feb 2022 14:14:55 +0100 Subject: [PATCH 13/36] Add license text to new classes. --- .../RequestFeatureValidatorProcessor.java | 16 ++++++++++++++++ .../validations/TestRequestFeatureValidator.java | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java index 0d22d3fcb825..3e22528b15e8 100644 --- a/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java +++ b/dev-support/annotations/src/main/java/org/apache/ozone/annotations/RequestFeatureValidatorProcessor.java @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.ozone.annotations; import javax.annotation.processing.AbstractProcessor; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java index a5c227b58dd5..4a5522addecc 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java @@ -1,3 +1,19 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ package org.apache.hadoop.ozone.om.request.validations; import com.google.testing.compile.Compilation; From c7ae28acb8386dc9e1c7dabb651d0a234af29aa6 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 02:28:20 +0100 Subject: [PATCH 14/36] Add junit test, fix revealed problems, and remove some unnecessary stuff. --- .../validation/RequestValidations.java | 4 +- .../request/validation/ValidatorRegistry.java | 74 ++-- .../GeneralValidatorsForTesting.java | 190 ++++++++ .../TestRequestFeatureValidator.java | 5 +- .../validation/TestRequestValidations.java | 416 ++++++++++++++++++ .../validation/TestValidatorRegistry.java | 262 +++++++++++ ...ValidatorsForOnlyNewClientValidations.java | 37 ++ 7 files changed, 942 insertions(+), 46 deletions(-) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java rename hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/{validations => validation}/TestRequestFeatureValidator.java (98%) create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index 49609e785dfe..2baf196f2b84 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -37,8 +37,8 @@ public class RequestValidations { private static final String DEFAULT_PACKAGE = "org.apache.hadoop.ozone"; private String validationsPackageName = DEFAULT_PACKAGE; - private ValidationContext context = ValidationContext.of(null, -1); - private ValidatorRegistry registry = ValidatorRegistry.emptyRegistry(); + private ValidationContext context = null; + private ValidatorRegistry registry = null; public RequestValidations fromPackage(String packageName) { validationsPackageName = packageName; diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java index beb68b4ea24f..d55528586c8f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java @@ -24,9 +24,12 @@ import org.reflections.util.ConfigurationBuilder; import java.lang.reflect.Method; +import java.net.URL; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.EnumMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -38,14 +41,13 @@ */ public class ValidatorRegistry { - private - EnumMap< - ValidationCondition, EnumMap, List>>> + private final EnumMap< + ValidationCondition, EnumMap, List>>> validators = new EnumMap<>(ValidationCondition.class); /** * Creates a {@link ValidatorRegistry} instance that discovers validation - * methods in the provided package and its sub-packages. + * methods in the provided package and the packages in the same resource. * A validation method is recognized by the {@link RequestFeatureValidator} * annotation that contains important information about how and when to use * the validator. @@ -53,8 +55,27 @@ public class ValidatorRegistry { * be discovered. */ ValidatorRegistry(String validatorPackage) { - initMaps(validatorPackage); - System.out.println(validators.entrySet()); + this(ClasspathHelper.forPackage(validatorPackage)); + } + + /** + * Creates a {@link ValidatorRegistry} instance that discovers validation + * methods under the provided URL. + * A validation method is recognized by the {@link RequestFeatureValidator} + * annotation that contains important information about how and when to use + * the validator. + * @param searchUrls the path in which the annotated methods are searched. + */ + ValidatorRegistry(Collection searchUrls) { + Reflections reflections = new Reflections(new ConfigurationBuilder() + .setUrls(searchUrls) + .setScanners(new MethodAnnotationsScanner()) + .useParallelExecutor() + ); + + Set describedValidators = + reflections.getMethodsAnnotatedWith(RequestFeatureValidator.class); + initMaps(describedValidators); } /** @@ -76,13 +97,13 @@ List validationsFor( return Collections.emptyList(); } - List returnValue = - validationsFor(conditions.get(0), requestType, phase); + Set returnValue = + new HashSet<>(validationsFor(conditions.get(0), requestType, phase)); for (int i = 1; i < conditions.size(); i++) { returnValue.addAll(validationsFor(conditions.get(i), requestType, phase)); } - return returnValue; + return new ArrayList<>(returnValue); } /** @@ -113,9 +134,9 @@ private List validationsFor( List returnValue = new LinkedList<>(); if (phase.equals(RequestProcessingPhase.PRE_PROCESS)) { - returnValue = phases.getLeft(); + returnValue.addAll(phases.getLeft()); } else if (phase.equals(RequestProcessingPhase.POST_PROCESS)) { - returnValue = phases.getRight(); + returnValue.addAll(phases.getRight()); } return returnValue; } @@ -128,19 +149,9 @@ private List validationsFor( * - values are Pair of lists, in which * - left side is the pre-processing validations list * - right side is the post-processing validations list - * @param validatorPackage the package in which the methods annotated with - * {@link RequestFeatureValidator} are gathered. + * @param describedValidators collection of the annotated methods to process. */ - void initMaps(String validatorPackage) { - Reflections reflections = new Reflections(new ConfigurationBuilder() - .setUrls(ClasspathHelper.forPackage(validatorPackage)) - .setScanners(new MethodAnnotationsScanner()) - .useParallelExecutor() - ); - - Set describedValidators = - reflections.getMethodsAnnotatedWith(RequestFeatureValidator.class); - + void initMaps(Collection describedValidators) { for (Method m : describedValidators) { RequestFeatureValidator descriptor = m.getAnnotation(RequestFeatureValidator.class); @@ -165,10 +176,6 @@ private EnumMap, List>> newTypeMap() { } private V getAndInitialize(K key, V defaultValue, Map from) { - if (defaultValue == null) { - throw new NullPointerException( - "Entry can not be initialized with null value."); - } V inMapValue = from.get(key); if (inMapValue == null || !from.containsKey(key)) { from.put(key, defaultValue); @@ -191,17 +198,4 @@ private Pair, List> newListPair() { return Pair.of(new ArrayList<>(), new ArrayList<>()); } - static ValidatorRegistry emptyRegistry() { - return new ValidatorRegistry("") { - @Override - List validationsFor(List conditions, - Type requestType, RequestProcessingPhase phase) { - return Collections.emptyList(); - } - - @Override - void initMaps(String validatorPackage) { } - }; - } - } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java new file mode 100644 index 000000000000..bb876d08b471 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.hadoop.ozone.om.request.validation; + +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.POST_PROCESS; +import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.PRE_PROCESS; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.CLUSTER_NEEDS_FINALIZATION; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.NEWER_CLIENT_REQUESTS; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.OLDER_CLIENT_REQUESTS; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.UNCONDITIONAL; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateVolume; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.DeleteKeys; + +public class GeneralValidatorsForTesting { + + @FunctionalInterface + public interface ValidationListener { + void validationCalled(String calledMethodName); + } + + private static List listeners = new ArrayList<>(); + + public static void addListener(ValidationListener listener){ + listeners.add(listener); + } + + public static void removeListener(ValidationListener listener) { + listeners.remove(listener); + } + + private static void fireValidationEvent(String calledMethodName) { + listeners.forEach(l -> l.validationCalled(calledMethodName)); + } + + @RequestFeatureValidator( + conditions = { CLUSTER_NEEDS_FINALIZATION }, + processingPhase = PRE_PROCESS, + requestType = CreateKey) + public static OMRequest preFinalizePreProcessCreateKeyValidator( + OMRequest req, ValidationContext ctx){ + fireValidationEvent("preFinalizePreProcessCreateKeyValidator"); + return req; + } + + @RequestFeatureValidator( + conditions = { CLUSTER_NEEDS_FINALIZATION }, + processingPhase = POST_PROCESS, + requestType = CreateKey) + public static OMResponse preFinalizePostProcessCreateKeyValidator( + OMRequest req, OMResponse resp, ValidationContext ctx){ + fireValidationEvent("preFinalizePostProcessCreateKeyValidator"); + return resp; + } + + @RequestFeatureValidator( + conditions = { NEWER_CLIENT_REQUESTS }, + processingPhase = PRE_PROCESS, + requestType = CreateKey) + public static OMRequest newClientPreProcessCreateKeyValidator( + OMRequest req, ValidationContext ctx){ + fireValidationEvent("newClientPreProcessCreateKeyValidator"); + return req; + } + + @RequestFeatureValidator( + conditions = { NEWER_CLIENT_REQUESTS }, + processingPhase = POST_PROCESS, + requestType = CreateKey) + public static OMResponse newClientPostProcessCreateKeyValidator( + OMRequest req, OMResponse resp, ValidationContext ctx){ + fireValidationEvent("newClientPostProcessCreateKeyValidator"); + return resp; + } + + @RequestFeatureValidator( + conditions = { OLDER_CLIENT_REQUESTS }, + processingPhase = PRE_PROCESS, + requestType = CreateKey) + public static OMRequest oldClientPreProcessCreateKeyValidator( + OMRequest req, ValidationContext ctx){ + fireValidationEvent("oldClientPreProcessCreateKeyValidator"); + return req; + } + + @RequestFeatureValidator( + conditions = { OLDER_CLIENT_REQUESTS }, + processingPhase = POST_PROCESS, + requestType = CreateKey) + public static OMResponse oldClientPostProcessCreateKeyValidator( + OMRequest req, OMResponse resp, ValidationContext ctx){ + fireValidationEvent("oldClientPostProcessCreateKeyValidator"); + return resp; + } + + @RequestFeatureValidator( + conditions = { UNCONDITIONAL }, + processingPhase = PRE_PROCESS, + requestType = CreateKey) + public static OMRequest unconditionalPreProcessCreateKeyValidator( + OMRequest req, ValidationContext ctx){ + fireValidationEvent("unconditionalPreProcessCreateKeyValidator"); + return req; + } + + @RequestFeatureValidator( + conditions = { UNCONDITIONAL }, + processingPhase = POST_PROCESS, + requestType = CreateKey) + public static OMResponse unconditionalPostProcessCreateKeyValidator( + OMRequest req, OMResponse resp, ValidationContext ctx){ + fireValidationEvent("unconditionalPostProcessCreateKeyValidator"); + return resp; + } + + @RequestFeatureValidator( + conditions = { CLUSTER_NEEDS_FINALIZATION, OLDER_CLIENT_REQUESTS }, + processingPhase = PRE_PROCESS, + requestType = CreateVolume) + public static OMRequest multiPurposePreProcessCreateVolumeValidator( + OMRequest req, ValidationContext ctx){ + fireValidationEvent("multiPurposePreProcessCreateVolumeValidator"); + return req; + } + + @RequestFeatureValidator( + conditions = { OLDER_CLIENT_REQUESTS, UNCONDITIONAL, + CLUSTER_NEEDS_FINALIZATION }, + processingPhase = POST_PROCESS, + requestType = CreateVolume) + public static OMResponse multiPurposePostProcessCreateVolumeValidator( + OMRequest req, OMResponse resp, ValidationContext ctx){ + fireValidationEvent("multiPurposePostProcessCreateVolumeValidator"); + return resp; + } + + @RequestFeatureValidator( + conditions = { NEWER_CLIENT_REQUESTS }, + processingPhase = POST_PROCESS, + requestType = CreateKey) + public static OMResponse newClientPostProcessCreateKeyValidator2( + OMRequest req, OMResponse resp, ValidationContext ctx){ + fireValidationEvent("newClientPostProcessCreateKeyValidator2"); + return resp; + } + + @RequestFeatureValidator( + conditions = {UNCONDITIONAL}, + processingPhase = PRE_PROCESS, + requestType = DeleteKeys + ) + public static OMRequest throwingPreProcessValidator( + OMRequest req, ValidationContext ctx) throws IOException { + fireValidationEvent("throwingPreProcessValidator"); + throw new IOException("IOException: fail for testing..."); + } + + @RequestFeatureValidator( + conditions = {UNCONDITIONAL}, + processingPhase = POST_PROCESS, + requestType = DeleteKeys + ) + public static OMResponse throwingPostProcessValidator( + OMRequest req, OMResponse resp, ValidationContext ctx) + throws IOException { + fireValidationEvent("throwingPostProcessValidator"); + throw new IOException("IOException: fail for testing..."); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidator.java similarity index 98% rename from hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java rename to hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidator.java index 4a5522addecc..1c82ebb15cc6 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validations/TestRequestFeatureValidator.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidator.java @@ -14,13 +14,10 @@ * License for the specific language governing permissions and limitations under * the License. */ -package org.apache.hadoop.ozone.om.request.validations; +package org.apache.hadoop.ozone.om.request.validation; import com.google.testing.compile.Compilation; import com.google.testing.compile.JavaFileObjects; -import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; -import org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase; -import org.apache.hadoop.ozone.om.request.validation.ValidationCondition; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; import org.apache.ozone.annotations.RequestFeatureValidatorProcessor; import org.junit.Test; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java new file mode 100644 index 000000000000..a671cbdc20d8 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java @@ -0,0 +1,416 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.hadoop.ozone.om.request.validation; + +import com.google.protobuf.ServiceException; +import org.apache.hadoop.ozone.om.request.validation.GeneralValidatorsForTesting.ValidationListener; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; +import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import sun.jvm.hotspot.utilities.AssertionFailure; + +import java.util.ArrayList; +import java.util.List; + +import static org.apache.hadoop.ozone.om.request.validation.ValidationContext.of; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.*; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.DeleteKeys; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.RenameKey; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class TestRequestValidations { + private static final String PACKAGE = + "org.apache.hadoop.ozone.om.request.validation"; + + private static final String PACKAGE_WO_VALIDATORS = + "org.apache.hadoop.hdds.annotation"; + + private final ValidationListenerImpl validationListener = + new ValidationListenerImpl(); + + @Before + public void setup() { + validationListener.attach(); + } + + @After + public void tearDown() { + validationListener.detach(); + } + + @Test(expected = NullPointerException.class) + public void testUsingRegistryWithoutLoading() throws ServiceException { + new RequestValidations() + .fromPackage(PACKAGE) + .withinContext(of(aFinalizedVersionManager(), 0)) + .validateRequest(aCreateKeyRequest(0)); + } + + @Test(expected = NullPointerException.class) + public void testUsingRegistryWithoutContext() throws ServiceException { + new RequestValidations() + .fromPackage(PACKAGE) + .load() + .validateRequest(aCreateKeyRequest(0)); + } + + @Test + public void testUsingRegistryWithoutPackage() throws ServiceException { + new RequestValidations() + .withinContext(of(aFinalizedVersionManager(), 0)) + .load() + .validateRequest(aCreateKeyRequest(0)); + + validationListener.assertNumOfEvents(1); + validationListener + .assertCalled("unconditionalPreProcessCreateKeyValidator"); + } + + @Test + public void testNoPreValdiationsWithoutValidationMethods() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadEmptyValidations(ctx); + + validations.validateRequest(aCreateKeyRequest(omVersion)); + + validationListener.assertNumOfEvents(0); + } + + @Test + public void testNoPostValdiationsWithoutValidationMethods() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadEmptyValidations(ctx); + + validations + .validateResponse(aCreateKeyRequest(omVersion), aCreateKeyResponse()); + + validationListener.assertNumOfEvents(0); + } + + @Test + public void testNoPreValidationsRunningForRequestTypeWithoutValidators() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateRequest(aRenameKeyRequest(omVersion)); + + validationListener.assertNumOfEvents(0); + } + + @Test + public void testNoPostValidationsAreRunningForRequestTypeWithoutValidators() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations. + validateResponse(aRenameKeyRequest(omVersion), aRenameKeyResponse()); + + validationListener.assertNumOfEvents(0); + } + + @Test + public void testUnconditionalPreProcessValidationsAreCalled() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateRequest(aCreateKeyRequest(omVersion)); + + validationListener.assertNumOfEvents(1); + validationListener + .assertCalled("unconditionalPreProcessCreateKeyValidator"); + } + + @Test + public void testUnconditionalPostProcessValidationsAreCalled() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations. + validateResponse(aCreateKeyRequest(omVersion), aCreateKeyResponse()); + + validationListener.assertNumOfEvents(1); + validationListener + .assertCalled("unconditionalPostProcessCreateKeyValidator"); + } + + @Test + public void testPreProcessorExceptionHandling() { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + try { + validations.validateRequest(aDeleteKeysRequest(omVersion)); + Assert.fail("ServiceException was expected but was not thrown."); + } catch (ServiceException ignored) {} + + validationListener.assertNumOfEvents(1); + validationListener.assertCalled("throwingPreProcessValidator"); + } + + @Test + public void testPostProcessorExceptionHandling() { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + try { + validations.validateResponse( + aDeleteKeysRequest(omVersion), aDeleteKeysResponse()); + Assert.fail("ServiceException was expected but was not thrown."); + } catch (ServiceException ignored) {} + + validationListener.assertNumOfEvents(1); + validationListener.assertCalled("throwingPostProcessValidator"); + } + + @Test + public void testNewClientConditionIsRecognizedAndPreValidatorsApplied() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateRequest(aCreateKeyRequest(omVersion + 1)); + + validationListener.assertNumOfEvents(2); + validationListener.assertAllCalled( + "unconditionalPreProcessCreateKeyValidator", + "newClientPreProcessCreateKeyValidator"); + } + + @Test + public void testNewClientConditionIsRecognizedAndPostValidatorsApplied() + throws ServiceException { + int omVersion = 0; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateResponse( + aCreateKeyRequest(omVersion + 1), aCreateKeyResponse()); + + validationListener.assertNumOfEvents(3); + validationListener.assertAllCalled( + "unconditionalPostProcessCreateKeyValidator", + "newClientPostProcessCreateKeyValidator", + "newClientPostProcessCreateKeyValidator2"); + } + + @Test + public void testOldClientConditionIsRecognizedAndPreValidatorsApplied() + throws ServiceException { + int omVersion = 2; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateRequest(aCreateKeyRequest(omVersion - 1)); + + validationListener.assertNumOfEvents(2); + validationListener.assertAllCalled( + "unconditionalPreProcessCreateKeyValidator", + "oldClientPreProcessCreateKeyValidator"); + } + + @Test + public void testOldClientConditionIsRecognizedAndPostValidatorsApplied() + throws ServiceException { + int omVersion = 2; + ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateResponse( + aCreateKeyRequest(omVersion - 1), aCreateKeyResponse()); + + validationListener.assertNumOfEvents(2); + validationListener.assertAllCalled( + "unconditionalPostProcessCreateKeyValidator", + "oldClientPostProcessCreateKeyValidator"); + } + + @Test + public void + testPreFinalizedWithOldClientConditionIsRecognizedAndPreValidatorsApplied() + throws ServiceException { + int omVersion = 2; + ValidationContext ctx = of(anUnfinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateRequest(aCreateKeyRequest(omVersion - 1)); + + validationListener.assertNumOfEvents(3); + validationListener.assertAllCalled( + "unconditionalPreProcessCreateKeyValidator", + "preFinalizePreProcessCreateKeyValidator", + "oldClientPreProcessCreateKeyValidator"); + } + + @Test + public void + testPreFinalizedWithOldClientConditionIsRecognizedAndPostValidatorsApplied() + throws ServiceException { + int omVersion = 2; + ValidationContext ctx = of(anUnfinalizedVersionManager(), omVersion); + RequestValidations validations = loadValidations(ctx); + + validations.validateResponse( + aCreateKeyRequest(omVersion - 1), aCreateKeyResponse()); + + validationListener.assertNumOfEvents(3); + validationListener.assertAllCalled( + "unconditionalPostProcessCreateKeyValidator", + "preFinalizePostProcessCreateKeyValidator", + "oldClientPostProcessCreateKeyValidator"); + } + + private RequestValidations loadValidations(ValidationContext ctx) { + return new RequestValidations() + .fromPackage(PACKAGE) + .withinContext(ctx) + .load(); + } + + private RequestValidations loadEmptyValidations(ValidationContext ctx) { + return new RequestValidations() + .fromPackage(PACKAGE_WO_VALIDATORS) + .withinContext(ctx) + .load(); + } + + private OMRequest aCreateKeyRequest(int version) { + return aRequest(CreateKey, version); + } + + private OMRequest aDeleteKeysRequest(int version) { + return aRequest(DeleteKeys, version); + } + + private OMRequest aRenameKeyRequest(int version) { + return aRequest(RenameKey, version); + } + + private OMRequest aRequest(Type type, int version) { + return OMRequest.newBuilder() + .setVersion(version) + .setCmdType(type) + .setClientId("TestClient") + .build(); + } + + private OMResponse aCreateKeyResponse() { + return aResponse(CreateKey); + } + + private OMResponse aDeleteKeysResponse() { + return aResponse(DeleteKeys); + } + + private OMResponse aRenameKeyResponse() { + return aResponse(RenameKey); + } + + private OMResponse aResponse(Type type) { + return OMResponse.newBuilder() + .setCmdType(type) + .setStatus(OK) + .build(); + } + + private LayoutVersionManager aFinalizedVersionManager() { + LayoutVersionManager vm = mock(LayoutVersionManager.class); + when(vm.needsFinalization()).thenReturn(false); + return vm; + } + + private LayoutVersionManager anUnfinalizedVersionManager() { + LayoutVersionManager vm = mock(LayoutVersionManager.class); + when(vm.needsFinalization()).thenReturn(true); + return vm; + } + + private static class ValidationListenerImpl implements ValidationListener { + List calledMethods = new ArrayList<>(); + + @Override + public void validationCalled(String calledMethodName) { + calledMethods.add(calledMethodName); + } + + public void attach(){ + GeneralValidatorsForTesting.addListener(this); + } + + public void detach() { + GeneralValidatorsForTesting.removeListener(this); + reset(); + } + + public void reset() { + calledMethods = new ArrayList<>(); + } + + public void assertCalled(String... methodNames) { + for (String methodName : methodNames) { + if (!calledMethods.contains(methodName)) { + throw new AssertionFailure("Expected method call for " + methodName + + " did not happened."); + } + } + } + + public void assertAllCalled(String... methodNames) { + List calls = new ArrayList<>(calledMethods); + for (String methodName : methodNames) { + if (!calls.remove(methodName)) { + throw new AssertionFailure("Expected method call for " + methodName + + " did not happened."); + } + } + if (!calls.isEmpty()) { + throw new AssertionFailure("Some of the methods were not called." + + "Missing calls for: " + calls); + } + } + + public void assertNumOfEvents(int count) { + if (calledMethods.size() != count) { + throw new AssertionFailure("Unexpected validation call count." + + " Expected: " + count + "; Happened: " + calledMethods.size()); + } + } + } +} + + diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java new file mode 100644 index 000000000000..c212144ea676 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java @@ -0,0 +1,262 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.hadoop.ozone.om.request.validation; + +import org.junit.Test; +import org.reflections.util.ClasspathHelper; + +import java.lang.reflect.Method; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.stream.Collectors; + +import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.POST_PROCESS; +import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.PRE_PROCESS; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.CLUSTER_NEEDS_FINALIZATION; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.NEWER_CLIENT_REQUESTS; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.OLDER_CLIENT_REQUESTS; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.UNCONDITIONAL; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateDirectory; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateVolume; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Validator registry tests. + * For validator method declarations see the GeneralValidatorsForTesting + * and ValidatorsForOnlyNewClientValidations (in ../avalidation2) classes. + */ +public class TestValidatorRegistry { + private static final String PACKAGE = + "org.apache.hadoop.ozone.om.request.validation"; + + private static final String PACKAGE2 = + "org.apache.hadoop.ozone.om.request.validation2"; + + private static final String PACKAGE_WO_VALIDATORS = + "org.apache.hadoop.hdds.annotation"; + + @Test + public void testNoValidatorsReturnedForEmptyConditionList() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor(emptyList(), CreateKey, PRE_PROCESS); + + assertTrue(validators.isEmpty()); + } + + @Test + public void testRegistryHasThePreFinalizePreProcessCreateKeyValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(CLUSTER_NEEDS_FINALIZATION), CreateKey, PRE_PROCESS); + + assertEquals(1, validators.size()); + String expectedMethodName = "preFinalizePreProcessCreateKeyValidator"; + assertEquals(expectedMethodName, validators.get(0).getName()); + } + + @Test + public void testRegistryHasThePreFinalizePostProcessCreateKeyValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(CLUSTER_NEEDS_FINALIZATION), CreateKey, POST_PROCESS); + + assertEquals(1, validators.size()); + String expectedMethodName = "preFinalizePostProcessCreateKeyValidator"; + assertEquals(expectedMethodName, validators.get(0).getName()); + } + + @Test + public void testRegistryHasTheNewClientPreProcessCreateKeyValidator() + throws MalformedURLException { + Collection urls = ClasspathHelper.forPackage(PACKAGE); + Collection urlsToUse = new ArrayList<>(); + for (URL url : urls) { + urlsToUse.add(new URL(url, PACKAGE.replaceAll("\\.", "/"))); + } + ValidatorRegistry registry = new ValidatorRegistry(urlsToUse); + List validators = + registry.validationsFor( + asList(NEWER_CLIENT_REQUESTS), CreateKey, PRE_PROCESS); + + assertEquals(1, validators.size()); + String expectedMethodName = "newClientPreProcessCreateKeyValidator"; + assertEquals(expectedMethodName, validators.get(0).getName()); + } + + @Test + public void testRegistryHasTheNewClientPostProcessCreateKeyValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(NEWER_CLIENT_REQUESTS), CreateKey, POST_PROCESS); + + assertEquals(2, validators.size()); + List methodNames = + validators.stream().map(Method::getName).collect(Collectors.toList()); + assertTrue( + methodNames.contains("newClientPostProcessCreateKeyValidator")); + assertTrue( + methodNames.contains("newClientPostProcessCreateKeyValidator2")); + } + + @Test + public void testRegistryHasTheOldClientPreProcessCreateKeyValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(OLDER_CLIENT_REQUESTS), CreateKey, PRE_PROCESS); + + assertEquals(1, validators.size()); + String expectedMethodName = "oldClientPreProcessCreateKeyValidator"; + assertEquals(expectedMethodName, validators.get(0).getName()); + } + + @Test + public void testRegistryHasTheOldClientPostProcessCreateKeyValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(OLDER_CLIENT_REQUESTS), CreateKey, POST_PROCESS); + + assertEquals(1, validators.size()); + String expectedMethodName = "oldClientPostProcessCreateKeyValidator"; + assertEquals(expectedMethodName, validators.get(0).getName()); + } + + @Test + public void testRegistryHasTheUnconditionalPreProcessCreateKeyValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(UNCONDITIONAL), CreateKey, PRE_PROCESS); + + assertEquals(1, validators.size()); + String expectedMethodName = "unconditionalPreProcessCreateKeyValidator"; + assertEquals(expectedMethodName, validators.get(0).getName()); + } + + @Test + public void testRegistryHasTheUnconditionalPostProcessCreateKeyValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(UNCONDITIONAL), CreateKey, POST_PROCESS); + + assertEquals(1, validators.size()); + String expectedMethodName = "unconditionalPostProcessCreateKeyValidator"; + assertEquals(expectedMethodName, validators.get(0).getName()); + } + + @Test + public void testRegistryHasTheMultiPurposePreProcessCreateVolumeValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List preFinalizeValidators = + registry.validationsFor( + asList(CLUSTER_NEEDS_FINALIZATION), CreateVolume, PRE_PROCESS); + List newClientValidators = + registry.validationsFor( + asList(OLDER_CLIENT_REQUESTS), CreateVolume, PRE_PROCESS); + + assertEquals(1, preFinalizeValidators.size()); + assertEquals(1, newClientValidators.size()); + String expectedMethodName = "multiPurposePreProcessCreateVolumeValidator"; + assertEquals(expectedMethodName, preFinalizeValidators.get(0).getName()); + assertEquals(expectedMethodName, newClientValidators.get(0).getName()); + } + + @Test + public void testRegistryHasTheMultiPurposePostProcessCreateVolumeValidator() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List preFinalizeValidators = + registry.validationsFor( + asList(CLUSTER_NEEDS_FINALIZATION), CreateVolume, POST_PROCESS); + List oldClientValidators = + registry.validationsFor( + asList(OLDER_CLIENT_REQUESTS), CreateVolume, POST_PROCESS); + List unconditionalValidators = + registry.validationsFor( + asList(UNCONDITIONAL), CreateVolume, POST_PROCESS); + + assertEquals(1, preFinalizeValidators.size()); + assertEquals(1, oldClientValidators.size()); + assertEquals(1, unconditionalValidators.size()); + String expectedMethodName = "multiPurposePostProcessCreateVolumeValidator"; + assertEquals(expectedMethodName, preFinalizeValidators.get(0).getName()); + assertEquals(expectedMethodName, oldClientValidators.get(0).getName()); + assertEquals(expectedMethodName, unconditionalValidators.get(0).getName()); + } + + @Test + public void testValidatorsAreReturnedForMultiCondition(){ + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + List validators = + registry.validationsFor( + asList(CLUSTER_NEEDS_FINALIZATION, NEWER_CLIENT_REQUESTS), + CreateKey, POST_PROCESS); + + assertEquals(3, validators.size()); + List methodNames = + validators.stream().map(Method::getName).collect(Collectors.toList()); + assertTrue( + methodNames.contains("preFinalizePostProcessCreateKeyValidator")); + assertTrue( + methodNames.contains("newClientPostProcessCreateKeyValidator")); + assertTrue( + methodNames.contains("newClientPostProcessCreateKeyValidator2")); + } + + @Test + public void testNoValidatorForRequestsAtAllReturnsEmptyList() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE_WO_VALIDATORS); + + assertTrue(registry.validationsFor( + asList(UNCONDITIONAL), CreateKey, PRE_PROCESS).isEmpty()); + } + + @Test + public void testNoValidatorForConditionReturnsEmptyList() + throws MalformedURLException { + Collection urls = ClasspathHelper.forPackage(PACKAGE2); + Collection urlsToUse = new ArrayList<>(); + for (URL url : urls) { + urlsToUse.add(new URL(url, PACKAGE2.replaceAll("\\.", "/"))); + } + ValidatorRegistry registry = new ValidatorRegistry(urlsToUse); + + assertTrue(registry.validationsFor( + asList(UNCONDITIONAL), CreateKey, PRE_PROCESS).isEmpty()); + } + + @Test + public void testNoDefinedValidationForRequestReturnsEmptyList() { + ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); + + assertTrue(registry.validationsFor( + asList(UNCONDITIONAL), CreateDirectory, null).isEmpty()); + } + +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java new file mode 100644 index 000000000000..1d62375101d0 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.hadoop.ozone.om.request.validation2; + +import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; +import org.apache.hadoop.ozone.om.request.validation.ValidationContext; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; + +import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.PRE_PROCESS; +import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.NEWER_CLIENT_REQUESTS; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; + +public class ValidatorsForOnlyNewClientValidations { + + @RequestFeatureValidator( + conditions = { NEWER_CLIENT_REQUESTS }, + processingPhase = PRE_PROCESS, + requestType = CreateKey) + public static OMRequest newClientPostProcessCreateKeyValidator2( + OMRequest req, ValidationContext ctx){ + return req; + } +} From f9ced3c8090a332c4084ae2d8b728214d352c5d9 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 12:13:05 +0100 Subject: [PATCH 15/36] Rename annotation processor test. --- ...Validator.java => TestRequestFeatureValidatorProcessor.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/{TestRequestFeatureValidator.java => TestRequestFeatureValidatorProcessor.java} (99%) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidatorProcessor.java similarity index 99% rename from hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidator.java rename to hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidatorProcessor.java index 1c82ebb15cc6..3584d326968c 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidator.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidatorProcessor.java @@ -41,7 +41,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; -public class TestRequestFeatureValidator { +public class TestRequestFeatureValidatorProcessor { private static final String CLASSNAME = "Validation"; From 99d017959b64e19811b2317cd858a0cf60e50393 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 19:38:38 +0100 Subject: [PATCH 16/36] Removed serverversion, and use ClientVersions to decide on the client age. --- .../validation/RequestValidations.java | 5 +++-- .../request/validation/ValidationContext.java | 19 ++++--------------- ...ManagerProtocolServerSideTranslatorPB.java | 9 +-------- 3 files changed, 8 insertions(+), 25 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index 2baf196f2b84..92df0fa73a7c 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.om.request.validation; import com.google.protobuf.ServiceException; +import org.apache.hadoop.ozone.ClientVersions; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -91,8 +92,8 @@ public OMResponse validateResponse(OMRequest request, OMResponse response) private List conditions(OMRequest request) { List conditions = new LinkedList<>(); conditions.add(ValidationCondition.UNCONDITIONAL); - if (context.serverVersion() != request.getVersion()) { - if (context.serverVersion() < request.getVersion()) { + if (ClientVersions.CURRENT_VERSION != request.getVersion()) { + if (ClientVersions.CURRENT_VERSION < request.getVersion()) { conditions.add(ValidationCondition.NEWER_CLIENT_REQUESTS); } else { conditions.add(ValidationCondition.OLDER_CLIENT_REQUESTS); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java index ce8cecf79c57..510d4336983a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationContext.java @@ -16,11 +16,13 @@ */ package org.apache.hadoop.ozone.om.request.validation; +import org.apache.hadoop.hdds.annotation.InterfaceStability; import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; /** * A context that contains useful information for request validator instances. */ +@InterfaceStability.Evolving public interface ValidationContext { /** @@ -32,32 +34,19 @@ public interface ValidationContext { */ LayoutVersionManager versionManager(); - /** - * Provides the protocol version of the server side running. - * @return the server side protocol version - */ - int serverVersion(); - /** * Creates a context object based on the given parameters. * * @param versionManager the {@link LayoutVersionManager} of the service - * @param serverVersion the server side protocol version * @return the {@link ValidationContext} specified by the parameters. */ - static ValidationContext of( - LayoutVersionManager versionManager, - int serverVersion) { + static ValidationContext of(LayoutVersionManager versionManager) { + return new ValidationContext() { @Override public LayoutVersionManager versionManager() { return versionManager; } - - @Override - public int serverVersion() { - return serverVersion; - } }; } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java index e731a3c54557..06aa6c546a3b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/protocolPB/OzoneManagerProtocolServerSideTranslatorPB.java @@ -121,9 +121,7 @@ public OzoneManagerProtocolServerSideTranslatorPB( new RequestValidations() .fromPackage(OM_REQUESTS_PACKAGE) .withinContext( - ValidationContext.of( - ozoneManager.getVersionManager(), - getServerProtocolVersion())) + ValidationContext.of(ozoneManager.getVersionManager())) .load(); } @@ -142,11 +140,6 @@ public OMResponse submitRequest(RpcController controller, return requestValidations.validateResponse(request, response); } - //TODO find out versioning and where it comes from... - private int getServerProtocolVersion() { - return 0; - } - private OMResponse processRequest(OMRequest request) throws ServiceException { if (isRatisEnabled) { From d7fed1ec13a1491c3fdb4e1452984ef473d07756 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 22:57:39 +0100 Subject: [PATCH 17/36] Fix checkstyle issues, and a test compilation issue caused be the removal of server version. --- .../GeneralValidatorsForTesting.java | 39 +++-- .../TestRequestFeatureValidatorProcessor.java | 7 + .../validation/TestRequestValidations.java | 147 +++++++++--------- .../validation/TestValidatorRegistry.java | 2 +- ...ValidatorsForOnlyNewClientValidations.java | 10 +- 5 files changed, 115 insertions(+), 90 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java index bb876d08b471..818077f6879b 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java @@ -33,8 +33,21 @@ import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateVolume; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.DeleteKeys; -public class GeneralValidatorsForTesting { +/** + * Some annotated request validator method, and facilities to help check if + * validations were properly called from tests where applicable. + */ +public final class GeneralValidatorsForTesting { + + private GeneralValidatorsForTesting() { } + /** + * Interface to easily add listeners that get notified if a certain validator + * method defined in this class was called. + * + * @see TestRequestValidations for more details on how this intercace is + * being used. + */ @FunctionalInterface public interface ValidationListener { void validationCalled(String calledMethodName); @@ -42,7 +55,7 @@ public interface ValidationListener { private static List listeners = new ArrayList<>(); - public static void addListener(ValidationListener listener){ + public static void addListener(ValidationListener listener) { listeners.add(listener); } @@ -59,7 +72,7 @@ private static void fireValidationEvent(String calledMethodName) { processingPhase = PRE_PROCESS, requestType = CreateKey) public static OMRequest preFinalizePreProcessCreateKeyValidator( - OMRequest req, ValidationContext ctx){ + OMRequest req, ValidationContext ctx) { fireValidationEvent("preFinalizePreProcessCreateKeyValidator"); return req; } @@ -69,7 +82,7 @@ public static OMRequest preFinalizePreProcessCreateKeyValidator( processingPhase = POST_PROCESS, requestType = CreateKey) public static OMResponse preFinalizePostProcessCreateKeyValidator( - OMRequest req, OMResponse resp, ValidationContext ctx){ + OMRequest req, OMResponse resp, ValidationContext ctx) { fireValidationEvent("preFinalizePostProcessCreateKeyValidator"); return resp; } @@ -79,7 +92,7 @@ public static OMResponse preFinalizePostProcessCreateKeyValidator( processingPhase = PRE_PROCESS, requestType = CreateKey) public static OMRequest newClientPreProcessCreateKeyValidator( - OMRequest req, ValidationContext ctx){ + OMRequest req, ValidationContext ctx) { fireValidationEvent("newClientPreProcessCreateKeyValidator"); return req; } @@ -89,7 +102,7 @@ public static OMRequest newClientPreProcessCreateKeyValidator( processingPhase = POST_PROCESS, requestType = CreateKey) public static OMResponse newClientPostProcessCreateKeyValidator( - OMRequest req, OMResponse resp, ValidationContext ctx){ + OMRequest req, OMResponse resp, ValidationContext ctx) { fireValidationEvent("newClientPostProcessCreateKeyValidator"); return resp; } @@ -99,7 +112,7 @@ public static OMResponse newClientPostProcessCreateKeyValidator( processingPhase = PRE_PROCESS, requestType = CreateKey) public static OMRequest oldClientPreProcessCreateKeyValidator( - OMRequest req, ValidationContext ctx){ + OMRequest req, ValidationContext ctx) { fireValidationEvent("oldClientPreProcessCreateKeyValidator"); return req; } @@ -109,7 +122,7 @@ public static OMRequest oldClientPreProcessCreateKeyValidator( processingPhase = POST_PROCESS, requestType = CreateKey) public static OMResponse oldClientPostProcessCreateKeyValidator( - OMRequest req, OMResponse resp, ValidationContext ctx){ + OMRequest req, OMResponse resp, ValidationContext ctx) { fireValidationEvent("oldClientPostProcessCreateKeyValidator"); return resp; } @@ -119,7 +132,7 @@ public static OMResponse oldClientPostProcessCreateKeyValidator( processingPhase = PRE_PROCESS, requestType = CreateKey) public static OMRequest unconditionalPreProcessCreateKeyValidator( - OMRequest req, ValidationContext ctx){ + OMRequest req, ValidationContext ctx) { fireValidationEvent("unconditionalPreProcessCreateKeyValidator"); return req; } @@ -129,7 +142,7 @@ public static OMRequest unconditionalPreProcessCreateKeyValidator( processingPhase = POST_PROCESS, requestType = CreateKey) public static OMResponse unconditionalPostProcessCreateKeyValidator( - OMRequest req, OMResponse resp, ValidationContext ctx){ + OMRequest req, OMResponse resp, ValidationContext ctx) { fireValidationEvent("unconditionalPostProcessCreateKeyValidator"); return resp; } @@ -139,7 +152,7 @@ public static OMResponse unconditionalPostProcessCreateKeyValidator( processingPhase = PRE_PROCESS, requestType = CreateVolume) public static OMRequest multiPurposePreProcessCreateVolumeValidator( - OMRequest req, ValidationContext ctx){ + OMRequest req, ValidationContext ctx) { fireValidationEvent("multiPurposePreProcessCreateVolumeValidator"); return req; } @@ -150,7 +163,7 @@ public static OMRequest multiPurposePreProcessCreateVolumeValidator( processingPhase = POST_PROCESS, requestType = CreateVolume) public static OMResponse multiPurposePostProcessCreateVolumeValidator( - OMRequest req, OMResponse resp, ValidationContext ctx){ + OMRequest req, OMResponse resp, ValidationContext ctx) { fireValidationEvent("multiPurposePostProcessCreateVolumeValidator"); return resp; } @@ -160,7 +173,7 @@ public static OMResponse multiPurposePostProcessCreateVolumeValidator( processingPhase = POST_PROCESS, requestType = CreateKey) public static OMResponse newClientPostProcessCreateKeyValidator2( - OMRequest req, OMResponse resp, ValidationContext ctx){ + OMRequest req, OMResponse resp, ValidationContext ctx) { fireValidationEvent("newClientPostProcessCreateKeyValidator2"); return resp; } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidatorProcessor.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidatorProcessor.java index 3584d326968c..2ba0f0986e9d 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidatorProcessor.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestFeatureValidatorProcessor.java @@ -41,6 +41,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertSame; +/** + * Compile tests against the annotation processor for the + * {@link RequestFeatureValidator} annotation. + * + * The processor should ensure the method signatures and return values, based + * on annotation arguments provided. + */ public class TestRequestFeatureValidatorProcessor { private static final String CLASSNAME = "Validation"; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java index a671cbdc20d8..ace398f239ec 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java @@ -17,16 +17,15 @@ package org.apache.hadoop.ozone.om.request.validation; import com.google.protobuf.ServiceException; +import org.apache.hadoop.ozone.ClientVersions; import org.apache.hadoop.ozone.om.request.validation.GeneralValidatorsForTesting.ValidationListener; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; import org.apache.hadoop.ozone.upgrade.LayoutVersionManager; import org.junit.After; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import sun.jvm.hotspot.utilities.AssertionFailure; import java.util.ArrayList; import java.util.List; @@ -36,9 +35,14 @@ import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.DeleteKeys; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.RenameKey; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +/** + * Testing the RequestValidations class that is used to run the validation for + * any given request that arrives to OzoneManager. + */ public class TestRequestValidations { private static final String PACKAGE = "org.apache.hadoop.ozone.om.request.validation"; @@ -63,8 +67,8 @@ public void tearDown() { public void testUsingRegistryWithoutLoading() throws ServiceException { new RequestValidations() .fromPackage(PACKAGE) - .withinContext(of(aFinalizedVersionManager(), 0)) - .validateRequest(aCreateKeyRequest(0)); + .withinContext(of(aFinalizedVersionManager())) + .validateRequest(aCreateKeyRequest(currentClientVersion())); } @Test(expected = NullPointerException.class) @@ -72,15 +76,15 @@ public void testUsingRegistryWithoutContext() throws ServiceException { new RequestValidations() .fromPackage(PACKAGE) .load() - .validateRequest(aCreateKeyRequest(0)); + .validateRequest(aCreateKeyRequest(currentClientVersion())); } @Test public void testUsingRegistryWithoutPackage() throws ServiceException { new RequestValidations() - .withinContext(of(aFinalizedVersionManager(), 0)) + .withinContext(of(aFinalizedVersionManager())) .load() - .validateRequest(aCreateKeyRequest(0)); + .validateRequest(aCreateKeyRequest(currentClientVersion())); validationListener.assertNumOfEvents(1); validationListener @@ -91,7 +95,7 @@ public void testUsingRegistryWithoutPackage() throws ServiceException { public void testNoPreValdiationsWithoutValidationMethods() throws ServiceException { int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadEmptyValidations(ctx); validations.validateRequest(aCreateKeyRequest(omVersion)); @@ -102,12 +106,11 @@ public void testNoPreValdiationsWithoutValidationMethods() @Test public void testNoPostValdiationsWithoutValidationMethods() throws ServiceException { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadEmptyValidations(ctx); - validations - .validateResponse(aCreateKeyRequest(omVersion), aCreateKeyResponse()); + validations.validateResponse( + aCreateKeyRequest(currentClientVersion()), aCreateKeyResponse()); validationListener.assertNumOfEvents(0); } @@ -115,11 +118,10 @@ public void testNoPostValdiationsWithoutValidationMethods() @Test public void testNoPreValidationsRunningForRequestTypeWithoutValidators() throws ServiceException { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); - validations.validateRequest(aRenameKeyRequest(omVersion)); + validations.validateRequest(aRenameKeyRequest(currentClientVersion())); validationListener.assertNumOfEvents(0); } @@ -127,12 +129,11 @@ public void testNoPreValidationsRunningForRequestTypeWithoutValidators() @Test public void testNoPostValidationsAreRunningForRequestTypeWithoutValidators() throws ServiceException { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); - validations. - validateResponse(aRenameKeyRequest(omVersion), aRenameKeyResponse()); + validations.validateResponse( + aRenameKeyRequest(currentClientVersion()), aRenameKeyResponse()); validationListener.assertNumOfEvents(0); } @@ -140,11 +141,10 @@ public void testNoPostValidationsAreRunningForRequestTypeWithoutValidators() @Test public void testUnconditionalPreProcessValidationsAreCalled() throws ServiceException { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); - validations.validateRequest(aCreateKeyRequest(omVersion)); + validations.validateRequest(aCreateKeyRequest(currentClientVersion())); validationListener.assertNumOfEvents(1); validationListener @@ -154,12 +154,11 @@ public void testUnconditionalPreProcessValidationsAreCalled() @Test public void testUnconditionalPostProcessValidationsAreCalled() throws ServiceException { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); - validations. - validateResponse(aCreateKeyRequest(omVersion), aCreateKeyResponse()); + validations.validateResponse( + aCreateKeyRequest(currentClientVersion()), aCreateKeyResponse()); validationListener.assertNumOfEvents(1); validationListener @@ -168,14 +167,13 @@ public void testUnconditionalPostProcessValidationsAreCalled() @Test public void testPreProcessorExceptionHandling() { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); try { - validations.validateRequest(aDeleteKeysRequest(omVersion)); - Assert.fail("ServiceException was expected but was not thrown."); - } catch (ServiceException ignored) {} + validations.validateRequest(aDeleteKeysRequest(currentClientVersion())); + fail("ServiceException was expected but was not thrown."); + } catch (ServiceException ignored) { } validationListener.assertNumOfEvents(1); validationListener.assertCalled("throwingPreProcessValidator"); @@ -183,15 +181,14 @@ public void testPreProcessorExceptionHandling() { @Test public void testPostProcessorExceptionHandling() { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); try { validations.validateResponse( - aDeleteKeysRequest(omVersion), aDeleteKeysResponse()); - Assert.fail("ServiceException was expected but was not thrown."); - } catch (ServiceException ignored) {} + aDeleteKeysRequest(currentClientVersion()), aDeleteKeysResponse()); + fail("ServiceException was expected but was not thrown."); + } catch (ServiceException ignored) { } validationListener.assertNumOfEvents(1); validationListener.assertCalled("throwingPostProcessValidator"); @@ -200,11 +197,10 @@ public void testPostProcessorExceptionHandling() { @Test public void testNewClientConditionIsRecognizedAndPreValidatorsApplied() throws ServiceException { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); - validations.validateRequest(aCreateKeyRequest(omVersion + 1)); + validations.validateRequest(aCreateKeyRequest(newerClientVersion())); validationListener.assertNumOfEvents(2); validationListener.assertAllCalled( @@ -215,12 +211,11 @@ public void testNewClientConditionIsRecognizedAndPreValidatorsApplied() @Test public void testNewClientConditionIsRecognizedAndPostValidatorsApplied() throws ServiceException { - int omVersion = 0; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); validations.validateResponse( - aCreateKeyRequest(omVersion + 1), aCreateKeyResponse()); + aCreateKeyRequest(newerClientVersion()), aCreateKeyResponse()); validationListener.assertNumOfEvents(3); validationListener.assertAllCalled( @@ -232,11 +227,10 @@ public void testNewClientConditionIsRecognizedAndPostValidatorsApplied() @Test public void testOldClientConditionIsRecognizedAndPreValidatorsApplied() throws ServiceException { - int omVersion = 2; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); - validations.validateRequest(aCreateKeyRequest(omVersion - 1)); + validations.validateRequest(aCreateKeyRequest(olderClientVersion())); validationListener.assertNumOfEvents(2); validationListener.assertAllCalled( @@ -247,12 +241,11 @@ public void testOldClientConditionIsRecognizedAndPreValidatorsApplied() @Test public void testOldClientConditionIsRecognizedAndPostValidatorsApplied() throws ServiceException { - int omVersion = 2; - ValidationContext ctx = of(aFinalizedVersionManager(), omVersion); + ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); validations.validateResponse( - aCreateKeyRequest(omVersion - 1), aCreateKeyResponse()); + aCreateKeyRequest(olderClientVersion()), aCreateKeyResponse()); validationListener.assertNumOfEvents(2); validationListener.assertAllCalled( @@ -261,14 +254,12 @@ public void testOldClientConditionIsRecognizedAndPostValidatorsApplied() } @Test - public void - testPreFinalizedWithOldClientConditionIsRecognizedAndPreValidatorsApplied() + public void testPreFinalizedWithOldClientConditionPreProcValidatorsApplied() throws ServiceException { - int omVersion = 2; - ValidationContext ctx = of(anUnfinalizedVersionManager(), omVersion); + ValidationContext ctx = of(anUnfinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); - validations.validateRequest(aCreateKeyRequest(omVersion - 1)); + validations.validateRequest(aCreateKeyRequest(olderClientVersion())); validationListener.assertNumOfEvents(3); validationListener.assertAllCalled( @@ -278,15 +269,13 @@ public void testOldClientConditionIsRecognizedAndPostValidatorsApplied() } @Test - public void - testPreFinalizedWithOldClientConditionIsRecognizedAndPostValidatorsApplied() + public void testPreFinalizedWithOldClientConditionPostProcValidatorsApplied() throws ServiceException { - int omVersion = 2; - ValidationContext ctx = of(anUnfinalizedVersionManager(), omVersion); + ValidationContext ctx = of(anUnfinalizedVersionManager()); RequestValidations validations = loadValidations(ctx); validations.validateResponse( - aCreateKeyRequest(omVersion - 1), aCreateKeyResponse()); + aCreateKeyRequest(olderClientVersion()), aCreateKeyResponse()); validationListener.assertNumOfEvents(3); validationListener.assertAllCalled( @@ -309,21 +298,33 @@ private RequestValidations loadEmptyValidations(ValidationContext ctx) { .load(); } - private OMRequest aCreateKeyRequest(int version) { - return aRequest(CreateKey, version); + private int olderClientVersion() { + return ClientVersions.CURRENT_VERSION - 1; + } + + private int currentClientVersion() { + return ClientVersions.CURRENT_VERSION; + } + + private int newerClientVersion() { + return ClientVersions.CURRENT_VERSION + 1; + } + + private OMRequest aCreateKeyRequest(int clientVersion) { + return aRequest(CreateKey, clientVersion); } - private OMRequest aDeleteKeysRequest(int version) { - return aRequest(DeleteKeys, version); + private OMRequest aDeleteKeysRequest(int clientVersion) { + return aRequest(DeleteKeys, clientVersion); } - private OMRequest aRenameKeyRequest(int version) { - return aRequest(RenameKey, version); + private OMRequest aRenameKeyRequest(int clientVersion) { + return aRequest(RenameKey, clientVersion); } - private OMRequest aRequest(Type type, int version) { + private OMRequest aRequest(Type type, int clientVersion) { return OMRequest.newBuilder() - .setVersion(version) + .setVersion(clientVersion) .setCmdType(type) .setClientId("TestClient") .build(); @@ -361,14 +362,14 @@ private LayoutVersionManager anUnfinalizedVersionManager() { } private static class ValidationListenerImpl implements ValidationListener { - List calledMethods = new ArrayList<>(); + private List calledMethods = new ArrayList<>(); @Override public void validationCalled(String calledMethodName) { calledMethods.add(calledMethodName); } - public void attach(){ + public void attach() { GeneralValidatorsForTesting.addListener(this); } @@ -384,8 +385,7 @@ public void reset() { public void assertCalled(String... methodNames) { for (String methodName : methodNames) { if (!calledMethods.contains(methodName)) { - throw new AssertionFailure("Expected method call for " + methodName - + " did not happened."); + fail("Expected method call for " + methodName + " did not happened."); } } } @@ -394,19 +394,18 @@ public void assertAllCalled(String... methodNames) { List calls = new ArrayList<>(calledMethods); for (String methodName : methodNames) { if (!calls.remove(methodName)) { - throw new AssertionFailure("Expected method call for " + methodName - + " did not happened."); + fail("Expected method call for " + methodName + " did not happened."); } } if (!calls.isEmpty()) { - throw new AssertionFailure("Some of the methods were not called." + fail("Some of the methods were not called." + "Missing calls for: " + calls); } } public void assertNumOfEvents(int count) { if (calledMethods.size() != count) { - throw new AssertionFailure("Unexpected validation call count." + fail("Unexpected validation call count." + " Expected: " + count + "; Happened: " + calledMethods.size()); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java index c212144ea676..8a668d16f9de 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java @@ -211,7 +211,7 @@ public void testRegistryHasTheMultiPurposePostProcessCreateVolumeValidator() { } @Test - public void testValidatorsAreReturnedForMultiCondition(){ + public void testValidatorsAreReturnedForMultiCondition() { ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); List validators = registry.validationsFor( diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java index 1d62375101d0..3ca72fecd863 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java @@ -24,14 +24,20 @@ import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.NEWER_CLIENT_REQUESTS; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; -public class ValidatorsForOnlyNewClientValidations { +/** + * Separate validator methods for a few specific tests that covers cases where + * there are almost no validators added. + */ +public final class ValidatorsForOnlyNewClientValidations { + + private ValidatorsForOnlyNewClientValidations() { } @RequestFeatureValidator( conditions = { NEWER_CLIENT_REQUESTS }, processingPhase = PRE_PROCESS, requestType = CreateKey) public static OMRequest newClientPostProcessCreateKeyValidator2( - OMRequest req, ValidationContext ctx){ + OMRequest req, ValidationContext ctx) { return req; } } From 01c948948f2117c5e81c7518b57ab280ba0eb992 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 23:08:29 +0100 Subject: [PATCH 18/36] Added spotbugs related info so that findbugs check can run. --- dev-support/annotations/pom.xml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/dev-support/annotations/pom.xml b/dev-support/annotations/pom.xml index 51ef06d7da80..e851ecf6d6e0 100644 --- a/dev-support/annotations/pom.xml +++ b/dev-support/annotations/pom.xml @@ -16,10 +16,30 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> UTF-8 UTF-8 + 3.1.12 + + + com.github.spotbugs + spotbugs + ${spotbugs.version} + provided + + + + + com.github.spotbugs + spotbugs-maven-plugin + ${spotbugs.version} + + 1024 + true + true + + org.apache.maven.plugins maven-compiler-plugin From 9a0258c5107dae9d54bb1ef44d66eea67b0f217a Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 23:47:12 +0100 Subject: [PATCH 19/36] Adjust rat to check the annotation processor module as well. --- hadoop-ozone/dev-support/checks/rat.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hadoop-ozone/dev-support/checks/rat.sh b/hadoop-ozone/dev-support/checks/rat.sh index 464d636f9381..b67c594cd62d 100755 --- a/hadoop-ozone/dev-support/checks/rat.sh +++ b/hadoop-ozone/dev-support/checks/rat.sh @@ -21,7 +21,9 @@ mkdir -p "$REPORT_DIR" REPORT_FILE="$REPORT_DIR/summary.txt" -cd hadoop-hdds || exit 1 +cd dev-support/annotations || exit 1 +mvn -B -fn org.apache.rat:apache-rat-plugin:0.13:check +cd ../../hadoop-hdds || exit 1 mvn -B -fn org.apache.rat:apache-rat-plugin:0.13:check cd ../hadoop-ozone || exit 1 mvn -B -fn org.apache.rat:apache-rat-plugin:0.13:check From 51e9f5ae0db6f843ae4335b15668c27a577a27cc Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 23:49:02 +0100 Subject: [PATCH 20/36] Update the jar-report.txt --- .../dist/src/main/license/jar-report.txt | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/hadoop-ozone/dist/src/main/license/jar-report.txt b/hadoop-ozone/dist/src/main/license/jar-report.txt index ec949fdf33a4..0c10e77d5f2d 100644 --- a/hadoop-ozone/dist/src/main/license/jar-report.txt +++ b/hadoop-ozone/dist/src/main/license/jar-report.txt @@ -1,10 +1,11 @@ +share/ozone/lib/FastInfoset.jar share/ozone/lib/accessors-smart.jar share/ozone/lib/activation.jar share/ozone/lib/animal-sniffer-annotations.jar share/ozone/lib/annotations.jar share/ozone/lib/annotations.jar -share/ozone/lib/aopalliance.jar share/ozone/lib/aopalliance-repackaged.jar +share/ozone/lib/aopalliance.jar share/ozone/lib/asm.jar share/ozone/lib/aspectjrt.jar share/ozone/lib/aspectjweaver.jar @@ -37,21 +38,20 @@ share/ozone/lib/disruptor.jar share/ozone/lib/dnsjava.jar share/ozone/lib/error_prone_annotations.jar share/ozone/lib/failureaccess.jar -share/ozone/lib/FastInfoset.jar share/ozone/lib/grpc-api.jar share/ozone/lib/grpc-context.jar share/ozone/lib/grpc-core.jar share/ozone/lib/grpc-netty.jar -share/ozone/lib/grpc-protobuf.jar share/ozone/lib/grpc-protobuf-lite.jar +share/ozone/lib/grpc-protobuf.jar share/ozone/lib/grpc-stub.jar share/ozone/lib/gson.jar share/ozone/lib/guava-jre.jar share/ozone/lib/guice-assistedinject.jar share/ozone/lib/guice-bridge.jar -share/ozone/lib/guice.jar share/ozone/lib/guice-multibindings.jar share/ozone/lib/guice-servlet.jar +share/ozone/lib/guice.jar share/ozone/lib/hadoop-annotations.jar share/ozone/lib/hadoop-auth.jar share/ozone/lib/hadoop-common.jar @@ -133,8 +133,8 @@ share/ozone/lib/jetty-xml.jar share/ozone/lib/jmespath-java.jar share/ozone/lib/joda-time.jar share/ozone/lib/jooq-codegen.jar -share/ozone/lib/jooq.jar share/ozone/lib/jooq-meta.jar +share/ozone/lib/jooq.jar share/ozone/lib/jsch.jar share/ozone/lib/json-smart.jar share/ozone/lib/jsp-api.jar @@ -164,17 +164,17 @@ share/ozone/lib/log4j.jar share/ozone/lib/metainf-services.jar share/ozone/lib/metrics-core.jar share/ozone/lib/netty-buffer.Final.jar -share/ozone/lib/netty-codec.Final.jar -share/ozone/lib/netty-codec-http2.Final.jar share/ozone/lib/netty-codec-http.Final.jar +share/ozone/lib/netty-codec-http2.Final.jar share/ozone/lib/netty-codec-socks.Final.jar +share/ozone/lib/netty-codec.Final.jar share/ozone/lib/netty-common.Final.jar -share/ozone/lib/netty-handler.Final.jar share/ozone/lib/netty-handler-proxy.Final.jar +share/ozone/lib/netty-handler.Final.jar share/ozone/lib/netty-resolver.Final.jar -share/ozone/lib/netty-transport.Final.jar share/ozone/lib/netty-transport-native-epoll.Final.jar share/ozone/lib/netty-transport-native-unix-common.Final.jar +share/ozone/lib/netty-transport.Final.jar share/ozone/lib/nimbus-jose-jwt.jar share/ozone/lib/okhttp.jar share/ozone/lib/okio.jar @@ -183,6 +183,7 @@ share/ozone/lib/opentracing-noop.jar share/ozone/lib/opentracing-tracerresolver.jar share/ozone/lib/opentracing-util.jar share/ozone/lib/osgi-resource-locator.jar +share/ozone/lib/ozone-annotation-processing.jar share/ozone/lib/ozone-client.jar share/ozone/lib/ozone-common.jar share/ozone/lib/ozone-csi.jar @@ -195,16 +196,16 @@ share/ozone/lib/ozone-insight.jar share/ozone/lib/ozone-interface-client.jar share/ozone/lib/ozone-interface-storage.jar share/ozone/lib/ozone-manager.jar -share/ozone/lib/ozone-reconcodegen.jar share/ozone/lib/ozone-recon.jar +share/ozone/lib/ozone-reconcodegen.jar share/ozone/lib/ozone-s3gateway.jar share/ozone/lib/ozone-tools.jar share/ozone/lib/perfmark-api.jar share/ozone/lib/picocli.jar +share/ozone/lib/proto-google-common-protos.jar +share/ozone/lib/protobuf-java-util.jar share/ozone/lib/protobuf-java.jar share/ozone/lib/protobuf-java.jar -share/ozone/lib/protobuf-java-util.jar -share/ozone/lib/proto-google-common-protos.jar share/ozone/lib/ratis-client.jar share/ozone/lib/ratis-common.jar share/ozone/lib/ratis-grpc.jar @@ -218,9 +219,9 @@ share/ozone/lib/ratis-tools.jar share/ozone/lib/re2j.jar share/ozone/lib/reflections.jar share/ozone/lib/rocksdbjni.jar +share/ozone/lib/simpleclient.jar share/ozone/lib/simpleclient_common.jar share/ozone/lib/simpleclient_dropwizard.jar -share/ozone/lib/simpleclient.jar share/ozone/lib/slf4j-api.jar share/ozone/lib/slf4j-log4j12.jar share/ozone/lib/snakeyaml.jar @@ -231,8 +232,8 @@ share/ozone/lib/spring-jcl.RELEASE.jar share/ozone/lib/spring-jdbc.RELEASE.jar share/ozone/lib/spring-tx.RELEASE.jar share/ozone/lib/sqlite-jdbc.jar -share/ozone/lib/stax2-api.jar share/ozone/lib/stax-ex.jar +share/ozone/lib/stax2-api.jar share/ozone/lib/token-provider.jar share/ozone/lib/txw2.jar share/ozone/lib/weld-servlet.Final.jar From 267b92f66ee44842709da15e9e171996e6e4340e Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 23:50:06 +0100 Subject: [PATCH 21/36] Extend some documentation, fix findbugs reported synchronization problem, remove unnecessary extra compile step from annotation processing module. --- dev-support/annotations/pom.xml | 19 ++++++++++++------- .../validation/RequestValidations.java | 2 +- pom.xml | 4 ++++ 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/dev-support/annotations/pom.xml b/dev-support/annotations/pom.xml index e851ecf6d6e0..e834ead52aa8 100644 --- a/dev-support/annotations/pom.xml +++ b/dev-support/annotations/pom.xml @@ -30,6 +30,10 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> + com.github.spotbugs spotbugs-maven-plugin @@ -40,6 +44,14 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> true + org.apache.maven.plugins maven-compiler-plugin @@ -58,13 +70,6 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> - - compile-project - compile - - compile - - diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index 92df0fa73a7c..06aece8f262f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -41,7 +41,7 @@ public class RequestValidations { private ValidationContext context = null; private ValidatorRegistry registry = null; - public RequestValidations fromPackage(String packageName) { + public synchronized RequestValidations fromPackage(String packageName) { validationsPackageName = packageName; return this; } diff --git a/pom.xml b/pom.xml index ffb32464f393..22064257cb13 100644 --- a/pom.xml +++ b/pom.xml @@ -164,6 +164,10 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs 1.1.1 3.0.0 + 3.1.12 2.1.7 From b27c17dced73cea852f6f6937334fa023fd76697 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Wed, 16 Feb 2022 23:57:56 +0100 Subject: [PATCH 22/36] Fix missing licenses after rat check adjustments. --- dev-support/annotations/pom.xml | 14 ++++++++++++++ .../javax.annotation.processing.Processor | 15 +++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/dev-support/annotations/pom.xml b/dev-support/annotations/pom.xml index e834ead52aa8..f24ac4b7dcbb 100644 --- a/dev-support/annotations/pom.xml +++ b/dev-support/annotations/pom.xml @@ -1,3 +1,17 @@ + + Date: Thu, 17 Feb 2022 00:43:40 +0100 Subject: [PATCH 23/36] Revert "Update the jar-report.txt" This reverts commit 585c0a700e310254c49a0d530be4ccfddd714dfc. --- .../dist/src/main/license/jar-report.txt | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/hadoop-ozone/dist/src/main/license/jar-report.txt b/hadoop-ozone/dist/src/main/license/jar-report.txt index 0c10e77d5f2d..ec949fdf33a4 100644 --- a/hadoop-ozone/dist/src/main/license/jar-report.txt +++ b/hadoop-ozone/dist/src/main/license/jar-report.txt @@ -1,11 +1,10 @@ -share/ozone/lib/FastInfoset.jar share/ozone/lib/accessors-smart.jar share/ozone/lib/activation.jar share/ozone/lib/animal-sniffer-annotations.jar share/ozone/lib/annotations.jar share/ozone/lib/annotations.jar -share/ozone/lib/aopalliance-repackaged.jar share/ozone/lib/aopalliance.jar +share/ozone/lib/aopalliance-repackaged.jar share/ozone/lib/asm.jar share/ozone/lib/aspectjrt.jar share/ozone/lib/aspectjweaver.jar @@ -38,20 +37,21 @@ share/ozone/lib/disruptor.jar share/ozone/lib/dnsjava.jar share/ozone/lib/error_prone_annotations.jar share/ozone/lib/failureaccess.jar +share/ozone/lib/FastInfoset.jar share/ozone/lib/grpc-api.jar share/ozone/lib/grpc-context.jar share/ozone/lib/grpc-core.jar share/ozone/lib/grpc-netty.jar -share/ozone/lib/grpc-protobuf-lite.jar share/ozone/lib/grpc-protobuf.jar +share/ozone/lib/grpc-protobuf-lite.jar share/ozone/lib/grpc-stub.jar share/ozone/lib/gson.jar share/ozone/lib/guava-jre.jar share/ozone/lib/guice-assistedinject.jar share/ozone/lib/guice-bridge.jar +share/ozone/lib/guice.jar share/ozone/lib/guice-multibindings.jar share/ozone/lib/guice-servlet.jar -share/ozone/lib/guice.jar share/ozone/lib/hadoop-annotations.jar share/ozone/lib/hadoop-auth.jar share/ozone/lib/hadoop-common.jar @@ -133,8 +133,8 @@ share/ozone/lib/jetty-xml.jar share/ozone/lib/jmespath-java.jar share/ozone/lib/joda-time.jar share/ozone/lib/jooq-codegen.jar -share/ozone/lib/jooq-meta.jar share/ozone/lib/jooq.jar +share/ozone/lib/jooq-meta.jar share/ozone/lib/jsch.jar share/ozone/lib/json-smart.jar share/ozone/lib/jsp-api.jar @@ -164,17 +164,17 @@ share/ozone/lib/log4j.jar share/ozone/lib/metainf-services.jar share/ozone/lib/metrics-core.jar share/ozone/lib/netty-buffer.Final.jar -share/ozone/lib/netty-codec-http.Final.jar +share/ozone/lib/netty-codec.Final.jar share/ozone/lib/netty-codec-http2.Final.jar +share/ozone/lib/netty-codec-http.Final.jar share/ozone/lib/netty-codec-socks.Final.jar -share/ozone/lib/netty-codec.Final.jar share/ozone/lib/netty-common.Final.jar -share/ozone/lib/netty-handler-proxy.Final.jar share/ozone/lib/netty-handler.Final.jar +share/ozone/lib/netty-handler-proxy.Final.jar share/ozone/lib/netty-resolver.Final.jar +share/ozone/lib/netty-transport.Final.jar share/ozone/lib/netty-transport-native-epoll.Final.jar share/ozone/lib/netty-transport-native-unix-common.Final.jar -share/ozone/lib/netty-transport.Final.jar share/ozone/lib/nimbus-jose-jwt.jar share/ozone/lib/okhttp.jar share/ozone/lib/okio.jar @@ -183,7 +183,6 @@ share/ozone/lib/opentracing-noop.jar share/ozone/lib/opentracing-tracerresolver.jar share/ozone/lib/opentracing-util.jar share/ozone/lib/osgi-resource-locator.jar -share/ozone/lib/ozone-annotation-processing.jar share/ozone/lib/ozone-client.jar share/ozone/lib/ozone-common.jar share/ozone/lib/ozone-csi.jar @@ -196,16 +195,16 @@ share/ozone/lib/ozone-insight.jar share/ozone/lib/ozone-interface-client.jar share/ozone/lib/ozone-interface-storage.jar share/ozone/lib/ozone-manager.jar -share/ozone/lib/ozone-recon.jar share/ozone/lib/ozone-reconcodegen.jar +share/ozone/lib/ozone-recon.jar share/ozone/lib/ozone-s3gateway.jar share/ozone/lib/ozone-tools.jar share/ozone/lib/perfmark-api.jar share/ozone/lib/picocli.jar -share/ozone/lib/proto-google-common-protos.jar -share/ozone/lib/protobuf-java-util.jar share/ozone/lib/protobuf-java.jar share/ozone/lib/protobuf-java.jar +share/ozone/lib/protobuf-java-util.jar +share/ozone/lib/proto-google-common-protos.jar share/ozone/lib/ratis-client.jar share/ozone/lib/ratis-common.jar share/ozone/lib/ratis-grpc.jar @@ -219,9 +218,9 @@ share/ozone/lib/ratis-tools.jar share/ozone/lib/re2j.jar share/ozone/lib/reflections.jar share/ozone/lib/rocksdbjni.jar -share/ozone/lib/simpleclient.jar share/ozone/lib/simpleclient_common.jar share/ozone/lib/simpleclient_dropwizard.jar +share/ozone/lib/simpleclient.jar share/ozone/lib/slf4j-api.jar share/ozone/lib/slf4j-log4j12.jar share/ozone/lib/snakeyaml.jar @@ -232,8 +231,8 @@ share/ozone/lib/spring-jcl.RELEASE.jar share/ozone/lib/spring-jdbc.RELEASE.jar share/ozone/lib/spring-tx.RELEASE.jar share/ozone/lib/sqlite-jdbc.jar -share/ozone/lib/stax-ex.jar share/ozone/lib/stax2-api.jar +share/ozone/lib/stax-ex.jar share/ozone/lib/token-provider.jar share/ozone/lib/txw2.jar share/ozone/lib/weld-servlet.Final.jar From 24911aa82f4a34b8c321733a5d7dbaf334bcecba Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 17 Feb 2022 00:46:03 +0100 Subject: [PATCH 24/36] Update the jar-report.txt --- hadoop-ozone/dist/src/main/license/jar-report.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-ozone/dist/src/main/license/jar-report.txt b/hadoop-ozone/dist/src/main/license/jar-report.txt index ec949fdf33a4..da8350d4b77a 100644 --- a/hadoop-ozone/dist/src/main/license/jar-report.txt +++ b/hadoop-ozone/dist/src/main/license/jar-report.txt @@ -183,6 +183,7 @@ share/ozone/lib/opentracing-noop.jar share/ozone/lib/opentracing-tracerresolver.jar share/ozone/lib/opentracing-util.jar share/ozone/lib/osgi-resource-locator.jar +share/ozone/lib/ozone-annotation-processing.jar share/ozone/lib/ozone-client.jar share/ozone/lib/ozone-common.jar share/ozone/lib/ozone-csi.jar From d1c899a9d7da49620219d21e4fb3a51223c54262 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 17 Feb 2022 19:05:42 +0100 Subject: [PATCH 25/36] Move the decision about whether a condition is present to the condition itself, with that generalize the validations side logic. --- .../validation/RequestValidations.java | 19 ++++-------- .../validation/ValidationCondition.java | 30 ++++++++++++++++--- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index 06aece8f262f..a87a5d226480 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -23,8 +23,10 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.LinkedList; import java.util.List; +import java.util.stream.Collectors; import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.POST_PROCESS; import static org.apache.hadoop.ozone.om.request.validation.RequestProcessingPhase.PRE_PROCESS; @@ -90,20 +92,9 @@ public OMResponse validateResponse(OMRequest request, OMResponse response) } private List conditions(OMRequest request) { - List conditions = new LinkedList<>(); - conditions.add(ValidationCondition.UNCONDITIONAL); - if (ClientVersions.CURRENT_VERSION != request.getVersion()) { - if (ClientVersions.CURRENT_VERSION < request.getVersion()) { - conditions.add(ValidationCondition.NEWER_CLIENT_REQUESTS); - } else { - conditions.add(ValidationCondition.OLDER_CLIENT_REQUESTS); - } - } - if (context.versionManager() != null - && context.versionManager().needsFinalization()) { - conditions.add(ValidationCondition.CLUSTER_NEEDS_FINALIZATION); - } - return conditions; + return Arrays.stream(ValidationCondition.values()) + .filter(c -> c.shouldApply(request, context)) + .collect(Collectors.toList()); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java index fb61a4af5fba..fce01a146da8 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -16,6 +16,11 @@ */ package org.apache.hadoop.ozone.om.request.validation; +import org.apache.hadoop.ozone.ClientVersions; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; + +import java.util.function.Predicate; + /** * Defines conditions for which validators can be assigned to. * @@ -30,21 +35,38 @@ public enum ValidationCondition { * Classifies validations that has to run after an upgrade until the cluster * is in a pre-finalized state. */ - CLUSTER_NEEDS_FINALIZATION, + CLUSTER_NEEDS_FINALIZATION(r -> true, + ctx -> ctx.versionManager().needsFinalization()), /** * Classifies validations that has to run, when the client uses an older * protocol version than the server. */ - OLDER_CLIENT_REQUESTS, + OLDER_CLIENT_REQUESTS(r -> r.getVersion() < ClientVersions.CURRENT_VERSION, + ctx -> true), /** * Classifies validations that has to run, when the client uses a newer * protocol version than the server. */ - NEWER_CLIENT_REQUESTS, + NEWER_CLIENT_REQUESTS(r -> r.getVersion() > ClientVersions.CURRENT_VERSION, + ctx -> true), /** * Classifies validations that has to run for every request. * If you plan to use this, please justify why the validation code should not * be part of the actual request handling code. */ - UNCONDITIONAL; + UNCONDITIONAL(r -> true, ctx -> true); + + private Predicate shouldApplyTo; + private Predicate shouldApplyIn; + + ValidationCondition( + Predicate shouldApplyTo, + Predicate shouldApplyIn) { + this.shouldApplyTo = shouldApplyTo; + this.shouldApplyIn = shouldApplyIn; + } + + boolean shouldApply(OMRequest req, ValidationContext ctx) { + return shouldApplyTo.test(req) && shouldApplyIn.test(ctx); + } } From 959e4a372e5b769e45bc5933efba2f04592ecc2a Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 17 Feb 2022 19:09:24 +0100 Subject: [PATCH 26/36] make predicates final in validaitonconditions enum. --- .../ozone/om/request/validation/ValidationCondition.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java index fce01a146da8..c7e6791c8831 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -56,8 +56,8 @@ public enum ValidationCondition { */ UNCONDITIONAL(r -> true, ctx -> true); - private Predicate shouldApplyTo; - private Predicate shouldApplyIn; + private final Predicate shouldApplyTo; + private final Predicate shouldApplyIn; ValidationCondition( Predicate shouldApplyTo, From 3637271ae1e2a22d38be8ec8244390a417ed6fb7 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 17 Feb 2022 20:24:18 +0100 Subject: [PATCH 27/36] Remove unused imports from RequestValidations. --- .../hadoop/ozone/om/request/validation/RequestValidations.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index a87a5d226480..429888ee2144 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -17,14 +17,12 @@ package org.apache.hadoop.ozone.om.request.validation; import com.google.protobuf.ServiceException; -import org.apache.hadoop.ozone.ClientVersions; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; -import java.util.LinkedList; import java.util.List; import java.util.stream.Collectors; From a414aefdceed3e20ebf9f6e512044ad502519f29 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 17 Feb 2022 20:25:52 +0100 Subject: [PATCH 28/36] Fix wording in apidoc. Co-authored-by: Ritesh H Shukla --- .../ozone/om/request/validation/RequestFeatureValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java index 1bb00c9a960a..5f1f08266b85 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestFeatureValidator.java @@ -24,7 +24,7 @@ import java.lang.annotation.Target; /** - * An annotation to mark methods that does certain request validations. + * An annotation to mark methods that do certain request validations. * * The methods annotated with this annotation are collected by the * {@link ValidatorRegistry} class during the initialization of the server. From b66c55bc85457609fb13651916d648181b6b3a92 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 17 Feb 2022 20:28:43 +0100 Subject: [PATCH 29/36] Add note about -proc:none in doc comment inside annotation procesor module's pom. --- dev-support/annotations/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-support/annotations/pom.xml b/dev-support/annotations/pom.xml index f24ac4b7dcbb..80b8b38e6f60 100644 --- a/dev-support/annotations/pom.xml +++ b/dev-support/annotations/pom.xml @@ -64,7 +64,7 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> | as due to the presence of | META-INF/services/javac.annotation.processing.Processor the compilation | would fail as the linked processor class can not be find while we are - | compiling it. + | compiling it, hence the compiler argument -proc:none is specified here. --> org.apache.maven.plugins From c8d4d97d225b193017d5b6aac6f02a942132055a Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Tue, 22 Feb 2022 01:45:33 +0100 Subject: [PATCH 30/36] Address CI issues caused by the test validators. Re-organize the validator's packages to be encapsulated within the tests' package. --- .../validation/TestRequestValidations.java | 8 ++++-- .../validation/TestValidatorRegistry.java | 17 +++++++++-- .../GeneralValidatorsForTesting.java | 28 +++++++++++++++++-- ...ValidatorsForOnlyNewClientValidations.java | 2 +- 4 files changed, 47 insertions(+), 8 deletions(-) rename hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/{ => testvalidatorset1}/GeneralValidatorsForTesting.java (87%) rename hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/{validation2 => validation/testvalidatorset2}/ValidatorsForOnlyNewClientValidations.java (96%) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java index ace398f239ec..5ef26ddbdf53 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java @@ -18,7 +18,8 @@ import com.google.protobuf.ServiceException; import org.apache.hadoop.ozone.ClientVersions; -import org.apache.hadoop.ozone.om.request.validation.GeneralValidatorsForTesting.ValidationListener; +import org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting; +import org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.ValidationListener; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type; @@ -31,6 +32,7 @@ import java.util.List; import static org.apache.hadoop.ozone.om.request.validation.ValidationContext.of; +import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.validatorTestsRunning; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.*; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.DeleteKeys; @@ -45,7 +47,7 @@ */ public class TestRequestValidations { private static final String PACKAGE = - "org.apache.hadoop.ozone.om.request.validation"; + "org.apache.hadoop.ozone.om.request.validation.testvalidatorset1"; private static final String PACKAGE_WO_VALIDATORS = "org.apache.hadoop.hdds.annotation"; @@ -55,12 +57,14 @@ public class TestRequestValidations { @Before public void setup() { + validatorTestsRunning = true; validationListener.attach(); } @After public void tearDown() { validationListener.detach(); + validatorTestsRunning = false; } @Test(expected = NullPointerException.class) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java index 8a668d16f9de..7c7cd6a4b236 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java @@ -16,6 +16,8 @@ */ package org.apache.hadoop.ozone.om.request.validation; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.reflections.util.ClasspathHelper; @@ -35,6 +37,7 @@ import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.NEWER_CLIENT_REQUESTS; import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.OLDER_CLIENT_REQUESTS; import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.UNCONDITIONAL; +import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.validatorTestsRunning; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateDirectory; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateVolume; @@ -48,14 +51,24 @@ */ public class TestValidatorRegistry { private static final String PACKAGE = - "org.apache.hadoop.ozone.om.request.validation"; + "org.apache.hadoop.ozone.om.request.validation.testvalidatorset1"; private static final String PACKAGE2 = - "org.apache.hadoop.ozone.om.request.validation2"; + "org.apache.hadoop.ozone.om.request.validation.testvalidatorset2"; private static final String PACKAGE_WO_VALIDATORS = "org.apache.hadoop.hdds.annotation"; + @Before + public void setup() { + validatorTestsRunning = true; + } + + @After + public void tearDown() { + validatorTestsRunning = false; + } + @Test public void testNoValidatorsReturnedForEmptyConditionList() { ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java similarity index 87% rename from hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java rename to hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java index 818077f6879b..bc7c8a49abd1 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/GeneralValidatorsForTesting.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java @@ -14,8 +14,11 @@ * License for the specific language governing permissions and limitations under * the License. */ -package org.apache.hadoop.ozone.om.request.validation; +package org.apache.hadoop.ozone.om.request.validation.testvalidatorset1; +import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; +import org.apache.hadoop.ozone.om.request.validation.TestRequestValidations; +import org.apache.hadoop.ozone.om.request.validation.ValidationContext; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; @@ -39,6 +42,18 @@ */ public final class GeneralValidatorsForTesting { + /** + * As the validators written here does not override any request or response + * but throw exceptions for specific tests, a test that wants to directly + * use a validator here, has to turn on this boolean, and the method that + * the test relies on has to check for this value. + * + * This is necessary to do not affect other tests that are testing requests + * processing, as for some of those tests this package is on the classpath + * and therefore the annotated validations are loadede for them. + */ + public static boolean validatorTestsRunning = false; + private GeneralValidatorsForTesting() { } /** @@ -186,7 +201,10 @@ public static OMResponse newClientPostProcessCreateKeyValidator2( public static OMRequest throwingPreProcessValidator( OMRequest req, ValidationContext ctx) throws IOException { fireValidationEvent("throwingPreProcessValidator"); - throw new IOException("IOException: fail for testing..."); + if (validatorTestsRunning) { + throw new IOException("IOException: fail for testing..."); + } + return req; } @RequestFeatureValidator( @@ -198,6 +216,10 @@ public static OMResponse throwingPostProcessValidator( OMRequest req, OMResponse resp, ValidationContext ctx) throws IOException { fireValidationEvent("throwingPostProcessValidator"); - throw new IOException("IOException: fail for testing..."); + if (validatorTestsRunning) { + throw new IOException("IOException: fail for testing..."); + } + return resp; } + } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset2/ValidatorsForOnlyNewClientValidations.java similarity index 96% rename from hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java rename to hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset2/ValidatorsForOnlyNewClientValidations.java index 3ca72fecd863..2ba34f48c54e 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation2/ValidatorsForOnlyNewClientValidations.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset2/ValidatorsForOnlyNewClientValidations.java @@ -14,7 +14,7 @@ * License for the specific language governing permissions and limitations under * the License. */ -package org.apache.hadoop.ozone.om.request.validation2; +package org.apache.hadoop.ozone.om.request.validation.testvalidatorset2; import org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator; import org.apache.hadoop.ozone.om.request.validation.ValidationContext; From 18a0e96a218a7a8df29a3fef731aac935116ef2a Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Tue, 22 Feb 2022 01:46:30 +0100 Subject: [PATCH 31/36] Add debug logging to expose in the logs which validator methods were invoked during request processing. --- .../ozone/om/request/validation/RequestValidations.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java index 429888ee2144..fe34b746094f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java @@ -19,6 +19,8 @@ import com.google.protobuf.ServiceException; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; @@ -35,6 +37,7 @@ */ public class RequestValidations { + static final Logger LOG = LoggerFactory.getLogger(RequestValidations.class); private static final String DEFAULT_PACKAGE = "org.apache.hadoop.ozone"; private String validationsPackageName = DEFAULT_PACKAGE; @@ -65,6 +68,9 @@ public OMRequest validateRequest(OMRequest request) throws ServiceException { for (Method m : validations) { validatedRequest = (OMRequest) m.invoke(null, validatedRequest, context); + LOG.debug("Running the {} request pre-process validation from {}.{}", + m.getName(), m.getDeclaringClass().getPackage().getName(), + m.getDeclaringClass().getSimpleName()); } } catch (IllegalAccessException | InvocationTargetException e) { throw new ServiceException(e); @@ -82,6 +88,9 @@ public OMResponse validateResponse(OMRequest request, OMResponse response) for (Method m : validations) { validatedResponse = (OMResponse) m.invoke(null, request, response, context); + LOG.debug("Running the {} request post-process validation from {}.{}", + m.getName(), m.getDeclaringClass().getPackage().getName(), + m.getDeclaringClass().getSimpleName()); } } catch (InvocationTargetException | IllegalAccessException e) { throw new ServiceException(e); From e82145dd5b762e8df51b1c85b334dd9afdd9ddf4 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Tue, 22 Feb 2022 02:28:11 +0100 Subject: [PATCH 32/36] Address checkstyle issues. --- .../om/request/validation/TestRequestValidations.java | 9 +++++---- .../om/request/validation/TestValidatorRegistry.java | 7 ++++--- .../testvalidatorset1/GeneralValidatorsForTesting.java | 10 +++++++++- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java index 5ef26ddbdf53..195e3ce32da6 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java @@ -32,8 +32,9 @@ import java.util.List; import static org.apache.hadoop.ozone.om.request.validation.ValidationContext.of; -import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.validatorTestsRunning; -import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.*; +import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.startValidatorTest; +import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.finishValidatorTest; +import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status.OK; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.DeleteKeys; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.RenameKey; @@ -57,14 +58,14 @@ public class TestRequestValidations { @Before public void setup() { - validatorTestsRunning = true; + startValidatorTest(); validationListener.attach(); } @After public void tearDown() { validationListener.detach(); - validatorTestsRunning = false; + finishValidatorTest(); } @Test(expected = NullPointerException.class) diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java index 7c7cd6a4b236..8a921a7ccba8 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java @@ -37,7 +37,8 @@ import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.NEWER_CLIENT_REQUESTS; import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.OLDER_CLIENT_REQUESTS; import static org.apache.hadoop.ozone.om.request.validation.ValidationCondition.UNCONDITIONAL; -import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.validatorTestsRunning; +import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.startValidatorTest; +import static org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.finishValidatorTest; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateDirectory; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateKey; import static org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Type.CreateVolume; @@ -61,12 +62,12 @@ public class TestValidatorRegistry { @Before public void setup() { - validatorTestsRunning = true; + startValidatorTest(); } @After public void tearDown() { - validatorTestsRunning = false; + finishValidatorTest(); } @Test diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java index bc7c8a49abd1..e4be6291ba3f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java @@ -52,7 +52,15 @@ public final class GeneralValidatorsForTesting { * processing, as for some of those tests this package is on the classpath * and therefore the annotated validations are loadede for them. */ - public static boolean validatorTestsRunning = false; + private static boolean validatorTestsRunning = false; + + public static void startValidatorTest() { + validatorTestsRunning = true; + } + + public static void finishValidatorTest() { + validatorTestsRunning = false; + } private GeneralValidatorsForTesting() { } From d87b1f8bf713a1a77fe6ab06e8de0c4b5e8dff4c Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Sat, 5 Mar 2022 00:42:04 +0100 Subject: [PATCH 33/36] Addressing review comments. --- dev-support/annotations/pom.xml | 26 ++++++++- .../RequestFeatureValidatorProcessor.java | 26 ++++++--- .../ozone/annotations/package-info.java | 5 ++ .../validation/ValidationCondition.java | 54 +++++++++-------- .../request/validation/ValidatorRegistry.java | 58 +++++++++---------- .../TestRequestFeatureValidatorProcessor.java | 2 +- .../validation/TestRequestValidations.java | 40 +++++++------ .../validation/TestValidatorRegistry.java | 48 +++++++-------- .../GeneralValidatorsForTesting.java | 23 ++++---- ...alidatorsForOnlyOldClientValidations.java} | 10 ++-- pom.xml | 8 +++ 11 files changed, 177 insertions(+), 123 deletions(-) create mode 100644 dev-support/annotations/src/main/java/org/apache/ozone/annotations/package-info.java rename hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset2/{ValidatorsForOnlyNewClientValidations.java => ValidatorsForOnlyOldClientValidations.java} (86%) diff --git a/dev-support/annotations/pom.xml b/dev-support/annotations/pom.xml index 80b8b38e6f60..e65400285b3c 100644 --- a/dev-support/annotations/pom.xml +++ b/dev-support/annotations/pom.xml @@ -31,6 +31,8 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> UTF-8 UTF-8 3.1.12 + 3.1.0 + 8.29 @@ -45,8 +47,9 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> com.github.spotbugs @@ -58,6 +61,25 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd"> true + + org.apache.maven.plugins + maven-checkstyle-plugin + ${maven-checkstyle-plugin.version} + + + com.puppycrawl.tools + checkstyle + ${checkstyle.version} + + + + ../../hadoop-hdds/dev-support/checkstyle/checkstyle.xml + ../../hadoop-hdds/dev-support/checkstyle/suppressions.xml + true + false + ${project.build.directory}/test/checkstyle-errors.xml + + 3.1.2 3.9.1 + 9.3 1200 1.12.124 From 697b1ac656a2700ba98e17851228caa300918545 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 11 Mar 2022 22:27:57 +0100 Subject: [PATCH 34/36] Addressing further review comments. --- .../validation/ValidationCondition.java | 25 +---- .../validation/TestRequestValidations.java | 95 +++---------------- .../validation/TestValidatorRegistry.java | 63 ------------ .../GeneralValidatorsForTesting.java | 40 -------- 4 files changed, 12 insertions(+), 211 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java index 886fdd023264..e8a00d819068 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -49,30 +49,7 @@ public boolean shouldApply(OMRequest req, ValidationContext ctx) { public boolean shouldApply(OMRequest req, ValidationContext ctx) { return req.getVersion() < ClientVersions.CURRENT_VERSION; } - } /*, - - /** - * Classifies validations that has to run, when the client uses a newer - * protocol version than the server. - *//* - NEWER_CLIENT_REQUESTS { - @Override - public boolean shouldApply(OMRequest req, ValidationContext ctx) { - return req.getVersion() > ClientVersions.CURRENT_VERSION; - } - }, - - /** - * Classifies validations that has to run for every request. - * If you plan to use this, please justify why the validation code should not - * be part of the actual request handling code. - *//* - UNCONDITIONAL { - @Override - public boolean shouldApply(OMRequest req, ValidationContext ctx) { - return true; - } - }*/; + }; public abstract boolean shouldApply(OMRequest req, ValidationContext ctx); } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java index 0dd799b942bd..2ec21364dc71 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java @@ -92,12 +92,10 @@ public void testUsingRegistryWithoutPackage() throws ServiceException { .validateRequest(aCreateKeyRequest(currentClientVersion())); validationListener.assertNumOfEvents(0); -// validationListener -// .assertCalled("unconditionalPreProcessCreateKeyValidator"); } @Test - public void testNoPreValdiationsWithoutValidationMethods() + public void testNoPreValidationsWithoutValidationMethods() throws ServiceException { int omVersion = 0; ValidationContext ctx = of(aFinalizedVersionManager()); @@ -109,7 +107,7 @@ public void testNoPreValdiationsWithoutValidationMethods() } @Test - public void testNoPostValdiationsWithoutValidationMethods() + public void testNoPostValidationsWithoutValidationMethods() throws ServiceException { ValidationContext ctx = of(aFinalizedVersionManager()); RequestValidations validations = loadEmptyValidations(ctx); @@ -143,33 +141,6 @@ public void testNoPostValidationsAreRunningForRequestTypeWithoutValidators() validationListener.assertNumOfEvents(0); } -/* @Test - public void testUnconditionalPreProcessValidationsAreCalled() - throws ServiceException { - ValidationContext ctx = of(aFinalizedVersionManager()); - RequestValidations validations = loadValidations(ctx); - - validations.validateRequest(aCreateKeyRequest(currentClientVersion())); - - validationListener.assertNumOfEvents(1); - validationListener - .assertCalled("unconditionalPreProcessCreateKeyValidator"); - } - - @Test - public void testUnconditionalPostProcessValidationsAreCalled() - throws ServiceException { - ValidationContext ctx = of(aFinalizedVersionManager()); - RequestValidations validations = loadValidations(ctx); - - validations.validateResponse( - aCreateKeyRequest(currentClientVersion()), aCreateKeyResponse()); - - validationListener.assertNumOfEvents(1); - validationListener - .assertCalled("unconditionalPostProcessCreateKeyValidator"); - }*/ - @Test public void testPreProcessorExceptionHandling() { ValidationContext ctx = of(aFinalizedVersionManager()); @@ -181,7 +152,8 @@ public void testPreProcessorExceptionHandling() { } catch (ServiceException ignored) { } validationListener.assertNumOfEvents(1); - validationListener.assertCalled("throwingPreProcessValidator"); + validationListener.assertExactListOfValidatorsCalled( + "throwingPreProcessValidator"); } @Test @@ -196,39 +168,10 @@ public void testPostProcessorExceptionHandling() { } catch (ServiceException ignored) { } validationListener.assertNumOfEvents(1); - validationListener.assertCalled("throwingPostProcessValidator"); - } - -/* @Test - public void testNewClientConditionIsRecognizedAndPreValidatorsApplied() - throws ServiceException { - ValidationContext ctx = of(aFinalizedVersionManager()); - RequestValidations validations = loadValidations(ctx); - - validations.validateRequest(aCreateKeyRequest(newerClientVersion())); - - validationListener.assertNumOfEvents(2); - validationListener.assertAllCalled( - "unconditionalPreProcessCreateKeyValidator", - "newClientPreProcessCreateKeyValidator"); + validationListener.assertExactListOfValidatorsCalled( + "throwingPostProcessValidator"); } - @Test - public void testNewClientConditionIsRecognizedAndPostValidatorsApplied() - throws ServiceException { - ValidationContext ctx = of(aFinalizedVersionManager()); - RequestValidations validations = loadValidations(ctx); - - validations.validateResponse( - aCreateKeyRequest(newerClientVersion()), aCreateKeyResponse()); - - validationListener.assertNumOfEvents(3); - validationListener.assertAllCalled( - "unconditionalPostProcessCreateKeyValidator", - "newClientPostProcessCreateKeyValidator", - "newClientPostProcessCreateKeyValidator2"); - }*/ - @Test public void testOldClientConditionIsRecognizedAndPreValidatorsApplied() throws ServiceException { @@ -238,8 +181,7 @@ public void testOldClientConditionIsRecognizedAndPreValidatorsApplied() validations.validateRequest(aCreateKeyRequest(olderClientVersion())); validationListener.assertNumOfEvents(1); - validationListener.assertAllCalled( -// "unconditionalPreProcessCreateKeyValidator", + validationListener.assertExactListOfValidatorsCalled( "oldClientPreProcessCreateKeyValidator"); } @@ -253,8 +195,7 @@ public void testOldClientConditionIsRecognizedAndPostValidatorsApplied() aCreateKeyRequest(olderClientVersion()), aCreateKeyResponse()); validationListener.assertNumOfEvents(2); - validationListener.assertAllCalled( -// "unconditionalPostProcessCreateKeyValidator", + validationListener.assertExactListOfValidatorsCalled( "oldClientPostProcessCreateKeyValidator", "oldClientPostProcessCreateKeyValidator2"); } @@ -268,8 +209,7 @@ public void testPreFinalizedWithOldClientConditionPreProcValidatorsApplied() validations.validateRequest(aCreateKeyRequest(olderClientVersion())); validationListener.assertNumOfEvents(2); - validationListener.assertAllCalled( -// "unconditionalPreProcessCreateKeyValidator", + validationListener.assertExactListOfValidatorsCalled( "preFinalizePreProcessCreateKeyValidator", "oldClientPreProcessCreateKeyValidator"); } @@ -284,8 +224,7 @@ public void testPreFinalizedWithOldClientConditionPostProcValidatorsApplied() aCreateKeyRequest(olderClientVersion()), aCreateKeyResponse()); validationListener.assertNumOfEvents(3); - validationListener.assertAllCalled( -// "unconditionalPostProcessCreateKeyValidator", + validationListener.assertExactListOfValidatorsCalled( "preFinalizePostProcessCreateKeyValidator", "oldClientPostProcessCreateKeyValidator", "oldClientPostProcessCreateKeyValidator2"); @@ -313,10 +252,6 @@ private int currentClientVersion() { return ClientVersions.CURRENT_VERSION; } -/* private int newerClientVersion() { - return ClientVersions.CURRENT_VERSION + 1; - }*/ - private OMRequest aCreateKeyRequest(int clientVersion) { return aRequest(CreateKey, clientVersion); } @@ -389,15 +324,7 @@ public void reset() { calledMethods = new ArrayList<>(); } - public void assertCalled(String... methodNames) { - for (String methodName : methodNames) { - if (!calledMethods.contains(methodName)) { - fail("Expected method call for " + methodName + " did not happened."); - } - } - } - - public void assertAllCalled(String... methodNames) { + public void assertExactListOfValidatorsCalled(String... methodNames) { List calls = new ArrayList<>(calledMethods); for (String methodName : methodNames) { if (!calls.remove(methodName)) { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java index a6800ba111a1..e11fa5160d5c 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java @@ -101,40 +101,6 @@ public void testRegistryHasThePreFinalizePostProcessCreateKeyValidator() { assertEquals(expectedMethodName, validators.get(0).getName()); } -/* @Test - public void testRegistryHasTheNewClientPreProcessCreateKeyValidator() - throws MalformedURLException { - Collection urls = ClasspathHelper.forPackage(PACKAGE); - Collection urlsToUse = new ArrayList<>(); - for (URL url : urls) { - urlsToUse.add(new URL(url, PACKAGE.replaceAll("\\.", "/"))); - } - ValidatorRegistry registry = new ValidatorRegistry(urlsToUse); - List validators = - registry.validationsFor( - asList(NEWER_CLIENT_REQUESTS), CreateKey, PRE_PROCESS); - - assertEquals(1, validators.size()); - String expectedMethodName = "newClientPreProcessCreateKeyValidator"; - assertEquals(expectedMethodName, validators.get(0).getName()); - } - - @Test - public void testRegistryHasTheNewClientPostProcessCreateKeyValidator() { - ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); - List validators = - registry.validationsFor( - asList(NEWER_CLIENT_REQUESTS), CreateKey, POST_PROCESS); - - assertEquals(2, validators.size()); - List methodNames = - validators.stream().map(Method::getName).collect(Collectors.toList()); - assertTrue( - methodNames.contains("newClientPostProcessCreateKeyValidator")); - assertTrue( - methodNames.contains("newClientPostProcessCreateKeyValidator2")); - }*/ - @Test public void testRegistryHasTheOldClientPreProcessCreateKeyValidator() { ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); @@ -163,30 +129,6 @@ public void testRegistryHasTheOldClientPostProcessCreateKeyValidator() { assertTrue(methodNames.contains("oldClientPostProcessCreateKeyValidator2")); } -/* @Test - public void testRegistryHasTheUnconditionalPreProcessCreateKeyValidator() { - ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); - List validators = - registry.validationsFor( - asList(UNCONDITIONAL), CreateKey, PRE_PROCESS); - - assertEquals(1, validators.size()); - String expectedMethodName = "unconditionalPreProcessCreateKeyValidator"; - assertEquals(expectedMethodName, validators.get(0).getName()); - } - - @Test - public void testRegistryHasTheUnconditionalPostProcessCreateKeyValidator() { - ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); - List validators = - registry.validationsFor( - asList(UNCONDITIONAL), CreateKey, POST_PROCESS); - - assertEquals(1, validators.size()); - String expectedMethodName = "unconditionalPostProcessCreateKeyValidator"; - assertEquals(expectedMethodName, validators.get(0).getName()); - }*/ - @Test public void testRegistryHasTheMultiPurposePreProcessCreateVolumeValidator() { ValidatorRegistry registry = new ValidatorRegistry(PACKAGE); @@ -213,17 +155,12 @@ public void testRegistryHasTheMultiPurposePostProcessCreateVolumeValidator() { List oldClientValidators = registry.validationsFor( asList(OLDER_CLIENT_REQUESTS), CreateVolume, POST_PROCESS); -// List unconditionalValidators = -// registry.validationsFor( -// asList(UNCONDITIONAL), CreateVolume, POST_PROCESS); assertEquals(1, preFinalizeValidators.size()); assertEquals(1, oldClientValidators.size()); -// assertEquals(1, unconditionalValidators.size()); String expectedMethodName = "multiPurposePostProcessCreateVolumeValidator"; assertEquals(expectedMethodName, preFinalizeValidators.get(0).getName()); assertEquals(expectedMethodName, oldClientValidators.get(0).getName()); -// assertEquals(expectedMethodName, unconditionalValidators.get(0).getName()); } @Test diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java index 6bd8a5100566..35c3afa4cf96 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java @@ -108,26 +108,6 @@ public static OMResponse preFinalizePostProcessCreateKeyValidator( return resp; } -/* @RequestFeatureValidator( - conditions = { NEWER_CLIENT_REQUESTS }, - processingPhase = PRE_PROCESS, - requestType = CreateKey) - public static OMRequest newClientPreProcessCreateKeyValidator( - OMRequest req, ValidationContext ctx) { - fireValidationEvent("newClientPreProcessCreateKeyValidator"); - return req; - } - - @RequestFeatureValidator( - conditions = { NEWER_CLIENT_REQUESTS }, - processingPhase = POST_PROCESS, - requestType = CreateKey) - public static OMResponse newClientPostProcessCreateKeyValidator( - OMRequest req, OMResponse resp, ValidationContext ctx) { - fireValidationEvent("newClientPostProcessCreateKeyValidator"); - return resp; - }*/ - @RequestFeatureValidator( conditions = { OLDER_CLIENT_REQUESTS }, processingPhase = PRE_PROCESS, @@ -148,26 +128,6 @@ public static OMResponse oldClientPostProcessCreateKeyValidator( return resp; } -/* @RequestFeatureValidator( - conditions = { UNCONDITIONAL }, - processingPhase = PRE_PROCESS, - requestType = CreateKey) - public static OMRequest unconditionalPreProcessCreateKeyValidator( - OMRequest req, ValidationContext ctx) { - fireValidationEvent("unconditionalPreProcessCreateKeyValidator"); - return req; - } - - @RequestFeatureValidator( - conditions = { UNCONDITIONAL }, - processingPhase = POST_PROCESS, - requestType = CreateKey) - public static OMResponse unconditionalPostProcessCreateKeyValidator( - OMRequest req, OMResponse resp, ValidationContext ctx) { - fireValidationEvent("unconditionalPostProcessCreateKeyValidator"); - return resp; - }*/ - @RequestFeatureValidator( conditions = { CLUSTER_NEEDS_FINALIZATION, OLDER_CLIENT_REQUESTS }, processingPhase = PRE_PROCESS, From eaac42ee63a10a62f4373cb7c0e6c926291dfe94 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Fri, 11 Mar 2022 22:37:47 +0100 Subject: [PATCH 35/36] Rebasing on new master and fix ClientVersions->ClientVersion rename. --- .../ozone/om/request/validation/ValidationCondition.java | 4 ++-- .../ozone/om/request/validation/TestRequestValidations.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java index e8a00d819068..9630500cbd53 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidationCondition.java @@ -16,7 +16,7 @@ */ package org.apache.hadoop.ozone.om.request.validation; -import org.apache.hadoop.ozone.ClientVersions; +import org.apache.hadoop.ozone.ClientVersion; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; /** @@ -47,7 +47,7 @@ public boolean shouldApply(OMRequest req, ValidationContext ctx) { OLDER_CLIENT_REQUESTS { @Override public boolean shouldApply(OMRequest req, ValidationContext ctx) { - return req.getVersion() < ClientVersions.CURRENT_VERSION; + return req.getVersion() < ClientVersion.CURRENT_VERSION; } }; diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java index 2ec21364dc71..4508eafd0459 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestRequestValidations.java @@ -17,7 +17,7 @@ package org.apache.hadoop.ozone.om.request.validation; import com.google.protobuf.ServiceException; -import org.apache.hadoop.ozone.ClientVersions; +import org.apache.hadoop.ozone.ClientVersion; import org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting; import org.apache.hadoop.ozone.om.request.validation.testvalidatorset1.GeneralValidatorsForTesting.ValidationListener; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMRequest; @@ -245,11 +245,11 @@ private RequestValidations loadEmptyValidations(ValidationContext ctx) { } private int olderClientVersion() { - return ClientVersions.CURRENT_VERSION - 1; + return ClientVersion.CURRENT_VERSION - 1; } private int currentClientVersion() { - return ClientVersions.CURRENT_VERSION; + return ClientVersion.CURRENT_VERSION; } private OMRequest aCreateKeyRequest(int clientVersion) { From 4d69af6e5071da017ae0e3b37f3bb2f400786142 Mon Sep 17 00:00:00 2001 From: Istvan Fajth Date: Thu, 17 Mar 2022 20:40:26 +0100 Subject: [PATCH 36/36] Re-trigger CI.