Copilot commented on code in PR #8404:
URL: https://github.com/apache/ozone/pull/8404#discussion_r2688432203
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java:
##########
@@ -88,82 +89,90 @@ private static void fireValidationEvent(String
calledMethodName) {
listeners.forEach(l -> l.validationCalled(calledMethodName));
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = PRE_PROCESS,
requestType = CreateKey)
- public static OMRequest preFinalizePreProcessCreateKeyValidator(
+ public static OMRequest preProcessCreateKeyQuotaLayoutValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("preFinalizePreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyQuotaLayoutValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.FUTURE_VERSION,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateKey)
+ public static OMRequest preProcessCreateKeyFutureClientValidator(
+ OMRequest req, ValidationContext ctx) {
+ fireValidationEvent("preProcessCreateKeyFutureClientValidator");
+ return req;
+ }
+
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse preFinalizePostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyQuotaLayoutValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("preFinalizePostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyQuotaLayoutValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
processingPhase = PRE_PROCESS,
- requestType = CreateKey)
- public static OMRequest oldClientPreProcessCreateKeyValidator(
+ requestType = CreateKey,
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT)
+ public static OMRequest preProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("oldClientPreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyBucketLayoutClientValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse oldClientPostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("oldClientPostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyBucketLayoutClientValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION, OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = PRE_PROCESS,
requestType = CreateVolume)
- public static OMRequest multiPurposePreProcessCreateVolumeValidator(
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateVolume)
+ public static OMRequest
multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
Review Comment:
Typo in the method name
"multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The
word "CLient" should be "Client" (lowercase 'l').
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java:
##########
@@ -42,156 +50,265 @@
* Registry that loads and stores the request validators to be applied by
* a service.
*/
-public class ValidatorRegistry {
+public class ValidatorRegistry<RequestType extends Enum<RequestType>> {
- private final EnumMap<ValidationCondition,
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>>
- validators = new EnumMap<>(ValidationCondition.class);
+ /**
+ * A validator registered should have the following parameters:
+ * applyBeforeVersion: Enum extending Version
+ * RequestType: Enum signifying the type of request.
+ * RequestProcessingPhase: Signifying if the validator is supposed to run
pre or post submitting the request.
+ * Based on the afforementioned parameters a complete map is built which
stores the validators in a sorted order of
Review Comment:
Typo in "afforementioned". It should be "aforementioned".
```suggestion
* Based on the aforementioned parameters a complete map is built which
stores the validators in a sorted order of
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java:
##########
@@ -88,82 +89,90 @@ private static void fireValidationEvent(String
calledMethodName) {
listeners.forEach(l -> l.validationCalled(calledMethodName));
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = PRE_PROCESS,
requestType = CreateKey)
- public static OMRequest preFinalizePreProcessCreateKeyValidator(
+ public static OMRequest preProcessCreateKeyQuotaLayoutValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("preFinalizePreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyQuotaLayoutValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.FUTURE_VERSION,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateKey)
+ public static OMRequest preProcessCreateKeyFutureClientValidator(
+ OMRequest req, ValidationContext ctx) {
+ fireValidationEvent("preProcessCreateKeyFutureClientValidator");
+ return req;
+ }
+
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse preFinalizePostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyQuotaLayoutValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("preFinalizePostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyQuotaLayoutValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
processingPhase = PRE_PROCESS,
- requestType = CreateKey)
- public static OMRequest oldClientPreProcessCreateKeyValidator(
+ requestType = CreateKey,
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT)
+ public static OMRequest preProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("oldClientPreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyBucketLayoutClientValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse oldClientPostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("oldClientPostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyBucketLayoutClientValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION, OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = PRE_PROCESS,
requestType = CreateVolume)
- public static OMRequest multiPurposePreProcessCreateVolumeValidator(
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateVolume)
+ public static OMRequest
multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("multiPurposePreProcessCreateVolumeValidator");
+
fireValidationEvent("multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS, CLUSTER_NEEDS_FINALIZATION },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = POST_PROCESS,
requestType = CreateVolume)
- public static OMResponse multiPurposePostProcessCreateVolumeValidator(
- OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("multiPurposePostProcessCreateVolumeValidator");
- return resp;
- }
-
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = POST_PROCESS,
- requestType = CreateKey)
- public static OMResponse oldClientPostProcessCreateKeyValidator2(
+ requestType = CreateVolume)
+ public static OMResponse
multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
Review Comment:
Typo in the method name
"multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator".
The word "CLient" should be "Client" (lowercase 'l').
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java:
##########
@@ -42,156 +50,265 @@
* Registry that loads and stores the request validators to be applied by
* a service.
*/
-public class ValidatorRegistry {
+public class ValidatorRegistry<RequestType extends Enum<RequestType>> {
- private final EnumMap<ValidationCondition,
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>>
- validators = new EnumMap<>(ValidationCondition.class);
+ /**
+ * A validator registered should have the following parameters:
+ * applyBeforeVersion: Enum extending Version
+ * RequestType: Enum signifying the type of request.
+ * RequestProcessingPhase: Signifying if the validator is supposed to run
pre or post submitting the request.
+ * Based on the afforementioned parameters a complete map is built which
stores the validators in a sorted order of
+ * the applyBeforeVersion value of the validator method.
+ * Thus when a request comes with a certain version value, all validators
containing `applyBeforeVersion` parameter
+ * greater than the request versions get triggered.
+ * {@link #validationsFor(Enum, RequestProcessingPhase, Class, Versioned)}
+ */
+ private final Map<Class<? extends Annotation>, EnumMap<RequestType,
+ EnumMap<RequestProcessingPhase, IndexedItems<Method, Versioned>>>>
indexedValidatorMap;
/**
* Creates a {@link ValidatorRegistry} instance that discovers validation
* 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.
- * @param validatorPackage the main package inside which validatiors should
- * be discovered.
+ * A validation method is recognized by all the annotations classes which
+ * are annotated by {@link RegisterValidator} annotation that contains
+ * important information about how and when to use the validator.
+ *
+ * @param requestType class of request type enum.
+ * @param validatorPackage the main package inside which validatiors
should
+ * be discovered.
+ * @param allowedValidators a set containing the various types of
version allowed to be registered.
+ * @param allowedProcessingPhases set of request processing phases which
would be allowed to be registered to
+ * registry.
*/
- ValidatorRegistry(String validatorPackage) {
- this(ClasspathHelper.forPackage(validatorPackage));
+ public ValidatorRegistry(Class<RequestType> requestType, String
validatorPackage,
+ Set<Class<? extends Annotation>> allowedValidators,
Set<RequestProcessingPhase> allowedProcessingPhases) {
+ this(requestType, ClasspathHelper.forPackage(validatorPackage),
allowedValidators, allowedProcessingPhases);
}
/**
* Creates a {@link ValidatorRegistry} instance that discovers validation
* methods under the provided URL.
- * A validation method is recognized by the {@link RequestFeatureValidator}
+ * A validation method is recognized by all annotations annotated by the
{@link RegisterValidator}
* annotation that contains important information about how and when to use
* the validator.
- * @param searchUrls the path in which the annotated methods are searched.
+ *
+ * @param requestType class of request type enum.
+ * @param searchUrls the path in which the annotated methods
are searched.
+ * @param allowedValidators a set containing the various types of
validator annotation allowed to be registered.
+ * @param allowedProcessingPhases set of request processing phases which
would be allowed to be registered to
+ * the registry.
*/
- ValidatorRegistry(Collection<URL> searchUrls) {
+ ValidatorRegistry(Class<RequestType> requestType, Collection<URL> searchUrls,
+ Set<Class<? extends Annotation>> allowedValidators,
+ Set<RequestProcessingPhase> allowedProcessingPhases) {
+ Class<RequestType[]> requestArrayClass = (Class<RequestType[]>)
Array.newInstance(requestType, 0)
+ .getClass();
+ Set<Class<? extends Annotation>> validatorsToBeRegistered =
+ new Reflections(new
ConfigurationBuilder().setUrls(ClasspathHelper.forPackage("org.apache.hadoop"))
+ .setScanners(Scanners.TypesAnnotated)
+
.setParallel(true)).getTypesAnnotatedWith(RegisterValidator.class).stream()
+ .filter(allowedValidators::contains)
+ .filter(annotationClass ->
getReturnTypeOfAnnotationMethod((Class<? extends Annotation>) annotationClass,
+ RegisterValidator.REQUEST_TYPE_METHOD_NAME)
+ .equals(requestArrayClass))
+ .map(annotationClass -> (Class<? extends Annotation>)
annotationClass)
+ .collect(Collectors.toSet());
+ this.indexedValidatorMap =
allowedValidators.stream().collect(ImmutableMap.toImmutableMap(Function.identity(),
+ validatorClass -> new EnumMap<>(requestType)));
Reflections reflections = new Reflections(new ConfigurationBuilder()
.setUrls(searchUrls)
.setScanners(Scanners.MethodsAnnotated)
.setParallel(true)
);
-
- Set<Method> describedValidators =
- reflections.getMethodsAnnotatedWith(RequestFeatureValidator.class);
- initMaps(describedValidators);
+ initMaps(requestArrayClass, allowedProcessingPhases,
validatorsToBeRegistered, reflections);
}
/**
- * Get the validators that has to be run in the given list of
- * {@link ValidationCondition}s, for the given requestType and
+ * Get the validators that has to be run in the given list of,
+ * for the given requestType and for the given request versions.
* {@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
+ * @param requestType the type of the protocol message
+ * @param phase the request processing phase
+ * @param requestVersions different versions extracted from the request.
* @return the list of validation methods that has to run.
*/
- List<Method> validationsFor(
- List<ValidationCondition> conditions,
- Type requestType,
- RequestProcessingPhase phase) {
+ public List<Method> validationsFor(RequestType requestType,
RequestProcessingPhase phase,
+ Map<Class<? extends Annotation>, ? extends Versioned> requestVersions) {
+ return requestVersions.entrySet().stream()
+ .flatMap(requestVersion -> this.validationsFor(requestType, phase,
requestVersion.getKey(),
+ requestVersion.getValue()).stream())
+ .distinct().collect(Collectors.toList());
+ }
- if (conditions.isEmpty() || validators.isEmpty()) {
- return Collections.emptyList();
- }
+ /**
+ * Get the validators that has to be run in the given list of,
+ * for the given requestType and for the given request versions.
+ * {@link RequestProcessingPhase}.
+ *
+ * @param requestType the type of the protocol message
+ * @param phase the request processing phase
+ * @param validatorClass annotation class of the validator
+ * @param requestVersion version extracted corresponding to the request.
+ * @return the list of validation methods that has to run.
+ */
+ public <V extends Versioned> List<Method> validationsFor(RequestType
requestType,
+ RequestProcessingPhase phase, Class<? extends Annotation>
validatorClass, V requestVersion) {
- Set<Method> returnValue = new HashSet<>();
+ return Optional.ofNullable(this.indexedValidatorMap.get(validatorClass))
+ .map(requestTypeMap -> requestTypeMap.get(requestType))
+ .map(phaseMap -> phaseMap.get(phase))
+ .map(indexedMethods -> requestVersion.version() < 0 ?
+ indexedMethods.getItemsEqualToIdx(requestVersion) :
+ indexedMethods.getItemsGreaterThanIdx(requestVersion))
+ .orElse(Collections.emptyList());
- for (ValidationCondition condition: conditions) {
- returnValue.addAll(validationsFor(condition, requestType, phase));
- }
- return new ArrayList<>(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.
+ * Calls a specified method on the validator.
+ * @throws IllegalArgumentException when the specified method in the
validator is invalid.
*/
- private List<Method> validationsFor(
- ValidationCondition condition,
- Type requestType,
- RequestProcessingPhase phase) {
-
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>
- requestTypeMap = validators.get(condition);
- if (requestTypeMap == null || requestTypeMap.isEmpty()) {
- return Collections.emptyList();
+ private static <ReturnValue, Validator extends Annotation> ReturnValue
callAnnotationMethod(
+ Validator validator, String methodName, Class<ReturnValue>
returnValueType) {
+ try {
+ return
returnValueType.cast(validator.getClass().getMethod(methodName).invoke(validator));
+ } catch (NoSuchMethodException e) {
+ throw new IllegalArgumentException("Method " + methodName + " not found
in class:" +
+ validator.getClass().getCanonicalName(), e);
+ } catch (InvocationTargetException | IllegalAccessException e) {
+ throw new IllegalArgumentException("Error while invoking Method " +
methodName + " from " +
+ validator.getClass().getCanonicalName(), e);
}
+ }
- EnumMap<RequestProcessingPhase, List<Method>> phases =
- requestTypeMap.get(requestType);
- if (phases == null) {
- return Collections.emptyList();
+ private static Class<?> getReturnTypeOfAnnotationMethod(Class<? extends
Annotation> clzz, String methodName) {
+ try {
+ return clzz.getMethod(methodName).getReturnType();
+ } catch (NoSuchMethodException e) {
+ throw new IllegalArgumentException("Method " + methodName + " not found
in class:" + clzz.getCanonicalName());
}
+ }
- List<Method> validatorsForPhase = phases.get(phase);
- if (validatorsForPhase == null) {
- return Collections.emptyList();
+ private static <Validator extends Annotation> Versioned
getApplyBeforeVersion(Validator validator) {
+ return callAnnotationMethod(validator,
RegisterValidator.APPLY_BEFORE_METHOD_NAME, Versioned.class);
+ }
+
+ private static <Validator extends Annotation> RequestProcessingPhase
getRequestPhase(Validator validator) {
+ return callAnnotationMethod(validator,
RegisterValidator.PROCESSING_PHASE_METHOD_NAME,
+ RequestProcessingPhase.class);
+ }
+
+ private <Validator extends Annotation> RequestType[]
getRequestType(Validator validator,
+ Class<RequestType[]> requestType) {
+ return callAnnotationMethod(validator,
RegisterValidator.REQUEST_TYPE_METHOD_NAME, requestType);
+ }
+
+ private static <V> void checkAllowedAnnotationValues(Set<V> values, V value,
String valueName, String methodName) {
+ if (!values.contains(value)) {
+ throw new IllegalArgumentException(
+ String.format("Invalid %1$s defined at annotation defined for method
: %2$s, Annotation value : %3$s " +
+ "Allowed versionType: %4$s", valueName, methodName,
value.toString(), values));
}
- return validatorsForPhase;
}
/**
* 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 describedValidators collection of the annotated methods to process.
+ * - An EnumMap with the RequestType as the key, and in which
+ * - values are an EnumMap with the request processing phase as the key, and
in which
+ * - values is an {@link IndexedItems } containing the validation list
+ *
+ * @param validatorsToBeRegistered collection of the annotated validtors to
process.
*/
- void initMaps(Collection<Method> describedValidators) {
- for (Method m : describedValidators) {
- RequestFeatureValidator descriptor =
- m.getAnnotation(RequestFeatureValidator.class);
- m.setAccessible(true);
-
- for (ValidationCondition condition : descriptor.conditions()) {
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>
- requestTypeMap = getAndInitialize(
- condition, this::newTypeMap, validators);
- EnumMap<RequestProcessingPhase, List<Method>> phases =
getAndInitialize(
- descriptor.requestType(), this::newPhaseMap, requestTypeMap);
- if (isPreProcessValidator(descriptor)) {
- getAndInitialize(PRE_PROCESS, ArrayList::new, phases).add(m);
- } else if (isPostProcessValidator(descriptor)) {
- getAndInitialize(POST_PROCESS, ArrayList::new, phases).add(m);
- }
- }
+ private void initMaps(Class<RequestType[]> requestType,
+ Set<RequestProcessingPhase> allowedPhases,
+ Collection<Class<? extends Annotation>> validatorsToBeRegistered,
+ Reflections reflections) {
+ for (Class<? extends Annotation> validator : validatorsToBeRegistered) {
+ registerValidator(requestType, allowedPhases, validator, reflections);
}
}
- private EnumMap<Type,
- EnumMap<RequestProcessingPhase, List<Method>>> newTypeMap() {
- return new EnumMap<>(Type.class);
+ private void registerValidator(Class<RequestType[]> requestType,
+ Set<RequestProcessingPhase> allowedPhases,
+ Class<? extends Annotation> validatorToBeRegistered,
+ Reflections reflections) {
+ Collection<Method> methods =
reflections.getMethodsAnnotatedWith(validatorToBeRegistered);
+ List<Pair<? extends Annotation, Method>> sortedMethodsByApplyBeforeVersion
= methods.stream()
+ .map(method -> Pair.of(method.getAnnotation(validatorToBeRegistered),
method))
+ .sorted(Comparator.comparing(validatorMethodPair ->
getApplyBeforeVersion(validatorMethodPair.getKey()),
+ Versioned.versionComparator()))
+ .collect(Collectors.toList());
+ for (Pair<? extends Annotation, Method> validatorMethodPair :
sortedMethodsByApplyBeforeVersion) {
+ Annotation validator = validatorMethodPair.getKey();
+ Method method = validatorMethodPair.getValue();
+ Versioned applyBeforeVersion = this.getApplyBeforeVersion(validator);
+ RequestProcessingPhase phase = this.getRequestPhase(validator);
+ checkAllowedAnnotationValues(allowedPhases, phase,
RegisterValidator.PROCESSING_PHASE_METHOD_NAME,
+ method.getName());
+ Set<RequestType> types = Sets.newHashSet(this.getRequestType(validator,
requestType));
+ method.setAccessible(true);
+ for (RequestType type : types) {
+ EnumMap<RequestType, EnumMap<RequestProcessingPhase,
IndexedItems<Method, Versioned>>> requestMap =
+ this.indexedValidatorMap.get(validatorToBeRegistered);
+ EnumMap<RequestProcessingPhase, IndexedItems<Method, Versioned>>
phaseMap =
+ requestMap.computeIfAbsent(type, k -> new
EnumMap<>(RequestProcessingPhase.class));
+ phaseMap.computeIfAbsent(phase, k -> new
IndexedItems<>(Versioned.versionComparator()))
+ .add(method, applyBeforeVersion);
+ }
+ }
}
- private EnumMap<RequestProcessingPhase, List<Method>> newPhaseMap() {
- return new EnumMap<>(RequestProcessingPhase.class);
- }
+ /**
+ * Class responsible for maintaining indexs of items. Here each item should
have an index corresponding to it.
Review Comment:
Typo in the word "indexs". It should be "indexes" or "indices".
```suggestion
* Class responsible for maintaining indexes of items. Here each item
should have an index corresponding to it.
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java:
##########
@@ -69,123 +81,129 @@ public void tearDown() {
finishValidatorTest();
}
+ private List<Method> getValidatorsForRequest(OzoneManagerProtocolProtos.Type
requestType,
+ RequestProcessingPhase phase, VersionExtractor extractor, Versioned
versioned) {
+ return getValidatorsForRequest(PACKAGE, requestType, phase,
Collections.singletonMap(extractor, versioned));
+ }
+
+ private List<Method> getValidatorsForRequest(OzoneManagerProtocolProtos.Type
requestType,
+ RequestProcessingPhase phase, Map<VersionExtractor, Versioned>
requestVersions) {
+ return getValidatorsForRequest(PACKAGE, requestType, phase,
requestVersions);
+ }
+
+ private List<Method> getValidatorsForRequest(String packageName,
OzoneManagerProtocolProtos.Type requestType,
+ RequestProcessingPhase phase, Map<VersionExtractor, Versioned>
requestVersions) {
+ ValidatorRegistry<OzoneManagerProtocolProtos.Type> registry =
+ new ValidatorRegistry<>(OzoneManagerProtocolProtos.Type.class,
packageName,
+
Arrays.stream(VersionExtractor.values()).map(VersionExtractor::getValidatorClass)
+ .collect(Collectors.toSet()), REQUEST_PROCESSING_PHASES);
+ return registry.validationsFor(requestType, phase,
requestVersions.entrySet().stream()
+ .collect(Collectors.toMap(e -> e.getKey().getValidatorClass(),
Map.Entry::getValue)));
+ }
+
@Test
public void testNoValidatorsReturnedForEmptyConditionList() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(emptyList(), CreateKey, PRE_PROCESS);
-
+ List<Method> validators = getValidatorsForRequest(CreateKey, PRE_PROCESS,
+ CLIENT_VERSION_EXTRACTOR, ClientVersion.CURRENT);
assertTrue(validators.isEmpty());
}
@Test
public void testRegistryHasThePreFinalizePreProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(CLUSTER_NEEDS_FINALIZATION), CreateKey, PRE_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, PRE_PROCESS,
+ ImmutableMap.of(CLIENT_VERSION_EXTRACTOR, ClientVersion.CURRENT,
+ LAYOUT_VERSION_EXTRACTOR, OMLayoutFeature.FILESYSTEM_SNAPSHOT));
assertEquals(1, validators.size());
- String expectedMethodName = "preFinalizePreProcessCreateKeyValidator";
+ String expectedMethodName = "preProcessCreateKeyQuotaLayoutValidator";
assertEquals(expectedMethodName, validators.get(0).getName());
}
@Test
public void testRegistryHasThePreFinalizePostProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(CLUSTER_NEEDS_FINALIZATION), CreateKey, POST_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, POST_PROCESS,
+ ImmutableMap.of(CLIENT_VERSION_EXTRACTOR, ClientVersion.CURRENT,
+ LAYOUT_VERSION_EXTRACTOR, OMLayoutFeature.BUCKET_LAYOUT_SUPPORT));
assertEquals(1, validators.size());
- String expectedMethodName = "preFinalizePostProcessCreateKeyValidator";
+ String expectedMethodName = "postProcessCreateKeyQuotaLayoutValidator";
assertEquals(expectedMethodName, validators.get(0).getName());
}
@Test
public void testRegistryHasTheOldClientPreProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(OLDER_CLIENT_REQUESTS), CreateKey, PRE_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, PRE_PROCESS,
CLIENT_VERSION_EXTRACTOR,
+ ClientVersion.ERASURE_CODING_SUPPORT);
assertEquals(2, validators.size());
List<String> methodNames =
validators.stream().map(Method::getName).collect(Collectors.toList());
- assertThat(methodNames).contains("oldClientPreProcessCreateKeyValidator");
- assertThat(methodNames).contains("oldClientPreProcessCreateKeyValidator2");
+
assertEquals(Arrays.asList("preProcessCreateKeyBucketLayoutClientValidator",
+ "preProcessCreateKeyBucketLayoutClientValidator"), methodNames);
}
@Test
public void testRegistryHasTheOldClientPostProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(OLDER_CLIENT_REQUESTS), CreateKey, POST_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, POST_PROCESS,
+ CLIENT_VERSION_EXTRACTOR, ClientVersion.ERASURE_CODING_SUPPORT);
- assertEquals(2, validators.size());
+ assertEquals(1, validators.size());
List<String> methodNames =
validators.stream().map(Method::getName).collect(Collectors.toList());
- assertThat(methodNames).contains("oldClientPostProcessCreateKeyValidator");
-
assertThat(methodNames).contains("oldClientPostProcessCreateKeyValidator2");
+
assertThat(methodNames).contains("postProcessCreateKeyBucketLayoutClientValidator");
}
@Test
public void testRegistryHasTheMultiPurposePreProcessCreateVolumeValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> preFinalizeValidators =
- registry.validationsFor(
- asList(CLUSTER_NEEDS_FINALIZATION), CreateVolume, PRE_PROCESS);
- List<Method> newClientValidators =
- registry.validationsFor(
- asList(OLDER_CLIENT_REQUESTS), CreateVolume, PRE_PROCESS);
+ List<Method> preFinalizeValidators = getValidatorsForRequest(CreateVolume,
PRE_PROCESS, LAYOUT_VERSION_EXTRACTOR,
+ OMLayoutFeature.HSYNC);
+ List<Method> newClientValidators = getValidatorsForRequest(CreateVolume,
PRE_PROCESS, CLIENT_VERSION_EXTRACTOR,
+ ClientVersion.ERASURE_CODING_SUPPORT);
assertEquals(1, preFinalizeValidators.size());
assertEquals(1, newClientValidators.size());
- String expectedMethodName = "multiPurposePreProcessCreateVolumeValidator";
+ String expectedMethodName =
"multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator";
assertEquals(expectedMethodName, preFinalizeValidators.get(0).getName());
assertEquals(expectedMethodName, newClientValidators.get(0).getName());
}
@Test
public void testRegistryHasTheMultiPurposePostProcessCreateVolumeValidator()
{
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> preFinalizeValidators =
- registry.validationsFor(
- asList(CLUSTER_NEEDS_FINALIZATION), CreateVolume, POST_PROCESS);
- List<Method> oldClientValidators =
- registry.validationsFor(
- asList(OLDER_CLIENT_REQUESTS), CreateVolume, POST_PROCESS);
+ List<Method> preFinalizeValidators = getValidatorsForRequest(CreateVolume,
POST_PROCESS, LAYOUT_VERSION_EXTRACTOR,
+ OMLayoutFeature.HSYNC);
+ List<Method> oldClientValidators = getValidatorsForRequest(CreateVolume,
POST_PROCESS, CLIENT_VERSION_EXTRACTOR,
+ ClientVersion.ERASURE_CODING_SUPPORT);
+ List<Method> combinedValidators = getValidatorsForRequest(CreateVolume,
POST_PROCESS,
+ ImmutableMap.of(LAYOUT_VERSION_EXTRACTOR, OMLayoutFeature.HSYNC,
+ CLIENT_VERSION_EXTRACTOR, ClientVersion.ERASURE_CODING_SUPPORT));
assertEquals(1, preFinalizeValidators.size());
assertEquals(1, oldClientValidators.size());
- String expectedMethodName = "multiPurposePostProcessCreateVolumeValidator";
+ assertEquals(1, combinedValidators.size());
+ String expectedMethodName =
"multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator";
Review Comment:
Typo in the expected method name
"multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator".
The word "CLient" should be "Client" (lowercase 'l').
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java:
##########
@@ -42,156 +50,265 @@
* Registry that loads and stores the request validators to be applied by
* a service.
*/
-public class ValidatorRegistry {
+public class ValidatorRegistry<RequestType extends Enum<RequestType>> {
- private final EnumMap<ValidationCondition,
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>>
- validators = new EnumMap<>(ValidationCondition.class);
+ /**
+ * A validator registered should have the following parameters:
+ * applyBeforeVersion: Enum extending Version
+ * RequestType: Enum signifying the type of request.
+ * RequestProcessingPhase: Signifying if the validator is supposed to run
pre or post submitting the request.
+ * Based on the afforementioned parameters a complete map is built which
stores the validators in a sorted order of
+ * the applyBeforeVersion value of the validator method.
+ * Thus when a request comes with a certain version value, all validators
containing `applyBeforeVersion` parameter
+ * greater than the request versions get triggered.
+ * {@link #validationsFor(Enum, RequestProcessingPhase, Class, Versioned)}
+ */
+ private final Map<Class<? extends Annotation>, EnumMap<RequestType,
+ EnumMap<RequestProcessingPhase, IndexedItems<Method, Versioned>>>>
indexedValidatorMap;
/**
* Creates a {@link ValidatorRegistry} instance that discovers validation
* 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.
- * @param validatorPackage the main package inside which validatiors should
- * be discovered.
+ * A validation method is recognized by all the annotations classes which
+ * are annotated by {@link RegisterValidator} annotation that contains
+ * important information about how and when to use the validator.
+ *
+ * @param requestType class of request type enum.
+ * @param validatorPackage the main package inside which validatiors
should
+ * be discovered.
+ * @param allowedValidators a set containing the various types of
version allowed to be registered.
+ * @param allowedProcessingPhases set of request processing phases which
would be allowed to be registered to
+ * registry.
*/
- ValidatorRegistry(String validatorPackage) {
- this(ClasspathHelper.forPackage(validatorPackage));
+ public ValidatorRegistry(Class<RequestType> requestType, String
validatorPackage,
+ Set<Class<? extends Annotation>> allowedValidators,
Set<RequestProcessingPhase> allowedProcessingPhases) {
+ this(requestType, ClasspathHelper.forPackage(validatorPackage),
allowedValidators, allowedProcessingPhases);
}
/**
* Creates a {@link ValidatorRegistry} instance that discovers validation
* methods under the provided URL.
- * A validation method is recognized by the {@link RequestFeatureValidator}
+ * A validation method is recognized by all annotations annotated by the
{@link RegisterValidator}
* annotation that contains important information about how and when to use
* the validator.
- * @param searchUrls the path in which the annotated methods are searched.
+ *
+ * @param requestType class of request type enum.
+ * @param searchUrls the path in which the annotated methods
are searched.
+ * @param allowedValidators a set containing the various types of
validator annotation allowed to be registered.
+ * @param allowedProcessingPhases set of request processing phases which
would be allowed to be registered to
+ * the registry.
*/
- ValidatorRegistry(Collection<URL> searchUrls) {
+ ValidatorRegistry(Class<RequestType> requestType, Collection<URL> searchUrls,
+ Set<Class<? extends Annotation>> allowedValidators,
+ Set<RequestProcessingPhase> allowedProcessingPhases) {
+ Class<RequestType[]> requestArrayClass = (Class<RequestType[]>)
Array.newInstance(requestType, 0)
+ .getClass();
+ Set<Class<? extends Annotation>> validatorsToBeRegistered =
+ new Reflections(new
ConfigurationBuilder().setUrls(ClasspathHelper.forPackage("org.apache.hadoop"))
+ .setScanners(Scanners.TypesAnnotated)
+
.setParallel(true)).getTypesAnnotatedWith(RegisterValidator.class).stream()
+ .filter(allowedValidators::contains)
+ .filter(annotationClass ->
getReturnTypeOfAnnotationMethod((Class<? extends Annotation>) annotationClass,
+ RegisterValidator.REQUEST_TYPE_METHOD_NAME)
+ .equals(requestArrayClass))
+ .map(annotationClass -> (Class<? extends Annotation>)
annotationClass)
+ .collect(Collectors.toSet());
+ this.indexedValidatorMap =
allowedValidators.stream().collect(ImmutableMap.toImmutableMap(Function.identity(),
+ validatorClass -> new EnumMap<>(requestType)));
Reflections reflections = new Reflections(new ConfigurationBuilder()
.setUrls(searchUrls)
.setScanners(Scanners.MethodsAnnotated)
.setParallel(true)
);
-
- Set<Method> describedValidators =
- reflections.getMethodsAnnotatedWith(RequestFeatureValidator.class);
- initMaps(describedValidators);
+ initMaps(requestArrayClass, allowedProcessingPhases,
validatorsToBeRegistered, reflections);
}
/**
- * Get the validators that has to be run in the given list of
- * {@link ValidationCondition}s, for the given requestType and
+ * Get the validators that has to be run in the given list of,
+ * for the given requestType and for the given request versions.
* {@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
+ * @param requestType the type of the protocol message
+ * @param phase the request processing phase
+ * @param requestVersions different versions extracted from the request.
* @return the list of validation methods that has to run.
*/
- List<Method> validationsFor(
- List<ValidationCondition> conditions,
- Type requestType,
- RequestProcessingPhase phase) {
+ public List<Method> validationsFor(RequestType requestType,
RequestProcessingPhase phase,
+ Map<Class<? extends Annotation>, ? extends Versioned> requestVersions) {
+ return requestVersions.entrySet().stream()
+ .flatMap(requestVersion -> this.validationsFor(requestType, phase,
requestVersion.getKey(),
+ requestVersion.getValue()).stream())
+ .distinct().collect(Collectors.toList());
+ }
- if (conditions.isEmpty() || validators.isEmpty()) {
- return Collections.emptyList();
- }
+ /**
+ * Get the validators that has to be run in the given list of,
+ * for the given requestType and for the given request versions.
+ * {@link RequestProcessingPhase}.
+ *
+ * @param requestType the type of the protocol message
+ * @param phase the request processing phase
+ * @param validatorClass annotation class of the validator
+ * @param requestVersion version extracted corresponding to the request.
+ * @return the list of validation methods that has to run.
+ */
+ public <V extends Versioned> List<Method> validationsFor(RequestType
requestType,
+ RequestProcessingPhase phase, Class<? extends Annotation>
validatorClass, V requestVersion) {
- Set<Method> returnValue = new HashSet<>();
+ return Optional.ofNullable(this.indexedValidatorMap.get(validatorClass))
+ .map(requestTypeMap -> requestTypeMap.get(requestType))
+ .map(phaseMap -> phaseMap.get(phase))
+ .map(indexedMethods -> requestVersion.version() < 0 ?
+ indexedMethods.getItemsEqualToIdx(requestVersion) :
+ indexedMethods.getItemsGreaterThanIdx(requestVersion))
+ .orElse(Collections.emptyList());
- for (ValidationCondition condition: conditions) {
- returnValue.addAll(validationsFor(condition, requestType, phase));
- }
- return new ArrayList<>(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.
+ * Calls a specified method on the validator.
+ * @throws IllegalArgumentException when the specified method in the
validator is invalid.
*/
- private List<Method> validationsFor(
- ValidationCondition condition,
- Type requestType,
- RequestProcessingPhase phase) {
-
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>
- requestTypeMap = validators.get(condition);
- if (requestTypeMap == null || requestTypeMap.isEmpty()) {
- return Collections.emptyList();
+ private static <ReturnValue, Validator extends Annotation> ReturnValue
callAnnotationMethod(
+ Validator validator, String methodName, Class<ReturnValue>
returnValueType) {
+ try {
+ return
returnValueType.cast(validator.getClass().getMethod(methodName).invoke(validator));
+ } catch (NoSuchMethodException e) {
+ throw new IllegalArgumentException("Method " + methodName + " not found
in class:" +
+ validator.getClass().getCanonicalName(), e);
+ } catch (InvocationTargetException | IllegalAccessException e) {
+ throw new IllegalArgumentException("Error while invoking Method " +
methodName + " from " +
+ validator.getClass().getCanonicalName(), e);
}
+ }
- EnumMap<RequestProcessingPhase, List<Method>> phases =
- requestTypeMap.get(requestType);
- if (phases == null) {
- return Collections.emptyList();
+ private static Class<?> getReturnTypeOfAnnotationMethod(Class<? extends
Annotation> clzz, String methodName) {
+ try {
+ return clzz.getMethod(methodName).getReturnType();
+ } catch (NoSuchMethodException e) {
+ throw new IllegalArgumentException("Method " + methodName + " not found
in class:" + clzz.getCanonicalName());
}
+ }
- List<Method> validatorsForPhase = phases.get(phase);
- if (validatorsForPhase == null) {
- return Collections.emptyList();
+ private static <Validator extends Annotation> Versioned
getApplyBeforeVersion(Validator validator) {
+ return callAnnotationMethod(validator,
RegisterValidator.APPLY_BEFORE_METHOD_NAME, Versioned.class);
+ }
+
+ private static <Validator extends Annotation> RequestProcessingPhase
getRequestPhase(Validator validator) {
+ return callAnnotationMethod(validator,
RegisterValidator.PROCESSING_PHASE_METHOD_NAME,
+ RequestProcessingPhase.class);
+ }
+
+ private <Validator extends Annotation> RequestType[]
getRequestType(Validator validator,
+ Class<RequestType[]> requestType) {
+ return callAnnotationMethod(validator,
RegisterValidator.REQUEST_TYPE_METHOD_NAME, requestType);
+ }
+
+ private static <V> void checkAllowedAnnotationValues(Set<V> values, V value,
String valueName, String methodName) {
+ if (!values.contains(value)) {
+ throw new IllegalArgumentException(
+ String.format("Invalid %1$s defined at annotation defined for method
: %2$s, Annotation value : %3$s " +
+ "Allowed versionType: %4$s", valueName, methodName,
value.toString(), values));
}
- return validatorsForPhase;
}
/**
* 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 describedValidators collection of the annotated methods to process.
+ * - An EnumMap with the RequestType as the key, and in which
+ * - values are an EnumMap with the request processing phase as the key, and
in which
+ * - values is an {@link IndexedItems } containing the validation list
+ *
+ * @param validatorsToBeRegistered collection of the annotated validtors to
process.
*/
- void initMaps(Collection<Method> describedValidators) {
- for (Method m : describedValidators) {
- RequestFeatureValidator descriptor =
- m.getAnnotation(RequestFeatureValidator.class);
- m.setAccessible(true);
-
- for (ValidationCondition condition : descriptor.conditions()) {
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>
- requestTypeMap = getAndInitialize(
- condition, this::newTypeMap, validators);
- EnumMap<RequestProcessingPhase, List<Method>> phases =
getAndInitialize(
- descriptor.requestType(), this::newPhaseMap, requestTypeMap);
- if (isPreProcessValidator(descriptor)) {
- getAndInitialize(PRE_PROCESS, ArrayList::new, phases).add(m);
- } else if (isPostProcessValidator(descriptor)) {
- getAndInitialize(POST_PROCESS, ArrayList::new, phases).add(m);
- }
- }
+ private void initMaps(Class<RequestType[]> requestType,
+ Set<RequestProcessingPhase> allowedPhases,
+ Collection<Class<? extends Annotation>> validatorsToBeRegistered,
+ Reflections reflections) {
+ for (Class<? extends Annotation> validator : validatorsToBeRegistered) {
+ registerValidator(requestType, allowedPhases, validator, reflections);
}
}
- private EnumMap<Type,
- EnumMap<RequestProcessingPhase, List<Method>>> newTypeMap() {
- return new EnumMap<>(Type.class);
+ private void registerValidator(Class<RequestType[]> requestType,
+ Set<RequestProcessingPhase> allowedPhases,
+ Class<? extends Annotation> validatorToBeRegistered,
+ Reflections reflections) {
+ Collection<Method> methods =
reflections.getMethodsAnnotatedWith(validatorToBeRegistered);
+ List<Pair<? extends Annotation, Method>> sortedMethodsByApplyBeforeVersion
= methods.stream()
+ .map(method -> Pair.of(method.getAnnotation(validatorToBeRegistered),
method))
+ .sorted(Comparator.comparing(validatorMethodPair ->
getApplyBeforeVersion(validatorMethodPair.getKey()),
+ Versioned.versionComparator()))
+ .collect(Collectors.toList());
+ for (Pair<? extends Annotation, Method> validatorMethodPair :
sortedMethodsByApplyBeforeVersion) {
+ Annotation validator = validatorMethodPair.getKey();
+ Method method = validatorMethodPair.getValue();
+ Versioned applyBeforeVersion = this.getApplyBeforeVersion(validator);
+ RequestProcessingPhase phase = this.getRequestPhase(validator);
+ checkAllowedAnnotationValues(allowedPhases, phase,
RegisterValidator.PROCESSING_PHASE_METHOD_NAME,
+ method.getName());
+ Set<RequestType> types = Sets.newHashSet(this.getRequestType(validator,
requestType));
+ method.setAccessible(true);
+ for (RequestType type : types) {
+ EnumMap<RequestType, EnumMap<RequestProcessingPhase,
IndexedItems<Method, Versioned>>> requestMap =
+ this.indexedValidatorMap.get(validatorToBeRegistered);
+ EnumMap<RequestProcessingPhase, IndexedItems<Method, Versioned>>
phaseMap =
+ requestMap.computeIfAbsent(type, k -> new
EnumMap<>(RequestProcessingPhase.class));
+ phaseMap.computeIfAbsent(phase, k -> new
IndexedItems<>(Versioned.versionComparator()))
+ .add(method, applyBeforeVersion);
+ }
+ }
}
- private EnumMap<RequestProcessingPhase, List<Method>> newPhaseMap() {
- return new EnumMap<>(RequestProcessingPhase.class);
- }
+ /**
+ * Class responsible for maintaining indexs of items. Here each item should
have an index corresponding to it.
+ * The class implements functions for efficiently fetching range gets on the
items added to the data structure.
+ *
+ * @param <T> Refers to the Type of the item in the data structure
+ * @param <IDX> Type of the index of an item added in the data structure. It
is important that the index is
+ * comparable to each other.
+ */
+ private static final class IndexedItems<T, IDX> {
+ private final List<T> items;
+ private final NavigableMap<IDX, Integer> indexMap;
- private <K, V> V getAndInitialize(K key, Supplier<V> defaultSupplier, Map<K,
V> from) {
- return from.computeIfAbsent(key, k -> defaultSupplier.get());
- }
+ private IndexedItems(Comparator<IDX> comparator) {
+ this.items = new ArrayList<>();
+ this.indexMap = new TreeMap<>(comparator);
+ }
- private boolean isPreProcessValidator(RequestFeatureValidator descriptor) {
- return descriptor.processingPhase()
- .equals(PRE_PROCESS);
- }
+ /**
+ * Add an item to the collection and update index if required. The order
of items added should have their index
+ * sorted in increasing order.
+ *
+ * @param item
+ * @param idx
Review Comment:
Missing Javadoc for parameters. The add method should document what the item
and idx parameters represent.
```suggestion
* @param item the item to add to this collection
* @param idx the index associated with the item; indices must be
provided in increasing order
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java:
##########
@@ -42,156 +50,265 @@
* Registry that loads and stores the request validators to be applied by
* a service.
*/
-public class ValidatorRegistry {
+public class ValidatorRegistry<RequestType extends Enum<RequestType>> {
- private final EnumMap<ValidationCondition,
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>>
- validators = new EnumMap<>(ValidationCondition.class);
+ /**
+ * A validator registered should have the following parameters:
+ * applyBeforeVersion: Enum extending Version
+ * RequestType: Enum signifying the type of request.
+ * RequestProcessingPhase: Signifying if the validator is supposed to run
pre or post submitting the request.
+ * Based on the afforementioned parameters a complete map is built which
stores the validators in a sorted order of
+ * the applyBeforeVersion value of the validator method.
+ * Thus when a request comes with a certain version value, all validators
containing `applyBeforeVersion` parameter
+ * greater than the request versions get triggered.
+ * {@link #validationsFor(Enum, RequestProcessingPhase, Class, Versioned)}
+ */
+ private final Map<Class<? extends Annotation>, EnumMap<RequestType,
+ EnumMap<RequestProcessingPhase, IndexedItems<Method, Versioned>>>>
indexedValidatorMap;
/**
* Creates a {@link ValidatorRegistry} instance that discovers validation
* 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.
- * @param validatorPackage the main package inside which validatiors should
- * be discovered.
+ * A validation method is recognized by all the annotations classes which
+ * are annotated by {@link RegisterValidator} annotation that contains
+ * important information about how and when to use the validator.
+ *
+ * @param requestType class of request type enum.
+ * @param validatorPackage the main package inside which validatiors
should
+ * be discovered.
+ * @param allowedValidators a set containing the various types of
version allowed to be registered.
+ * @param allowedProcessingPhases set of request processing phases which
would be allowed to be registered to
+ * registry.
*/
- ValidatorRegistry(String validatorPackage) {
- this(ClasspathHelper.forPackage(validatorPackage));
+ public ValidatorRegistry(Class<RequestType> requestType, String
validatorPackage,
+ Set<Class<? extends Annotation>> allowedValidators,
Set<RequestProcessingPhase> allowedProcessingPhases) {
+ this(requestType, ClasspathHelper.forPackage(validatorPackage),
allowedValidators, allowedProcessingPhases);
}
/**
* Creates a {@link ValidatorRegistry} instance that discovers validation
* methods under the provided URL.
- * A validation method is recognized by the {@link RequestFeatureValidator}
+ * A validation method is recognized by all annotations annotated by the
{@link RegisterValidator}
* annotation that contains important information about how and when to use
* the validator.
- * @param searchUrls the path in which the annotated methods are searched.
+ *
+ * @param requestType class of request type enum.
+ * @param searchUrls the path in which the annotated methods
are searched.
+ * @param allowedValidators a set containing the various types of
validator annotation allowed to be registered.
+ * @param allowedProcessingPhases set of request processing phases which
would be allowed to be registered to
+ * the registry.
*/
- ValidatorRegistry(Collection<URL> searchUrls) {
+ ValidatorRegistry(Class<RequestType> requestType, Collection<URL> searchUrls,
+ Set<Class<? extends Annotation>> allowedValidators,
+ Set<RequestProcessingPhase> allowedProcessingPhases) {
+ Class<RequestType[]> requestArrayClass = (Class<RequestType[]>)
Array.newInstance(requestType, 0)
+ .getClass();
+ Set<Class<? extends Annotation>> validatorsToBeRegistered =
+ new Reflections(new
ConfigurationBuilder().setUrls(ClasspathHelper.forPackage("org.apache.hadoop"))
+ .setScanners(Scanners.TypesAnnotated)
+
.setParallel(true)).getTypesAnnotatedWith(RegisterValidator.class).stream()
+ .filter(allowedValidators::contains)
+ .filter(annotationClass ->
getReturnTypeOfAnnotationMethod((Class<? extends Annotation>) annotationClass,
+ RegisterValidator.REQUEST_TYPE_METHOD_NAME)
+ .equals(requestArrayClass))
+ .map(annotationClass -> (Class<? extends Annotation>)
annotationClass)
+ .collect(Collectors.toSet());
+ this.indexedValidatorMap =
allowedValidators.stream().collect(ImmutableMap.toImmutableMap(Function.identity(),
+ validatorClass -> new EnumMap<>(requestType)));
Reflections reflections = new Reflections(new ConfigurationBuilder()
.setUrls(searchUrls)
.setScanners(Scanners.MethodsAnnotated)
.setParallel(true)
);
-
- Set<Method> describedValidators =
- reflections.getMethodsAnnotatedWith(RequestFeatureValidator.class);
- initMaps(describedValidators);
+ initMaps(requestArrayClass, allowedProcessingPhases,
validatorsToBeRegistered, reflections);
}
/**
- * Get the validators that has to be run in the given list of
- * {@link ValidationCondition}s, for the given requestType and
+ * Get the validators that has to be run in the given list of,
+ * for the given requestType and for the given request versions.
* {@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
+ * @param requestType the type of the protocol message
+ * @param phase the request processing phase
+ * @param requestVersions different versions extracted from the request.
* @return the list of validation methods that has to run.
*/
- List<Method> validationsFor(
- List<ValidationCondition> conditions,
- Type requestType,
- RequestProcessingPhase phase) {
+ public List<Method> validationsFor(RequestType requestType,
RequestProcessingPhase phase,
+ Map<Class<? extends Annotation>, ? extends Versioned> requestVersions) {
+ return requestVersions.entrySet().stream()
+ .flatMap(requestVersion -> this.validationsFor(requestType, phase,
requestVersion.getKey(),
+ requestVersion.getValue()).stream())
+ .distinct().collect(Collectors.toList());
+ }
- if (conditions.isEmpty() || validators.isEmpty()) {
- return Collections.emptyList();
- }
+ /**
+ * Get the validators that has to be run in the given list of,
+ * for the given requestType and for the given request versions.
+ * {@link RequestProcessingPhase}.
+ *
+ * @param requestType the type of the protocol message
+ * @param phase the request processing phase
+ * @param validatorClass annotation class of the validator
+ * @param requestVersion version extracted corresponding to the request.
+ * @return the list of validation methods that has to run.
+ */
+ public <V extends Versioned> List<Method> validationsFor(RequestType
requestType,
+ RequestProcessingPhase phase, Class<? extends Annotation>
validatorClass, V requestVersion) {
- Set<Method> returnValue = new HashSet<>();
+ return Optional.ofNullable(this.indexedValidatorMap.get(validatorClass))
+ .map(requestTypeMap -> requestTypeMap.get(requestType))
+ .map(phaseMap -> phaseMap.get(phase))
+ .map(indexedMethods -> requestVersion.version() < 0 ?
+ indexedMethods.getItemsEqualToIdx(requestVersion) :
+ indexedMethods.getItemsGreaterThanIdx(requestVersion))
+ .orElse(Collections.emptyList());
- for (ValidationCondition condition: conditions) {
- returnValue.addAll(validationsFor(condition, requestType, phase));
- }
- return new ArrayList<>(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.
+ * Calls a specified method on the validator.
+ * @throws IllegalArgumentException when the specified method in the
validator is invalid.
*/
- private List<Method> validationsFor(
- ValidationCondition condition,
- Type requestType,
- RequestProcessingPhase phase) {
-
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>
- requestTypeMap = validators.get(condition);
- if (requestTypeMap == null || requestTypeMap.isEmpty()) {
- return Collections.emptyList();
+ private static <ReturnValue, Validator extends Annotation> ReturnValue
callAnnotationMethod(
+ Validator validator, String methodName, Class<ReturnValue>
returnValueType) {
+ try {
+ return
returnValueType.cast(validator.getClass().getMethod(methodName).invoke(validator));
+ } catch (NoSuchMethodException e) {
+ throw new IllegalArgumentException("Method " + methodName + " not found
in class:" +
+ validator.getClass().getCanonicalName(), e);
+ } catch (InvocationTargetException | IllegalAccessException e) {
+ throw new IllegalArgumentException("Error while invoking Method " +
methodName + " from " +
+ validator.getClass().getCanonicalName(), e);
}
+ }
- EnumMap<RequestProcessingPhase, List<Method>> phases =
- requestTypeMap.get(requestType);
- if (phases == null) {
- return Collections.emptyList();
+ private static Class<?> getReturnTypeOfAnnotationMethod(Class<? extends
Annotation> clzz, String methodName) {
+ try {
+ return clzz.getMethod(methodName).getReturnType();
+ } catch (NoSuchMethodException e) {
+ throw new IllegalArgumentException("Method " + methodName + " not found
in class:" + clzz.getCanonicalName());
}
+ }
- List<Method> validatorsForPhase = phases.get(phase);
- if (validatorsForPhase == null) {
- return Collections.emptyList();
+ private static <Validator extends Annotation> Versioned
getApplyBeforeVersion(Validator validator) {
+ return callAnnotationMethod(validator,
RegisterValidator.APPLY_BEFORE_METHOD_NAME, Versioned.class);
+ }
+
+ private static <Validator extends Annotation> RequestProcessingPhase
getRequestPhase(Validator validator) {
+ return callAnnotationMethod(validator,
RegisterValidator.PROCESSING_PHASE_METHOD_NAME,
+ RequestProcessingPhase.class);
+ }
+
+ private <Validator extends Annotation> RequestType[]
getRequestType(Validator validator,
+ Class<RequestType[]> requestType) {
+ return callAnnotationMethod(validator,
RegisterValidator.REQUEST_TYPE_METHOD_NAME, requestType);
+ }
+
+ private static <V> void checkAllowedAnnotationValues(Set<V> values, V value,
String valueName, String methodName) {
+ if (!values.contains(value)) {
+ throw new IllegalArgumentException(
+ String.format("Invalid %1$s defined at annotation defined for method
: %2$s, Annotation value : %3$s " +
+ "Allowed versionType: %4$s", valueName, methodName,
value.toString(), values));
}
- return validatorsForPhase;
}
/**
* 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 describedValidators collection of the annotated methods to process.
+ * - An EnumMap with the RequestType as the key, and in which
+ * - values are an EnumMap with the request processing phase as the key, and
in which
+ * - values is an {@link IndexedItems } containing the validation list
+ *
+ * @param validatorsToBeRegistered collection of the annotated validtors to
process.
Review Comment:
Typo in the word "validtors". It should be "validators".
```suggestion
* @param validatorsToBeRegistered collection of the annotated validators
to process.
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java:
##########
@@ -69,123 +81,129 @@ public void tearDown() {
finishValidatorTest();
}
+ private List<Method> getValidatorsForRequest(OzoneManagerProtocolProtos.Type
requestType,
+ RequestProcessingPhase phase, VersionExtractor extractor, Versioned
versioned) {
+ return getValidatorsForRequest(PACKAGE, requestType, phase,
Collections.singletonMap(extractor, versioned));
+ }
+
+ private List<Method> getValidatorsForRequest(OzoneManagerProtocolProtos.Type
requestType,
+ RequestProcessingPhase phase, Map<VersionExtractor, Versioned>
requestVersions) {
+ return getValidatorsForRequest(PACKAGE, requestType, phase,
requestVersions);
+ }
+
+ private List<Method> getValidatorsForRequest(String packageName,
OzoneManagerProtocolProtos.Type requestType,
+ RequestProcessingPhase phase, Map<VersionExtractor, Versioned>
requestVersions) {
+ ValidatorRegistry<OzoneManagerProtocolProtos.Type> registry =
+ new ValidatorRegistry<>(OzoneManagerProtocolProtos.Type.class,
packageName,
+
Arrays.stream(VersionExtractor.values()).map(VersionExtractor::getValidatorClass)
+ .collect(Collectors.toSet()), REQUEST_PROCESSING_PHASES);
+ return registry.validationsFor(requestType, phase,
requestVersions.entrySet().stream()
+ .collect(Collectors.toMap(e -> e.getKey().getValidatorClass(),
Map.Entry::getValue)));
+ }
+
@Test
public void testNoValidatorsReturnedForEmptyConditionList() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(emptyList(), CreateKey, PRE_PROCESS);
-
+ List<Method> validators = getValidatorsForRequest(CreateKey, PRE_PROCESS,
+ CLIENT_VERSION_EXTRACTOR, ClientVersion.CURRENT);
assertTrue(validators.isEmpty());
}
@Test
public void testRegistryHasThePreFinalizePreProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(CLUSTER_NEEDS_FINALIZATION), CreateKey, PRE_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, PRE_PROCESS,
+ ImmutableMap.of(CLIENT_VERSION_EXTRACTOR, ClientVersion.CURRENT,
+ LAYOUT_VERSION_EXTRACTOR, OMLayoutFeature.FILESYSTEM_SNAPSHOT));
assertEquals(1, validators.size());
- String expectedMethodName = "preFinalizePreProcessCreateKeyValidator";
+ String expectedMethodName = "preProcessCreateKeyQuotaLayoutValidator";
assertEquals(expectedMethodName, validators.get(0).getName());
}
@Test
public void testRegistryHasThePreFinalizePostProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(CLUSTER_NEEDS_FINALIZATION), CreateKey, POST_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, POST_PROCESS,
+ ImmutableMap.of(CLIENT_VERSION_EXTRACTOR, ClientVersion.CURRENT,
+ LAYOUT_VERSION_EXTRACTOR, OMLayoutFeature.BUCKET_LAYOUT_SUPPORT));
assertEquals(1, validators.size());
- String expectedMethodName = "preFinalizePostProcessCreateKeyValidator";
+ String expectedMethodName = "postProcessCreateKeyQuotaLayoutValidator";
assertEquals(expectedMethodName, validators.get(0).getName());
}
@Test
public void testRegistryHasTheOldClientPreProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(OLDER_CLIENT_REQUESTS), CreateKey, PRE_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, PRE_PROCESS,
CLIENT_VERSION_EXTRACTOR,
+ ClientVersion.ERASURE_CODING_SUPPORT);
assertEquals(2, validators.size());
List<String> methodNames =
validators.stream().map(Method::getName).collect(Collectors.toList());
- assertThat(methodNames).contains("oldClientPreProcessCreateKeyValidator");
- assertThat(methodNames).contains("oldClientPreProcessCreateKeyValidator2");
+
assertEquals(Arrays.asList("preProcessCreateKeyBucketLayoutClientValidator",
+ "preProcessCreateKeyBucketLayoutClientValidator"), methodNames);
}
@Test
public void testRegistryHasTheOldClientPostProcessCreateKeyValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> validators =
- registry.validationsFor(
- asList(OLDER_CLIENT_REQUESTS), CreateKey, POST_PROCESS);
+ List<Method> validators = getValidatorsForRequest(CreateKey, POST_PROCESS,
+ CLIENT_VERSION_EXTRACTOR, ClientVersion.ERASURE_CODING_SUPPORT);
- assertEquals(2, validators.size());
+ assertEquals(1, validators.size());
List<String> methodNames =
validators.stream().map(Method::getName).collect(Collectors.toList());
- assertThat(methodNames).contains("oldClientPostProcessCreateKeyValidator");
-
assertThat(methodNames).contains("oldClientPostProcessCreateKeyValidator2");
+
assertThat(methodNames).contains("postProcessCreateKeyBucketLayoutClientValidator");
}
@Test
public void testRegistryHasTheMultiPurposePreProcessCreateVolumeValidator() {
- ValidatorRegistry registry = new ValidatorRegistry(PACKAGE);
- List<Method> preFinalizeValidators =
- registry.validationsFor(
- asList(CLUSTER_NEEDS_FINALIZATION), CreateVolume, PRE_PROCESS);
- List<Method> newClientValidators =
- registry.validationsFor(
- asList(OLDER_CLIENT_REQUESTS), CreateVolume, PRE_PROCESS);
+ List<Method> preFinalizeValidators = getValidatorsForRequest(CreateVolume,
PRE_PROCESS, LAYOUT_VERSION_EXTRACTOR,
+ OMLayoutFeature.HSYNC);
+ List<Method> newClientValidators = getValidatorsForRequest(CreateVolume,
PRE_PROCESS, CLIENT_VERSION_EXTRACTOR,
+ ClientVersion.ERASURE_CODING_SUPPORT);
assertEquals(1, preFinalizeValidators.size());
assertEquals(1, newClientValidators.size());
- String expectedMethodName = "multiPurposePreProcessCreateVolumeValidator";
+ String expectedMethodName =
"multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator";
Review Comment:
Typo in the expected method name
"multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The
word "CLient" should be "Client" (lowercase 'l').
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java:
##########
@@ -88,82 +89,90 @@ private static void fireValidationEvent(String
calledMethodName) {
listeners.forEach(l -> l.validationCalled(calledMethodName));
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = PRE_PROCESS,
requestType = CreateKey)
- public static OMRequest preFinalizePreProcessCreateKeyValidator(
+ public static OMRequest preProcessCreateKeyQuotaLayoutValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("preFinalizePreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyQuotaLayoutValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.FUTURE_VERSION,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateKey)
+ public static OMRequest preProcessCreateKeyFutureClientValidator(
+ OMRequest req, ValidationContext ctx) {
+ fireValidationEvent("preProcessCreateKeyFutureClientValidator");
+ return req;
+ }
+
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse preFinalizePostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyQuotaLayoutValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("preFinalizePostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyQuotaLayoutValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
processingPhase = PRE_PROCESS,
- requestType = CreateKey)
- public static OMRequest oldClientPreProcessCreateKeyValidator(
+ requestType = CreateKey,
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT)
+ public static OMRequest preProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("oldClientPreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyBucketLayoutClientValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse oldClientPostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("oldClientPostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyBucketLayoutClientValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION, OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = PRE_PROCESS,
requestType = CreateVolume)
- public static OMRequest multiPurposePreProcessCreateVolumeValidator(
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateVolume)
+ public static OMRequest
multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("multiPurposePreProcessCreateVolumeValidator");
+
fireValidationEvent("multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator");
Review Comment:
Typo in the method name
"multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The
word "CLient" should be "Client" (lowercase 'l').
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/package-info.java:
##########
@@ -28,25 +28,29 @@
* 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
+ * - a client connects to the server and performs an operation corresponding
+ * to a feature the server hasn't finalized for which these requests might
have
+ * to be rejected.
* - 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.
+ * These are the current registered
+ * {@link org.apache.hadoop.ozone.om.request.validation.VersionExtractor}s
+ * which would be extracted out of the om request and all validators
+ * fulfilling the condition would be run.
*
- * The system uses a reflection based discovery to find methods that are
+ * The system uses a reflection based discovery to find annotations that are
* annotated with the
- * {@link
org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator}
+ * {@link org.apache.hadoop.ozone.request.validation.RegisterValidator}
* 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
- * }.
+ * This annotation is used to register a particular annotation which in turn
would be used to specify
+ * the request type to which the validation should be applied,
+ * and the request processing phase in which we apply the validation and the
maxVersion corresponding to which this
+ * is supposed to run.
+ *
+ * One validator can be applied based on multiple trigger conditions, E.g.
+ * A validator registered can have both {@link
org.apache.hadoop.ozone.om.request.validation.OMClientVersionValidator},
+ * {@link
org.apache.hadoop.ozone.om.request.validation.OMLayoutVersionValidator}
+ *
* 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.
Review Comment:
Typo in "reql". It should be "real".
```suggestion
* any complex validation has to happen in the real request handling.
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java:
##########
@@ -42,156 +50,265 @@
* Registry that loads and stores the request validators to be applied by
* a service.
*/
-public class ValidatorRegistry {
+public class ValidatorRegistry<RequestType extends Enum<RequestType>> {
- private final EnumMap<ValidationCondition,
- EnumMap<Type, EnumMap<RequestProcessingPhase, List<Method>>>>
- validators = new EnumMap<>(ValidationCondition.class);
+ /**
+ * A validator registered should have the following parameters:
+ * applyBeforeVersion: Enum extending Version
+ * RequestType: Enum signifying the type of request.
+ * RequestProcessingPhase: Signifying if the validator is supposed to run
pre or post submitting the request.
+ * Based on the afforementioned parameters a complete map is built which
stores the validators in a sorted order of
+ * the applyBeforeVersion value of the validator method.
+ * Thus when a request comes with a certain version value, all validators
containing `applyBeforeVersion` parameter
+ * greater than the request versions get triggered.
+ * {@link #validationsFor(Enum, RequestProcessingPhase, Class, Versioned)}
+ */
+ private final Map<Class<? extends Annotation>, EnumMap<RequestType,
+ EnumMap<RequestProcessingPhase, IndexedItems<Method, Versioned>>>>
indexedValidatorMap;
/**
* Creates a {@link ValidatorRegistry} instance that discovers validation
* 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.
- * @param validatorPackage the main package inside which validatiors should
- * be discovered.
+ * A validation method is recognized by all the annotations classes which
+ * are annotated by {@link RegisterValidator} annotation that contains
+ * important information about how and when to use the validator.
+ *
+ * @param requestType class of request type enum.
+ * @param validatorPackage the main package inside which validatiors
should
Review Comment:
Typo in the word "validatiors". It should be "validators".
```suggestion
* @param validatorPackage the main package inside which validators
should
```
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java:
##########
@@ -88,82 +89,90 @@ private static void fireValidationEvent(String
calledMethodName) {
listeners.forEach(l -> l.validationCalled(calledMethodName));
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = PRE_PROCESS,
requestType = CreateKey)
- public static OMRequest preFinalizePreProcessCreateKeyValidator(
+ public static OMRequest preProcessCreateKeyQuotaLayoutValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("preFinalizePreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyQuotaLayoutValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.FUTURE_VERSION,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateKey)
+ public static OMRequest preProcessCreateKeyFutureClientValidator(
+ OMRequest req, ValidationContext ctx) {
+ fireValidationEvent("preProcessCreateKeyFutureClientValidator");
+ return req;
+ }
+
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse preFinalizePostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyQuotaLayoutValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("preFinalizePostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyQuotaLayoutValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
processingPhase = PRE_PROCESS,
- requestType = CreateKey)
- public static OMRequest oldClientPreProcessCreateKeyValidator(
+ requestType = CreateKey,
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT)
+ public static OMRequest preProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("oldClientPreProcessCreateKeyValidator");
+ fireValidationEvent("preProcessCreateKeyBucketLayoutClientValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = POST_PROCESS,
requestType = CreateKey)
- public static OMResponse oldClientPostProcessCreateKeyValidator(
+ public static OMResponse postProcessCreateKeyBucketLayoutClientValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("oldClientPostProcessCreateKeyValidator");
+ fireValidationEvent("postProcessCreateKeyBucketLayoutClientValidator");
return resp;
}
- @RequestFeatureValidator(
- conditions = { CLUSTER_NEEDS_FINALIZATION, OLDER_CLIENT_REQUESTS },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = PRE_PROCESS,
requestType = CreateVolume)
- public static OMRequest multiPurposePreProcessCreateVolumeValidator(
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
+ processingPhase = PRE_PROCESS,
+ requestType = CreateVolume)
+ public static OMRequest
multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
OMRequest req, ValidationContext ctx) {
- fireValidationEvent("multiPurposePreProcessCreateVolumeValidator");
+
fireValidationEvent("multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator");
return req;
}
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS, CLUSTER_NEEDS_FINALIZATION },
+ @OMClientVersionValidator(
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT,
processingPhase = POST_PROCESS,
requestType = CreateVolume)
- public static OMResponse multiPurposePostProcessCreateVolumeValidator(
- OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("multiPurposePostProcessCreateVolumeValidator");
- return resp;
- }
-
- @RequestFeatureValidator(
- conditions = { OLDER_CLIENT_REQUESTS },
+ @OMLayoutVersionValidator(
+ applyBefore = OMLayoutFeature.QUOTA,
processingPhase = POST_PROCESS,
- requestType = CreateKey)
- public static OMResponse oldClientPostProcessCreateKeyValidator2(
+ requestType = CreateVolume)
+ public static OMResponse
multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
OMRequest req, OMResponse resp, ValidationContext ctx) {
- fireValidationEvent("oldClientPostProcessCreateKeyValidator2");
+
fireValidationEvent("multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator");
Review Comment:
Typo in the method name
"multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator".
The word "CLient" should be "Client" (lowercase 'l').
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
