This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 44d55effba [#6523] fix(core): Filter null prop when init properties
(#6524)
44d55effba is described below
commit 44d55effba1782e73985bf1b4261c8017e508848
Author: Tianhang <[email protected]>
AuthorDate: Wed Mar 26 06:44:29 2025 +0800
[#6523] fix(core): Filter null prop when init properties (#6524)
### What changes were proposed in this pull request?
Filters out any entries with null keys/values before collection, to
prevent potential NullPointerExceptions when accessing entries
### Why are the changes needed?
Due to the cancellation of the operation to reload the topic to fill in
its other properties after the alter operation in
https://github.com/apache/gravitino/issues/3729, there is a risk that
the value of the downstream property may return null, which in turn
could trigger a NullPointerException in the properties() method of the
Topic class.
Fix: #6523
### Does this PR introduce _any_ user-facing change?
no
### How was this patch tested?
UT added
---------
Co-authored-by: teo <[email protected]>
---
.../gravitino/catalog/EntityCombinedFileset.java | 1 +
.../gravitino/catalog/EntityCombinedModel.java | 1 +
.../catalog/EntityCombinedModelVersion.java | 1 +
.../gravitino/catalog/EntityCombinedSchema.java | 1 +
.../gravitino/catalog/EntityCombinedTable.java | 1 +
.../gravitino/catalog/EntityCombinedTopic.java | 1 +
.../gravitino/meta/TestEntityCombinedObject.java | 183 +++++++++++++++++++++
7 files changed, 189 insertions(+)
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedFileset.java
b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedFileset.java
index c7b847fc9c..9b7fc8bd35 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedFileset.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedFileset.java
@@ -77,6 +77,7 @@ public final class EntityCombinedFileset implements Fileset {
public Map<String, String> properties() {
return fileset.properties().entrySet().stream()
.filter(p -> !hiddenProperties.contains(p.getKey()))
+ .filter(entry -> entry.getKey() != null && entry.getValue() != null)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModel.java
b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModel.java
index 4aeefa0be5..740a5dbd5e 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModel.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModel.java
@@ -69,6 +69,7 @@ public final class EntityCombinedModel implements Model {
? null
: model.properties().entrySet().stream()
.filter(e -> !hiddenProperties.contains(e.getKey()))
+ .filter(entry -> entry.getKey() != null && entry.getValue() !=
null)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModelVersion.java
b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModelVersion.java
index b41e2889de..748a5c1238 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModelVersion.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedModelVersion.java
@@ -71,6 +71,7 @@ public final class EntityCombinedModelVersion implements
ModelVersion {
? null
: modelVersion.properties().entrySet().stream()
.filter(e -> !hiddenProperties.contains(e.getKey()))
+ .filter(entry -> entry.getKey() != null && entry.getValue() !=
null)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedSchema.java
b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedSchema.java
index ce3d0a3be7..a9cd7cfe8a 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedSchema.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedSchema.java
@@ -85,6 +85,7 @@ public final class EntityCombinedSchema implements Schema {
public Map<String, String> properties() {
return schema.properties().entrySet().stream()
.filter(e -> !hiddenProperties.contains(e.getKey()))
+ .filter(entry -> entry.getKey() != null && entry.getValue() != null)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTable.java
b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTable.java
index 70cbd0ace4..a70699ac7f 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTable.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTable.java
@@ -96,6 +96,7 @@ public final class EntityCombinedTable implements Table {
public Map<String, String> properties() {
return table.properties().entrySet().stream()
.filter(p -> !hiddenProperties.contains(p.getKey()))
+ .filter(entry -> entry.getKey() != null && entry.getValue() != null)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTopic.java
b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTopic.java
index 972df622b3..6193fadacf 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTopic.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/EntityCombinedTopic.java
@@ -84,6 +84,7 @@ public class EntityCombinedTopic implements Topic {
public Map<String, String> properties() {
return topic.properties().entrySet().stream()
.filter(p -> !hiddenProperties.contains(p.getKey()))
+ .filter(entry -> entry.getKey() != null && entry.getValue() != null)
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
}
diff --git
a/core/src/test/java/org/apache/gravitino/meta/TestEntityCombinedObject.java
b/core/src/test/java/org/apache/gravitino/meta/TestEntityCombinedObject.java
new file mode 100644
index 0000000000..a24b810bf7
--- /dev/null
+++ b/core/src/test/java/org/apache/gravitino/meta/TestEntityCombinedObject.java
@@ -0,0 +1,183 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.meta;
+
+import static org.mockito.Mockito.mock;
+
+import com.google.common.collect.ImmutableSet;
+import java.time.Instant;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.apache.gravitino.Schema;
+import org.apache.gravitino.catalog.EntityCombinedFileset;
+import org.apache.gravitino.catalog.EntityCombinedModel;
+import org.apache.gravitino.catalog.EntityCombinedSchema;
+import org.apache.gravitino.catalog.EntityCombinedTable;
+import org.apache.gravitino.catalog.EntityCombinedTopic;
+import org.apache.gravitino.file.Fileset;
+import org.apache.gravitino.messaging.Topic;
+import org.apache.gravitino.model.Model;
+import org.apache.gravitino.rel.Table;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+public class TestEntityCombinedObject {
+ private final AuditInfo auditInfo =
+ AuditInfo.builder()
+ .withCreator("creator")
+ .withCreateTime(Instant.parse("2025-01-01T00:00:00Z"))
+ .withLastModifier("modifier")
+ .withLastModifiedTime(Instant.parse("2025-01-02T00:00:00Z"))
+ .build();
+
+ // Properties test data;
+ private final Map<String, String> entityProperties =
+ new HashMap<String, String>() {
+ {
+ put("k1", "v1");
+ put("k2", "v2");
+ put("k3", "k3");
+ put("k5", null);
+ put(null, null);
+ }
+ };
+ private final Set<String> hiddenProperties = ImmutableSet.of("k3", "k4");
+ private final Schema originSchema = mockSchema();
+ private final Topic originTopic = mockTopic();
+ private final Table originTable = mockTable();
+ private final Fileset originFileset = mockFileset();
+ private final Model originModel = mockModel();
+
+ @Test
+ public void testSchema() {
+ EntityCombinedSchema entityCombinedSchema =
+
EntityCombinedSchema.of(originSchema).withHiddenProperties(hiddenProperties);
+ Assertions.assertEquals(originSchema.name(), entityCombinedSchema.name());
+ Assertions.assertEquals(originSchema.comment(),
entityCombinedSchema.comment());
+ Map<String, String> filterProp = new HashMap<>(originSchema.properties());
+ filterProp.remove("k3");
+ filterProp.remove(null);
+ filterProp.remove("k5");
+ Assertions.assertEquals(filterProp, entityCombinedSchema.properties());
+ Assertions.assertEquals(originSchema.auditInfo(),
entityCombinedSchema.auditInfo());
+ }
+
+ @Test
+ public void testTopic() {
+ EntityCombinedTopic entityCombinedTopic =
+
EntityCombinedTopic.of(originTopic).withHiddenProperties(hiddenProperties);
+ Assertions.assertEquals(originTopic.name(), entityCombinedTopic.name());
+ Assertions.assertEquals(originTopic.comment(),
entityCombinedTopic.comment());
+ Map<String, String> filterProp = new HashMap<>(originTopic.properties());
+ filterProp.remove("k3");
+ filterProp.remove(null);
+ filterProp.remove("k5");
+ Assertions.assertEquals(filterProp, entityCombinedTopic.properties());
+ Assertions.assertEquals(originTopic.auditInfo(),
entityCombinedTopic.auditInfo());
+ }
+
+ @Test
+ public void testTable() {
+ EntityCombinedTable entityCombinedTable =
+
EntityCombinedTable.of(originTable).withHiddenProperties(hiddenProperties);
+ Assertions.assertEquals(originTable.name(), entityCombinedTable.name());
+ Assertions.assertEquals(originTable.comment(),
entityCombinedTable.comment());
+ Map<String, String> filterProp = new HashMap<>(originTopic.properties());
+ filterProp.remove("k3");
+ filterProp.remove(null);
+ filterProp.remove("k5");
+ Assertions.assertEquals(filterProp, entityCombinedTable.properties());
+ Assertions.assertEquals(originTable.auditInfo(),
entityCombinedTable.auditInfo());
+ }
+
+ @Test
+ public void testFileset() {
+ EntityCombinedFileset entityCombinedFileset =
+
EntityCombinedFileset.of(originFileset).withHiddenProperties(hiddenProperties);
+ Assertions.assertEquals(originFileset.name(),
entityCombinedFileset.name());
+ Assertions.assertEquals(originFileset.comment(),
entityCombinedFileset.comment());
+ Map<String, String> filterProp = new HashMap<>(originTopic.properties());
+ filterProp.remove("k3");
+ filterProp.remove(null);
+ filterProp.remove("k5");
+ Assertions.assertEquals(filterProp, entityCombinedFileset.properties());
+ Assertions.assertEquals(originFileset.auditInfo(),
entityCombinedFileset.auditInfo());
+ }
+
+ @Test
+ public void testModel() {
+ EntityCombinedModel entityCombinedModel =
+
EntityCombinedModel.of(originModel).withHiddenProperties(hiddenProperties);
+ Assertions.assertEquals(originModel.name(), entityCombinedModel.name());
+ Assertions.assertEquals(originModel.comment(),
entityCombinedModel.comment());
+ Map<String, String> filterProp = new HashMap<>(originModel.properties());
+ filterProp.remove("k3");
+ filterProp.remove(null);
+ filterProp.remove("k5");
+ Assertions.assertEquals(filterProp, entityCombinedModel.properties());
+ Assertions.assertEquals(originModel.auditInfo(),
entityCombinedModel.auditInfo());
+ }
+
+ private Schema mockSchema() {
+ Schema mockSchema = mock(Schema.class);
+ Mockito.when(mockSchema.name()).thenReturn("testSchema");
+ Mockito.when(mockSchema.comment()).thenReturn("test schema comment");
+ Mockito.when(mockSchema.auditInfo()).thenReturn(auditInfo);
+ Mockito.when(mockSchema.properties()).thenReturn(entityProperties);
+ return mockSchema;
+ }
+
+ private Topic mockTopic() {
+ Topic mockTopic = mock(Topic.class);
+ Mockito.when(mockTopic.name()).thenReturn("testTopic");
+ Mockito.when(mockTopic.comment()).thenReturn("test topic comment");
+ Mockito.when(mockTopic.auditInfo()).thenReturn(auditInfo);
+ Mockito.when(mockTopic.properties()).thenReturn(entityProperties);
+ return mockTopic;
+ }
+
+ private Fileset mockFileset() {
+ Fileset mockFileset = mock(Fileset.class);
+ Mockito.when(mockFileset.name()).thenReturn("testFileset");
+ Mockito.when(mockFileset.comment()).thenReturn("test fileset comment");
+ Mockito.when(mockFileset.auditInfo()).thenReturn(auditInfo);
+ Mockito.when(mockFileset.properties()).thenReturn(entityProperties);
+ return mockFileset;
+ }
+
+ private Table mockTable() {
+ Table mockTable = mock(Table.class);
+ Mockito.when(mockTable.name()).thenReturn("testTable");
+ Mockito.when(mockTable.comment()).thenReturn("test table comment");
+ Mockito.when(mockTable.auditInfo()).thenReturn(auditInfo);
+ Mockito.when(mockTable.properties()).thenReturn(entityProperties);
+ return mockTable;
+ }
+
+ private Model mockModel() {
+ Model mockModel = mock(Model.class);
+ Mockito.when(mockModel.name()).thenReturn("testModel");
+ Mockito.when(mockModel.comment()).thenReturn("test model comment");
+ Mockito.when(mockModel.auditInfo()).thenReturn(auditInfo);
+ Mockito.when(mockModel.properties()).thenReturn(entityProperties);
+ return mockModel;
+ }
+}
