HDDS-12983. Validator Registry Changes for Supporting Version based validations #8404
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alidations Change-Id: I939888c989eaec0a62884aa564e2f24a594122ef
Change-Id: Icec10ce7bb452753db0936212b0757c01279b533
Change-Id: I9a010c86d0128da8140fa33b46eaeef9da0dc283
adoroszlai
reviewed
May 14, 2025
Comment on lines
138
to
140
| public List<Method> validationsFor(RequestType requestType, | ||
| RequestProcessingPhase phase, | ||
| Map<Class<? extends Annotation>, ? extends Versioned> requestVersions) { |
There was a problem hiding this comment.
Please do not format method signature like this. Whenever visibility / return type / method name / other modifiers are changed, we would have to reindent all parameters.
Comment on lines
177
to
179
| Validator validator, String methodName, Class<ReturnValue> returnValueType) { | ||
| try { | ||
| return (ReturnValue) validator.getClass().getMethod(methodName).invoke(validator); |
There was a problem hiding this comment.
Use returnValueType (currently unused) to cast the result, instead of using unchecked cast (ReturnValue).
| processingPhase = RequestProcessingPhase.PRE_PROCESS, | ||
| requestType = Type.CreateBucket | ||
| requestType = Type.CreateBucket, | ||
| applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT |
There was a problem hiding this comment.
nit:
Suggested change
| applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT | |
| applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT |
Comment on lines
50
to
52
| * 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} |
There was a problem hiding this comment.
I don't think this sentence makes sense.
There was a problem hiding this comment.
updated the sentence lmk if it is ok now
Comment on lines
143
to
146
| ValidatorRegistry<OzoneManagerProtocolProtos.Type> registry = | ||
| new ValidatorRegistry<>(OzoneManagerProtocolProtos.Type.class, PACKAGE, | ||
| Arrays.stream(VersionExtractor.values()).map(VersionExtractor::getValidatorClass).collect(Collectors.toSet()), | ||
| REQUEST_PROCESSING_PHASES); |
There was a problem hiding this comment.
Can we avoid duplicating this logic in each test case?
Comment on lines
309
to
311
| * @return All the items which has an index value greater than given index value. | ||
| */ | ||
| public List<T> getItemsEqualToIdx(IDX indexValue) { |
There was a problem hiding this comment.
Javadoc mismatch ("greater than" vs. EqualTo)
| */ | ||
| private static final class IndexedItems<T, IDX extends Comparable<IDX>> { | ||
| private final List<T> items; | ||
| private final TreeMap<IDX, Integer> indexMap; |
There was a problem hiding this comment.
Declare as interface: NavigableMap.
Comment on lines
246
to
249
| .sorted((validatorMethodPair1, validatorMethodPair2) -> | ||
| Integer.compare( | ||
| this.getApplyBeforeVersion(validatorMethodPair1.getKey()).version(), | ||
| this.getApplyBeforeVersion(validatorMethodPair2.getKey()).version())) |
There was a problem hiding this comment.
Extract this lambda to a static method.
Comment on lines
78
to
79
| List<Method> validations = registry.validationsFor(request.getCmdType(), PRE_PROCESS, | ||
| this.getVersions(request)); |
There was a problem hiding this comment.
nit: remove this. and do not wrap line.
Change-Id: Ibf524d42e3d8535377105a07f8ca3213c8c62199
Change-Id: I67634e5517239bdec3965ed8819f9af6ecf577dd
Change-Id: Idf26bbc7e83877a71f46bafdf0216d70733a92ee
Change-Id: Idecefb392dbf5c01227d7c3702db1781b9edb073
Change-Id: I174cea546d6fd021e58966b3a5bca3672e9d567e # Conflicts: # hadoop-ozone/client/pom.xml # hadoop-ozone/interface-storage/pom.xml # hadoop-ozone/recon-codegen/pom.xml # hadoop-ozone/s3gateway/pom.xml
Change-Id: I132006b2b5a638e7f19d9ec1beb33b332d105e98
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Making changes to the ValidatorRegistry class to be able to pull the validators registered and inturn being able to get all the underlying validator methods based on the usage of the registered feature validator based on a specified version, request processing phase and request type efficiently.
This is the last PR for completing the request validator fixes after breaking down
#6932 into smaller PRs
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12983
How was this patch tested?
Adding unit tests