swamirishi commented on code in PR #8404:
URL: https://github.com/apache/ozone/pull/8404#discussion_r2103208269
##########
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 in multiple, E.g.
+ * {@link
org.apache.hadoop.ozone.om.request.validation.OMClientVersionValidator},
+ * {@link
org.apache.hadoop.ozone.om.request.validation.OMLayoutVersionValidator}
Review Comment:
updated the sentence lmk if it is ok now
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java:
##########
@@ -461,10 +461,10 @@ public static OMRequest
handleCreateBucketWithBucketLayoutDuringPreFinalize(
* write to them, instead of using the server default which may be in a
layout
* they do not understand.
*/
- @RequestFeatureValidator(
- conditions = ValidationCondition.OLDER_CLIENT_REQUESTS,
+ @OMClientVersionValidator(
processingPhase = RequestProcessingPhase.PRE_PROCESS,
- requestType = Type.CreateBucket
+ requestType = Type.CreateBucket,
+ applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT
Review Comment:
done
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java:
##########
@@ -42,156 +47,274 @@
* 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 Versioned>, EnumMap<RequestType,
+ EnumMap<RequestProcessingPhase, IndexedItems<Method, Integer>>>>
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.
+ * 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 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
+ * registry.
*/
- ValidatorRegistry(Collection<URL> searchUrls) {
+ public 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(annotationClass
->
+ (Class<? extends Versioned>)
getReturnTypeOfAnnotationMethod(annotationClass,
+ RegisterValidator.APPLY_BEFORE_METHOD_NAME),
+ 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 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) {
-
- if (conditions.isEmpty() || validators.isEmpty()) {
- return Collections.emptyList();
- }
-
- Set<Method> returnValue = new HashSet<>();
-
- for (ValidationCondition condition: conditions) {
- returnValue.addAll(validationsFor(condition, requestType, phase));
- }
- return new ArrayList<>(returnValue);
+ 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());
}
/**
- * Grabs validations for one particular condition.
+ * 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 condition conditions that are present for the request
* @param requestType the type of the protocol message
* @param phase the request processing phase
+ * @param requestVersion version extracted corresponding to the request.
* @return the list of validation methods that has to run.
*/
- 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();
+ public <V extends Versioned> List<Method> validationsFor(RequestType
requestType,
+
RequestProcessingPhase phase,
+ Class<? extends
Annotation> validatorClass,
+ V requestVersion) {
+
+ return
Optional.ofNullable(this.indexedValidatorMap.get(requestVersion.getClass()))
+ .map(requestTypeMap -> requestTypeMap.get(requestType))
+ .map(phaseMap -> phaseMap.get(phase))
+ .map(indexedMethods -> requestVersion.version() < 0 ?
+ indexedMethods.getItemsEqualToIdx(requestVersion.version()) :
+ indexedMethods.getItemsGreaterThanIdx(requestVersion.version()))
+ .orElse(Collections.emptyList());
+
+ }
+
+ /**
+ * Calls a specified method on the validator.
+ * @Throws IllegalArgumentException when the specified method in the
validator is invalid.
+ */
+ private <ReturnValue, Validator extends Annotation> ReturnValue
callAnnotationMethod(
+ Validator validator, String methodName, Class<ReturnValue>
returnValueType) {
+ try {
+ return (ReturnValue)
validator.getClass().getMethod(methodName).invoke(validator);
Review Comment:
done
--
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: issues-unsubscr...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
