HDDS-12983. Validator Registry Changes for Supporting Version based validations by swamirishi · Pull Request #8404 · apache/ozone

swamirishi · 2025-05-06T20:44:18Z

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

…alidations Change-Id: I939888c989eaec0a62884aa564e2f24a594122ef

Change-Id: Icec10ce7bb452753db0936212b0757c01279b533

Change-Id: I9a010c86d0128da8140fa33b46eaeef9da0dc283

adoroszlai

Thanks @swamirishi for the patch.

adoroszlai · 2025-05-14T09:07:42Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

+  public List<Method> validationsFor(RequestType requestType,
+                                     RequestProcessingPhase phase,
+                                     Map<Class<? extends Annotation>, ? extends Versioned> requestVersions) {


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.

adoroszlai · 2025-05-14T09:09:08Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

+      Validator validator, String methodName, Class<ReturnValue> returnValueType) {
+    try {
+      return (ReturnValue) validator.getClass().getMethod(methodName).invoke(validator);


Use returnValueType (currently unused) to cast the result, instead of using unchecked cast (ReturnValue).

adoroszlai · 2025-05-14T09:12:13Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java

      processingPhase = RequestProcessingPhase.PRE_PROCESS,
-      requestType = Type.CreateBucket
+      requestType = Type.CreateBucket,
+      applyBefore =  ClientVersion.BUCKET_LAYOUT_SUPPORT


nit:

Suggested change

applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT

applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT

adoroszlai · 2025-05-14T09:51:55Z

.../ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/package-info.java

+ * 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}


I don't think this sentence makes sense.

updated the sentence lmk if it is ok now

adoroszlai · 2025-05-14T09:56:44Z

...nager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java

+    ValidatorRegistry<OzoneManagerProtocolProtos.Type> registry =
+        new ValidatorRegistry<>(OzoneManagerProtocolProtos.Type.class, PACKAGE,
+        Arrays.stream(VersionExtractor.values()).map(VersionExtractor::getValidatorClass).collect(Collectors.toSet()),
+        REQUEST_PROCESSING_PHASES);


Can we avoid duplicating this logic in each test case?

adoroszlai · 2025-05-14T10:01:10Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

+     * @return All the items which has an index value greater than given index value.
+     */
+    public List<T> getItemsEqualToIdx(IDX indexValue) {


Javadoc mismatch ("greater than" vs. EqualTo)

adoroszlai · 2025-05-14T10:02:10Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

+   */
+  private static final class IndexedItems<T, IDX extends Comparable<IDX>> {
+    private final List<T> items;
+    private final TreeMap<IDX, Integer> indexMap;


Declare as interface: NavigableMap.

adoroszlai · 2025-05-14T10:02:56Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

+        .sorted((validatorMethodPair1, validatorMethodPair2) ->
+            Integer.compare(
+                this.getApplyBeforeVersion(validatorMethodPair1.getKey()).version(),
+                this.getApplyBeforeVersion(validatorMethodPair2.getKey()).version()))


Extract this lambda to a static method.

adoroszlai · 2025-05-14T10:06:32Z

...-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java

+    List<Method> validations = registry.validationsFor(request.getCmdType(), PRE_PROCESS,
+        this.getVersions(request));


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

github-actions · 2025-11-12T00:05:28Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

github-actions · 2025-11-19T00:05:44Z

Thank you for your contribution. This PR is being closed due to inactivity. If needed, feel free to reopen it.

Change-Id: Ie1076419cae849563efc8a14ac97e5c0c83828a9 # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRenameRequest.java

Change-Id: I461d46461905d041a613b101182d24e18a143d5c

adoroszlai · 2025-12-19T13:19:41Z

...-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMDirectoryCreateRequest.java

+  @OMLayoutVersionValidator(
      processingPhase = RequestProcessingPhase.PRE_PROCESS,
-      requestType = Type.CreateDirectory
+      requestType = Type.CreateDirectory,
+      applyBefore = OMLayoutFeature.ERASURE_CODED_STORAGE_SUPPORT


Please simply replace the two changed lines instead of moving things around.

- @RequestFeatureValidator( - conditions = ValidationCondition.CLUSTER_NEEDS_FINALIZATION, + @OMLayoutVersionValidator( + applyBefore = OMLayoutFeature.ERASURE_CODED_STORAGE_SUPPORT,

adoroszlai · 2025-12-19T13:51:16Z

hadoop-hdds/client/pom.xml

-                    <bannedImport>org.apache.hadoop.ozone.om.request.validation.RequestFeatureValidator</bannedImport>
+                    <bannedImport>org.apache.hadoop.ozone.om.request.validation.OMLayoutVersionValidator</bannedImport>
+                    <bannedImport>org.apache.hadoop.ozone.om.request.validation.OMClientVersionValidator</bannedImport>


Created #9530 to reduce the change that this PR needs to make in POM files. The other PR:

adds the new annotations, since they were already introduced in HDDS-11981

more importantly, removes RequestFeatureValidator from modules where it's not even accessible

So this PR will only need to update POMs for the removal of RequestFeatureValidator, and only in fewer modules.

merged with master branch

Change-Id: Ie6717a5c5e5e6821c2ec8d7abc7a5d4a20d6767f # Conflicts: # hadoop-hdds/client/pom.xml # hadoop-hdds/common/pom.xml # hadoop-hdds/container-service/pom.xml # hadoop-hdds/framework/pom.xml # hadoop-hdds/server-scm/pom.xml # hadoop-ozone/cli-admin/pom.xml # hadoop-ozone/cli-shell/pom.xml # hadoop-ozone/common/pom.xml # hadoop-ozone/csi/pom.xml # hadoop-ozone/freon/pom.xml # hadoop-ozone/insight/pom.xml # hadoop-ozone/recon/pom.xml # hadoop-ozone/tools/pom.xml # pom.xml

Change-Id: Ia2f6792669628c67088b8b5a273fb271476e8848

swamirishi · 2025-12-29T12:50:44Z

@adoroszlai I have addressed all your review comments. Can you take a look at the change now.

adoroszlai · 2026-01-07T19:17:06Z

...-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/RequestValidations.java

-    OMResponse validatedResponse = response;
+    List<Method> validations = registry.validationsFor(request.getCmdType(), POST_PROCESS, this.getVersions(request));
+
+    OMResponse validatedResponse = response.toBuilder().build();


Why is response being copied here?

just wanted to ensure the validators don't change the original object passed so that the caller can get both the objects. I can remove it if you want me to. It is inconsequential

Yes, please remove, it is unnecessary. OMResponse (as a protobuf message) is immutable, so validators cannot change it.

adoroszlai · 2026-01-10T09:27:46Z

@errose28 @fapifta please take a look

Copilot

Pull request overview

This pull request refactors the request validation framework in Ozone Manager to support version-based validations instead of condition-based validations. The changes replace the ValidationCondition enum and RequestFeatureValidator annotation with a more flexible version-based system using OMClientVersionValidator and OMLayoutVersionValidator annotations.

Changes:

Introduced version-based validator registration using @RegisterValidator meta-annotation
Replaced condition-based validation with version comparison logic in ValidatorRegistry
Updated all validator usages across S3, key, file, and bucket request handlers
Added support for both client version and layout version validators
Implemented IndexedItems data structure for efficient version-range lookups

Reviewed changes

Copilot reviewed 30 out of 30 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
ValidatorRegistry.java	Core refactoring to support version-based validator lookups with IndexedItems for efficient range queries
RequestValidations.java	Updated to extract versions from requests and lookup validators based on version ranges
ValidationCondition.java	Removed - replaced by version-based approach
RequestFeatureValidator.java	Removed - replaced by OMClientVersionValidator and OMLayoutVersionValidator
VersionExtractor.java	New enum to extract client and layout versions from OM requests
package-info.java	Updated documentation to reflect new version-based validation approach
Versioned.java	Added versionComparator() utility method
OzoneManagerRequestHandler.java	Updated validators to use OMClientVersionValidator
S3 multipart request handlers	Updated validators to use new annotations with explicit version parameters
Key/file/bucket request handlers	Updated validators to use new annotations with explicit version parameters
Test files	Updated tests to work with version-based validation logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-13T23:23:00Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

+   * 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


Typo in "afforementioned". It should be "aforementioned".

Suggested change

* Based on the afforementioned parameters a complete map is built which stores the validators in a sorted order of

* Based on the aforementioned parameters a complete map is built which stores the validators in a sorted order of

Copilot · 2026-01-13T23:23:01Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

-    return new EnumMap<>(RequestProcessingPhase.class);
-  }
+  /**
+   * Class responsible for maintaining indexs of items. Here each item should have an index corresponding to it.


Typo in the word "indexs". It should be "indexes" or "indices".

Suggested change

* Class responsible for maintaining indexs of items. Here each item should have an index corresponding to it.

* Class responsible for maintaining indexes of items. Here each item should have an index corresponding to it.

Copilot · 2026-01-13T23:23:01Z

...apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java

+      applyBefore = OMLayoutFeature.QUOTA,
+      processingPhase = PRE_PROCESS,
+      requestType = CreateVolume)
+  public static OMRequest multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(


Typo in the method name "multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The word "CLient" should be "Client" (lowercase 'l').

Copilot · 2026-01-13T23:23:01Z

...nager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java

    assertEquals(1, oldClientValidators.size());
-    String expectedMethodName = "multiPurposePostProcessCreateVolumeValidator";
+    assertEquals(1, combinedValidators.size());
+    String expectedMethodName = "multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator";


Typo in the expected method name "multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The word "CLient" should be "Client" (lowercase 'l').

Copilot · 2026-01-13T23:23:01Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/ValidatorRegistry.java

+     * @param item
+     * @param idx


Missing Javadoc for parameters. The add method should document what the item and idx parameters represent.

Suggested change

* @param item

* @param idx

* @param item the item to add to this collection

* @param idx the index associated with the item; indices must be provided in increasing order

Copilot · 2026-01-13T23:23:02Z

...apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java

+  public static OMRequest multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
      OMRequest req, ValidationContext ctx) {
-    fireValidationEvent("multiPurposePreProcessCreateVolumeValidator");
+    fireValidationEvent("multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator");


Typo in the method name "multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The word "CLient" should be "Client" (lowercase 'l').

Copilot · 2026-01-13T23:23:03Z

...apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java

-      requestType = CreateKey)
-  public static OMResponse oldClientPostProcessCreateKeyValidator2(
+      requestType = CreateVolume)
+  public static OMResponse multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(


Typo in the method name "multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The word "CLient" should be "Client" (lowercase 'l').

Copilot · 2026-01-13T23:23:03Z

...apache/hadoop/ozone/om/request/validation/testvalidatorset1/GeneralValidatorsForTesting.java

+  public static OMResponse multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator(
      OMRequest req, OMResponse resp, ValidationContext ctx) {
-    fireValidationEvent("oldClientPostProcessCreateKeyValidator2");
+    fireValidationEvent("multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator");


Typo in the method name "multiPurposePostProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The word "CLient" should be "Client" (lowercase 'l').

Copilot · 2026-01-13T23:23:03Z

...nager/src/test/java/org/apache/hadoop/ozone/om/request/validation/TestValidatorRegistry.java

    assertEquals(1, preFinalizeValidators.size());
    assertEquals(1, newClientValidators.size());
-    String expectedMethodName = "multiPurposePreProcessCreateVolumeValidator";
+    String expectedMethodName = "multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator";


Typo in the expected method name "multiPurposePreProcessCreateVolumeBucketLayoutCLientQuotaLayoutValidator". The word "CLient" should be "Client" (lowercase 'l').

Copilot · 2026-01-13T23:23:03Z

.../ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/validation/package-info.java

+ *
 * 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.


Typo in "reql". It should be "real".

Suggested change

* any complex validation has to happen in the reql request handling.

* any complex validation has to happen in the real request handling.

github-actions · 2026-02-04T00:06:36Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

github-actions · 2026-02-26T00:11:00Z

This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.

adoroszlai · 2026-02-26T08:16:04Z

@errose28 @fapifta can we wrap this up?

HDDS-12983. Validator Registry Changes for Supporting Version based v…

2fb399c

…alidations Change-Id: I939888c989eaec0a62884aa564e2f24a594122ef

swamirishi requested review from adoroszlai and fapifta May 6, 2025 20:44

swamirishi added 2 commits May 6, 2025 16:51

HDDS-12983. Revert module move change

16dd2d2

Change-Id: Icec10ce7bb452753db0936212b0757c01279b533

HDDS-12983. Fix Validator registry

de26a00

Change-Id: I9a010c86d0128da8140fa33b46eaeef9da0dc283

adoroszlai reviewed May 14, 2025

View reviewed changes

swamirishi added 3 commits May 20, 2025 08:06

HDDS-12983. Change key to validator map

bf98388

Change-Id: Ibf524d42e3d8535377105a07f8ca3213c8c62199

Merge remote-tracking branch 'apache/master' into HEAD

36cd0d5

Change-Id: I67634e5517239bdec3965ed8819f9af6ecf577dd

HDDS-12983. Address review comments

d4b9b33

Change-Id: Idf26bbc7e83877a71f46bafdf0216d70733a92ee

swamirishi requested a review from sumitagrawl May 20, 2025 15:23

swamirishi added 3 commits May 20, 2025 11:41

HDDS-12983. Fix checkstyle issue

64e0f58

Change-Id: Idecefb392dbf5c01227d7c3702db1781b9edb073

Merge remote-tracking branch 'apache/master' into HEAD

4e18baa

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

Resolve merge conflicts

3ffac6c

Change-Id: I132006b2b5a638e7f19d9ec1beb33b332d105e98

github-actions bot added the stale label Nov 12, 2025

github-actions bot closed this Nov 19, 2025

swamirishi reopened this Dec 17, 2025

HDDS-12983. Fix checkstyle

3743515

Change-Id: I461d46461905d041a613b101182d24e18a143d5c

github-actions bot removed the stale label Dec 18, 2025

adoroszlai reviewed Dec 19, 2025

View reviewed changes

swamirishi added 2 commits December 29, 2025 07:38

HDDS-12983. Address review comments

a3a696c

Change-Id: Ia2f6792669628c67088b8b5a273fb271476e8848

swamirishi requested a review from adoroszlai December 29, 2025 14:15

adoroszlai reviewed Jan 7, 2026

View reviewed changes

remove unecessary copy

33deff8

adoroszlai requested a review from errose28 January 10, 2026 09:27

smengcl requested review from Copilot and smengcl January 13, 2026 23:14

Copilot started reviewing on behalf of smengcl January 13, 2026 23:15 View session

Copilot AI reviewed Jan 13, 2026

View reviewed changes

github-actions bot added the stale label Feb 4, 2026

swamirishi removed the stale label Feb 4, 2026

github-actions bot added the stale label Feb 26, 2026

github-actions bot removed the stale label Feb 27, 2026

swamirishi marked this pull request as ready for review March 1, 2026 16:26

	applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT
	applyBefore = ClientVersion.BUCKET_LAYOUT_SUPPORT

		List<Method> validations = registry.validationsFor(request.getCmdType(), PRE_PROCESS,
		this.getVersions(request));

	* Based on the afforementioned parameters a complete map is built which stores the validators in a sorted order of
	* Based on the aforementioned parameters a complete map is built which stores the validators in a sorted order of

	* Class responsible for maintaining indexs of items. Here each item should have an index corresponding to it.
	* Class responsible for maintaining indexes of items. Here each item should have an index corresponding to it.

	* any complex validation has to happen in the reql request handling.
	* any complex validation has to happen in the real request handling.

Conversation

swamirishi commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Nov 12, 2025

Uh oh!

github-actions bot commented Nov 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swamirishi commented Dec 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adoroszlai commented Jan 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

swamirishi commented May 6, 2025 •

edited

Loading

adoroszlai Jan 8, 2026 •

edited

Loading