HDDS-11132. Update the OLDER_CLIENT_REQUEST validations so that validations don't fail

[swamirishi]

What changes were proposed in this pull request?

With the introduction of a new client version in https://issues.apache.org/jira/browse/HDDS-10983, the latest version is 4.
The OLDER_CLIENT_REQUEST validation checked against the latest versions for access to newer non-legacy buckets but since the addition of a newer version all client versions prior to 4 becomes an older client. This blocks access to OBS/FSO buckets and EC key lookups for older clients version 2 & 3.
This is related to the original patch submitted by @sadanand48 #6919

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11132

How was this patch tested?

Unit tests need to be added.

[adoroszlai]

The same "return early if client is new enough" check can be added in all methods (changing only the enum value being compared to, and the return type (request/response)):

    ClientVersion clientVersion = ClientVersion.fromProtoValue(req.getVersion());
    if (ClientVersion.BUCKET_LAYOUT_SUPPORT.compareTo(clientVersion) <= 0) {
      return req;
    }

That would:

• simplify this change,
• help avoid checkstyle problems,
• make the logic easier to understand for future readers of the code,
• prepare for the future refactoring discussed in HDDS-11132. Update the OLDER_CLIENT_REQUEST validations to check against bucket types. #6919.

Suggested change

	if (ClientVersion.fromProtoValue(req.getVersion())
	.compareTo(ClientVersion.BUCKET_LAYOUT_SUPPORT) < 0) {
	DeleteBucketRequest request = req.getDeleteBucketRequest();
	if (request.hasBucketName() && request.hasVolumeName()) {
	BucketLayout bucketLayout = ctx.getBucketLayout(
	request.getVolumeName(), request.getBucketName());
	bucketLayout.validateSupportedOperation();
	}
	}
	return req;
	ClientVersion clientVersion = ClientVersion.fromProtoValue(req.getVersion());
	if (ClientVersion.BUCKET_LAYOUT_SUPPORT.compareTo(clientVersion) <= 0) {
	return req;
	}
	DeleteBucketRequest request = req.getDeleteBucketRequest();
	if (request.hasBucketName() && request.hasVolumeName()) {
	BucketLayout bucketLayout = ctx.getBucketLayout(
	request.getVolumeName(), request.getBucketName());
	bucketLayout.validateSupportedOperation();
	}
	return req;

[swamirishi]

yeah sure will make this change add it in a separate if block

[fapifta]

I would even go further a bit...
What if we define a method in the ClientVersion enum, call it let's say isNewerThan(ClientVersion), and use that instead of the compareTo?
With that this block can be reduced to an if (ClientVersion.fromProtoValue(req.getVersion()).isNewerThan(ClientVersion.BUCKET_LAYOUT_SUPPORT))

An other option may be (though it means introducing a new method for every version) to add a static method named like isNewerThanBucketLayout(int version), do the proto to enum conversion in the method than the compareTo, and return a boolen, so that the usage would look like:
if (ClientVersion.isNewerThanBucketLayout(req.getVersion()))

[adoroszlai]

I'd suggest:

ClientVersion.BUCKET_LAYOUT_SUPPORT.isSupportedByClient(req.getVersion())

(which also highlights why "SUPPORT" shouldn't be part of the enum constant name...)

[errose28]

@swamirishi should we close this in favor of #6932 to avoid confusion?