rdblue commented on code in PR #8147:
URL: https://github.com/apache/iceberg/pull/8147#discussion_r1296165510


##########
core/src/main/java/org/apache/iceberg/view/ViewMetadata.java:
##########
@@ -91,41 +109,215 @@ default ViewMetadata checkAndNormalize() {
         "Unsupported format version: %s",
         formatVersion());
 
-    Preconditions.checkArgument(versions().size() > 0, "Invalid view versions: 
empty");
-    Preconditions.checkArgument(history().size() > 0, "Invalid view history: 
empty");
-    Preconditions.checkArgument(schemas().size() > 0, "Invalid schemas: 
empty");
+    return this;
+  }
+
+  static Builder builder() {
+    return new Builder();
+  }
 
-    Preconditions.checkArgument(
-        versionsById().containsKey(currentVersionId()),
-        "Cannot find current version %s in view versions: %s",
-        currentVersionId(),
-        versionsById().keySet());
+  static Builder buildFrom(ViewMetadata base) {
+    return new Builder(base);
+  }
 
-    Preconditions.checkArgument(
-        schemasById().containsKey(currentSchemaId()),
-        "Cannot find current schema with id %s in schemas: %s",
-        currentSchemaId(),
-        schemasById().keySet());
+  class Builder {
+    private int formatVersion = DEFAULT_VIEW_FORMAT_VERSION;
+    private String location;
+    private List<Schema> schemas = Lists.newArrayList();
+    private int currentVersionId;
+    private List<ViewVersion> versions = Lists.newArrayList();
+    private List<ViewHistoryEntry> history = Lists.newArrayList();
+    private Map<String, String> properties = Maps.newHashMap();
+    private List<MetadataUpdate> changes = Lists.newArrayList();
 
-    int versionHistorySizeToKeep =
-        PropertyUtil.propertyAsInt(
-            properties(),
-            ViewProperties.VERSION_HISTORY_SIZE,
-            ViewProperties.VERSION_HISTORY_SIZE_DEFAULT);
-
-    if (versionHistorySizeToKeep <= 0) {
-      LOG.warn(
-          "{} must be positive but was {}",
-          ViewProperties.VERSION_HISTORY_SIZE,
-          versionHistorySizeToKeep);
-    } else if (versions().size() > versionHistorySizeToKeep) {
-      List<ViewVersion> versions =
-          versions().subList(versions().size() - versionHistorySizeToKeep, 
versions().size());
-      List<ViewHistoryEntry> history =
-          history().subList(history().size() - versionHistorySizeToKeep, 
history().size());
-      return 
ImmutableViewMetadata.builder().from(this).versions(versions).history(history).build();
+    private Builder() {}
+
+    private Builder(ViewMetadata base) {
+      this.formatVersion = base.formatVersion();
+      this.location = base.location();
+      this.schemas = Lists.newArrayList(base.schemas());
+      this.currentVersionId = base.currentVersionId();
+      this.versions = Lists.newArrayList(base.versions());
+      this.history = Lists.newArrayList(base.history());
+      this.properties = Maps.newHashMap(base.properties());
+      this.changes = Lists.newArrayList(base.changes());
     }
 
-    return this;
+    public Builder upgradeFormatVersion(int newFormatVersion) {
+      Preconditions.checkArgument(
+          newFormatVersion >= this.formatVersion,
+          "Cannot downgrade v%s view to v%s",
+          formatVersion,
+          newFormatVersion);
+
+      if (this.formatVersion == newFormatVersion) {
+        return this;
+      }
+
+      this.formatVersion = newFormatVersion;
+      this.changes.add(new 
MetadataUpdate.UpgradeFormatVersion(newFormatVersion));
+      return this;
+    }
+
+    public Builder setLocation(String newLocation) {
+      Preconditions.checkArgument(null != newLocation, "Invalid location: 
null");
+      if (null != this.location && this.location.equals(newLocation)) {
+        return this;
+      }
+
+      this.location = newLocation;
+      this.changes.add(new MetadataUpdate.SetLocation(newLocation));
+      return this;
+    }
+
+    public Builder setCurrentVersionId(int newVersionId) {
+      if (this.currentVersionId == newVersionId) {
+        return this;
+      }
+
+      this.currentVersionId = newVersionId;
+      this.changes.add(new MetadataUpdate.SetCurrentViewVersion(newVersionId));
+      return this;
+    }
+
+    public Builder addSchema(Schema schema) {
+      this.schemas.add(schema);
+      this.changes.add(new MetadataUpdate.AddSchema(schema, 
schema.highestFieldId()));
+      return this;
+    }
+
+    public Builder setSchemas(Iterable<Schema> schemasToAdd) {
+      int highestFieldId =
+          Lists.newArrayList(schemasToAdd).stream()
+              .map(Schema::highestFieldId)
+              .max(Integer::compareTo)
+              .orElse(0);
+      for (Schema schema : schemasToAdd) {
+        this.schemas.add(schema);
+        this.changes.add(new MetadataUpdate.AddSchema(schema, highestFieldId));
+      }
+
+      return this;
+    }
+
+    public Builder addVersion(ViewVersion version) {
+      this.versions.add(version);
+      this.changes.add(new MetadataUpdate.AddViewVersion(version));
+      this.history.add(
+          ImmutableViewHistoryEntry.builder()
+              .timestampMillis(version.timestampMillis())
+              .versionId(version.versionId())
+              .build());
+      return this;
+    }
+
+    public Builder setProperties(Map<String, String> updated) {
+      if (updated.isEmpty()) {
+        return this;
+      }
+
+      this.properties.putAll(updated);
+      this.changes.add(new MetadataUpdate.SetProperties(updated));
+      return this;
+    }
+
+    public Builder removeProperties(Set<String> propertiesToRemove) {
+      if (propertiesToRemove.isEmpty()) {
+        return this;
+      }
+
+      propertiesToRemove.forEach(this.properties::remove);
+      this.changes.add(new 
MetadataUpdate.RemoveProperties(propertiesToRemove));
+      return this;
+    }
+
+    private static Map<Integer, ViewVersion> indexVersions(List<ViewVersion> 
versionsToIndex) {
+      ImmutableMap.Builder<Integer, ViewVersion> builder = 
ImmutableMap.builder();
+      for (ViewVersion version : versionsToIndex) {
+        builder.put(version.versionId(), version);
+      }
+
+      return builder.build();
+    }
+
+    private static Map<Integer, Schema> indexSchemas(List<Schema> 
schemasToIndex) {
+      ImmutableMap.Builder<Integer, Schema> builder = ImmutableMap.builder();
+      for (Schema schema : schemasToIndex) {
+        builder.put(schema.schemaId(), schema);
+      }
+
+      return builder.build();
+    }
+
+    public ViewMetadata build() {
+      Preconditions.checkArgument(null != location, "Invalid location: null");
+      Preconditions.checkArgument(versions.size() > 0, "Invalid view versions: 
empty");
+      Preconditions.checkArgument(history.size() > 0, "Invalid view history: 
empty");
+      Preconditions.checkArgument(schemas.size() > 0, "Invalid schemas: 
empty");
+
+      Map<Integer, ViewVersion> versionsById = indexVersions(versions);
+      Preconditions.checkArgument(
+          versionsById.containsKey(currentVersionId),
+          "Cannot find current version %s in view versions: %s",
+          currentVersionId,
+          versionsById.keySet());
+
+      int currentSchemaId = versionsById.get(currentVersionId).schemaId();
+      Map<Integer, Schema> schemasById = indexSchemas(schemas);
+      Preconditions.checkArgument(
+          schemasById.containsKey(currentSchemaId),
+          "Cannot find current schema with id %s in schemas: %s",
+          currentSchemaId,
+          schemasById.keySet());
+
+      int versionHistorySizeToKeep =
+          PropertyUtil.propertyAsInt(
+              properties,
+              ViewProperties.VERSION_HISTORY_SIZE,
+              ViewProperties.VERSION_HISTORY_SIZE_DEFAULT);
+
+      if (versionHistorySizeToKeep <= 0) {

Review Comment:
   Now that this is in the builder, we can throw an exception if it is invalid. 
Maybe convert this to a precondition?



-- 
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]

Reply via email to