swamirishi commented on code in PR #9356:
URL: https://github.com/apache/ozone/pull/9356#discussion_r2556127874
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartKeyInfo.java:
##########
@@ -293,18 +293,18 @@ public Builder setUpdateID(long id) {
return this;
}
- @Override
- public Builder withUpdateID(long newValue) {
- super.withUpdateID(newValue);
- return this;
- }
-
public Builder setParentID(long parentObjId) {
this.parentID = parentObjId;
return this;
}
public OmMultipartKeyInfo build() {
+ validate();
Review Comment:
Instead of each build method calling validate we can call super.build() and
that could call validate() method. The changes wouldn't be this big.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/WithObjectID.java:
##########
@@ -70,18 +71,24 @@ public String getObjectInfo() {
}
/** Builder for {@link WithObjectID}. */
- public static class Builder extends WithMetadata.Builder {
+ public abstract static class Builder extends WithMetadata.Builder {
+ private final long initialObjectID;
+ private final long initialUpdateID;
private long objectID;
private long updateID;
protected Builder() {
super();
+ initialObjectID = 0;
+ initialUpdateID = OzoneConsts.DEFAULT_OM_UPDATE_ID;
}
protected Builder(WithObjectID obj) {
super(obj);
Review Comment:
nit :
https://github.com/apache/ozone/blob/56e7988f0f587891c04d2c65e01ee40c49b4f3dd/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/WithMetadata.java#L62-L64
The parameter type here should be WithMetadata
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/WithObjectID.java:
##########
@@ -168,9 +118,20 @@ public long getUpdateID() {
return updateID;
}
- /** Hook method, customized in subclasses. */
- public String getObjectInfo() {
- return this.toString();
+ protected void validate() {
+ if (initialObjectID != objectID && initialObjectID != 0 && objectID !=
OBJECT_ID_RECLAIM_BLOCKS) {
+ throw new UnsupportedOperationException("Attempt to modify object ID "
+
+ "which is not zero. Current Object ID is " + initialObjectID);
+ }
+
+ if (updateID < initialUpdateID) {
+ throw new IllegalArgumentException(String.format(
+ "Trying to set updateID to %d which is not greater than the " +
+ "current value of %d for %s", updateID, initialUpdateID,
+ buildMaybeInvalid().getObjectInfo()));
+ }
}
+
+ protected abstract WithObjectID buildMaybeInvalid();
Review Comment:
```suggestion
protected abstract WithObjectID buildObject();
public WithObjectID build() {
validate();
return buildObject();
}
```
--
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]
