nastra commented on a change in pull request #4266:
URL: https://github.com/apache/iceberg/pull/4266#discussion_r819572485
##########
File path:
core/src/main/java/org/apache/iceberg/rest/requests/UpdateNamespacePropertiesRequest.java
##########
@@ -19,109 +19,35 @@
package org.apache.iceberg.rest.requests;
-import java.util.Collection;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import java.util.List;
import java.util.Map;
-import java.util.Objects;
import org.apache.iceberg.relocated.com.google.common.base.MoreObjects;
-import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
-import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
-import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
-import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
-import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.immutables.value.Value;
/**
* A REST request to set and/or remove properties on a namespace.
+ *
+ * Note that Immutable classes by definition don't have a default no-arg
constructor (which is required for Jackson),
+ * therefore the @{@link JsonSerialize}/@{@link JsonDeserialize} annotations
on the class will generate what's
+ * necessary for Jackson-binding to properly work with this class.
*/
-public class UpdateNamespacePropertiesRequest {
-
- private List<String> removals;
- private Map<String, String> updates;
-
- public UpdateNamespacePropertiesRequest() {
- // Required for Jackson deserialization.
- }
[email protected]
+@JsonSerialize(as = ImmutableUpdateNamespacePropertiesRequest.class)
+@JsonDeserialize(as = ImmutableUpdateNamespacePropertiesRequest.class)
+public abstract class UpdateNamespacePropertiesRequest {
- private UpdateNamespacePropertiesRequest(List<String> removals, Map<String,
String> updates) {
- this.removals = removals;
- this.updates = updates;
- validate();
- }
+ public abstract List<String> removals();
- UpdateNamespacePropertiesRequest validate() {
- return this;
- }
-
- public List<String> removals() {
- return removals == null ? ImmutableList.of() : removals;
- }
-
- public Map<String, String> updates() {
- return updates == null ? ImmutableMap.of() : updates;
- }
+ public abstract Map<String, String> updates();
@Override
public String toString() {
return MoreObjects.toStringHelper(this)
- .add("removals", removals)
- .add("updates", updates)
+ .add("removals", removals())
+ .add("updates", updates())
.toString();
}
-
- public static Builder builder() {
- return new Builder();
- }
-
- public static class Builder {
- private final ImmutableSet.Builder<String> removalsBuilder =
ImmutableSet.builder();
- private final ImmutableMap.Builder<String, String> updatesBuilder =
ImmutableMap.builder();
-
- private Builder() {
- }
-
- public Builder remove(String removal) {
- Preconditions.checkNotNull(removal,
- "Invalid property to remove: null");
- removalsBuilder.add(removal);
- return this;
- }
-
- public Builder removeAll(Collection<String> removals) {
- Preconditions.checkNotNull(removals,
- "Invalid list of properties to remove: null");
- Preconditions.checkArgument(!removals.contains(null),
- "Invalid property to remove: null");
- removalsBuilder.addAll(removals);
- return this;
- }
-
- public Builder update(String key, String value) {
- Preconditions.checkNotNull(key,
- "Invalid property to update: null");
- Preconditions.checkNotNull(value,
- "Invalid value to update for key [%s]: null. Use remove instead",
key);
Review comment:
same here in terms of validation: These are all implicitly checked. The
only thing that could be improved in Immutables when this fails would be a
similar error message like this one here, where the actual `key` with the null
value is shown.
I'll open a PR on the Immutables side to address this
--
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]
