Skip to content

HDDS-10518. Create base Builder in WithMetadata and WithObjectID #6378

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
szetszwo merged 6 commits into from
Mar 27, 2024
Merged

HDDS-10518. Create base Builder in WithMetadata and WithObjectID #6378

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
176f2fd
HDDS-10518. Create base Builder in WithMetadata and WithObjectID
adoroszlai Mar 13, 2024
d13984a
restore setMetadata() semantics in OzoneVolume/Bucket
adoroszlai Mar 15, 2024
21d7db7
fix infinite loop
adoroszlai Mar 15, 2024
50107d4
Merge remote-tracking branch 'origin/master' into HDDS-10518
adoroszlai Mar 23, 2024
4c2f5cd
Merge remote-tracking branch 'origin/master' into HDDS-10518
adoroszlai Mar 27, 2024
65e61d6
add missing `@Override`
adoroszlai Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ public class OzoneBucket extends WithMetadata {
private String owner;

protected OzoneBucket(Builder builder) {
setMetadata(builder.metadata);
super(builder);
this.proxy = builder.proxy;
this.volumeName = builder.volumeName;
this.name = builder.name; // bucket name
Expand Down Expand Up @@ -954,8 +954,7 @@ public static Builder newBuilder(ConfigurationSource conf,
/**
* Inner builder for OzoneBucket.
*/
public static class Builder {
private Map<String, String> metadata;
public static class Builder extends WithMetadata.Builder {
private ConfigurationSource conf;
private ClientProtocol proxy;
private String volumeName;
Expand Down Expand Up @@ -984,7 +983,7 @@ private Builder(ConfigurationSource conf, ClientProtocol proxy) {
}

public Builder setMetadata(Map<String, String> metadata) {
this.metadata = metadata;
addAllMetadata(metadata);

@ivandika3 ivandika3 Mar 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Should not affect current usage, but the semantic seems to be changed changed from "replacing" to "adding". Might need to clear the metadata before adding it or can change the method name to addAllMetadata (but OzoneBucket is client-facing class so might not be a good idea).

I'm OK if you decide not to change it at all.

@adoroszlai adoroszlai Mar 15, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Updated to clear any existing metadata for these two classes.

return this;
}

Expand Down
7 changes: 3 additions & 4 deletions hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public class OzoneVolume extends WithMetadata {
private long refCount;

protected OzoneVolume(Builder builder) {
setMetadata(builder.metadata);
super(builder);
this.proxy = builder.proxy;
this.name = builder.name;
this.admin = builder.admin;
Expand Down Expand Up @@ -409,8 +409,7 @@ public static Builder newBuilder(ConfigurationSource conf,
/**
* Inner builder for OzoneVolume.
*/
public static class Builder {
private Map<String, String> metadata;
public static class Builder extends WithMetadata.Builder {
private ConfigurationSource conf;
private ClientProtocol proxy;
private String name;
Expand Down Expand Up @@ -483,7 +482,7 @@ public Builder setRefCount(long refCount) {
}

public Builder setMetadata(Map<String, String> metadata) {
this.metadata = metadata;
addAllMetadata(metadata);

@ivandika3 ivandika3 Mar 15, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment to OzoneBucket.Builder#setMetadata.

return this;
}

Expand Down
8 changes: 3 additions & 5 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketArgs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/
package org.apache.hadoop.ozone.om.helpers;

import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;

Expand Down Expand Up @@ -67,7 +66,7 @@ public final class OmBucketArgs extends WithMetadata implements Auditable {
private final String ownerName;

private OmBucketArgs(Builder b) {
setMetadata(b.metadata);
super(b);
this.volumeName = b.volumeName;
this.bucketName = b.bucketName;
this.isVersionEnabled = b.isVersionEnabled;
Expand Down Expand Up @@ -214,12 +213,11 @@ public Map<String, String> toAuditMap() {
/**
* Builder for OmBucketArgs.
*/
public static class Builder {
public static class Builder extends WithMetadata.Builder {
private String volumeName;
private String bucketName;
private Boolean isVersionEnabled;
private StorageType storageType;
private final Map<String, String> metadata = new HashMap<>();
private boolean quotaInBytesSet = false;
private long quotaInBytes;
private boolean quotaInNamespaceSet = false;
Expand Down Expand Up @@ -259,7 +257,7 @@ public Builder setBucketEncryptionKey(BucketEncryptionKeyInfo info) {
}

public Builder addAllMetadata(Map<String, String> map) {
metadata.putAll(map);
super.addAllMetadata(map);
Comment on lines 260 to +261

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

return this;
}

Expand Down
52 changes: 18 additions & 34 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@


import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -110,9 +109,7 @@ public static Codec<OmBucketInfo> getCodec() {
private String owner;

private OmBucketInfo(Builder b) {
setMetadata(b.metadata);
setObjectID(b.objectID);
setUpdateID(b.updateID);
super(b);
this.volumeName = b.volumeName;
this.bucketName = b.bucketName;
this.acls = b.acls;
Expand Down Expand Up @@ -361,20 +358,17 @@ public OmBucketInfo copyObject() {
}

public Builder toBuilder() {
return new Builder()
return new Builder(this)
.setVolumeName(volumeName)
.setBucketName(bucketName)
.setStorageType(storageType)
.setIsVersionEnabled(isVersionEnabled)
.setCreationTime(creationTime)
.setModificationTime(modificationTime)
.setObjectID(getObjectID())
.setUpdateID(getUpdateID())
.setBucketEncryptionKey(bekInfo)
.setSourceVolume(sourceVolume)
.setSourceBucket(sourceBucket)
.setAcls(acls)
.addAllMetadata(getMetadata())
.setUsedBytes(usedBytes)
.setUsedNamespace(usedNamespace)
.setQuotaInBytes(quotaInBytes)
Expand All @@ -387,37 +381,30 @@ public Builder toBuilder() {
/**
* Builder for OmBucketInfo.
*/
public static class Builder {
public static class Builder extends WithObjectID.Builder {
private String volumeName;
private String bucketName;
private List<OzoneAcl> acls;
private Boolean isVersionEnabled;
private StorageType storageType;
private final List<OzoneAcl> acls = new ArrayList<>();
private boolean isVersionEnabled;
private StorageType storageType = StorageType.DISK;
private long creationTime;
private long modificationTime;
private long objectID;
private long updateID;
private Map<String, String> metadata;
private BucketEncryptionKeyInfo bekInfo;
private String sourceVolume;
private String sourceBucket;
private long usedBytes;
private long usedNamespace;
private long quotaInBytes;
private long quotaInNamespace;
private BucketLayout bucketLayout;
private long quotaInBytes = OzoneConsts.QUOTA_RESET;
private long quotaInNamespace = OzoneConsts.QUOTA_RESET;
private BucketLayout bucketLayout = BucketLayout.DEFAULT;
private String owner;
private DefaultReplicationConfig defaultReplicationConfig;

public Builder() {
//Default values
this.acls = new ArrayList<>();
this.isVersionEnabled = false;
this.storageType = StorageType.DISK;
this.metadata = new HashMap<>();
this.quotaInBytes = OzoneConsts.QUOTA_RESET;
this.quotaInNamespace = OzoneConsts.QUOTA_RESET;
this.bucketLayout = BucketLayout.DEFAULT;
}

private Builder(OmBucketInfo obj) {
super(obj);
}

public Builder setVolumeName(String volume) {
Expand Down Expand Up @@ -448,7 +435,7 @@ public Builder addAcl(OzoneAcl ozoneAcl) {
return this;
}

public Builder setIsVersionEnabled(Boolean versionFlag) {
public Builder setIsVersionEnabled(boolean versionFlag) {
this.isVersionEnabled = versionFlag;
return this;
}
Expand All @@ -469,24 +456,22 @@ public Builder setModificationTime(long modifiedOn) {
}

public Builder setObjectID(long obId) {
this.objectID = obId;
super.setObjectID(obId);
Comment on lines 459 to +460

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

return this;
}

public Builder setUpdateID(long id) {
this.updateID = id;
super.setUpdateID(id);
Comment on lines 465 to +466

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

return this;
}

public Builder addMetadata(String key, String value) {
metadata.put(key, value);
super.addMetadata(key, value);
Comment on lines 471 to +472

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

return this;
}

public Builder addAllMetadata(Map<String, String> additionalMetadata) {

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

if (additionalMetadata != null) {
metadata.putAll(additionalMetadata);
}
super.addAllMetadata(additionalMetadata);
return this;
}

Expand Down Expand Up @@ -556,7 +541,6 @@ public OmBucketInfo build() {
Preconditions.checkNotNull(volumeName);
Preconditions.checkNotNull(bucketName);
Preconditions.checkNotNull(acls);
Preconditions.checkNotNull(isVersionEnabled);
Preconditions.checkNotNull(storageType);
return new OmBucketInfo(this);
}
Expand Down
27 changes: 7 additions & 20 deletions hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmDirectoryInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.apache.hadoop.ozone.OzoneConsts;
import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.DirectoryInfo;

import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -55,12 +54,9 @@ public static Codec<OmDirectoryInfo> getCodec() {
private final List<OzoneAcl> acls;

public OmDirectoryInfo(Builder builder) {
super(builder);
this.name = builder.name;
this.acls = builder.acls;
setMetadata(builder.metadata);
setObjectID(builder.objectID);
setUpdateID(builder.updateID);
setParentObjectID(builder.parentObjectID);
this.creationTime = builder.creationTime;
this.modificationTime = builder.modificationTime;
}
Expand All @@ -77,38 +73,31 @@ public static OmDirectoryInfo.Builder newBuilder() {
/**
* Builder for Directory Info.
*/
public static class Builder {
private long parentObjectID; // pointer to parent directory

private long objectID;
private long updateID;

public static class Builder extends WithParentObjectId.Builder {
private String name;

private long creationTime;
private long modificationTime;

private final List<OzoneAcl> acls;
private final Map<String, String> metadata;

public Builder() {
//Default values
this.acls = new LinkedList<>();
this.metadata = new HashMap<>();
}

public Builder setParentObjectID(long parentObjectId) {

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

this.parentObjectID = parentObjectId;
super.setParentObjectID(parentObjectId);
return this;
}

public Builder setObjectID(long objectId) {

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

this.objectID = objectId;
super.setObjectID(objectId);
return this;
}

public Builder setUpdateID(long updateId) {

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

this.updateID = updateId;
super.setUpdateID(updateId);
return this;
}

Expand Down Expand Up @@ -142,14 +131,12 @@ public Builder addAcl(OzoneAcl ozoneAcl) {
}

public Builder addMetadata(String key, String value) {

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

metadata.put(key, value);
super.addMetadata(key, value);
return this;
}

public Builder addAllMetadata(Map<String, String> additionalMetadata) {

@szetszwo szetszwo Mar 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @Override.

if (additionalMetadata != null) {
metadata.putAll(additionalMetadata);
}
super.addAllMetadata(additionalMetadata);
return this;
}

Expand Down
Loading