This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 0355835c4e [#13405] fix(core): null-guard and defensively copy uris in
the ModelVersionInfo map constructor (#13406)
0355835c4e is described below
commit 0355835c4e220a64645bc405d238d431f4e38eb1
Author: YangJie <[email protected]>
AuthorDate: Tue Sep 22 05:15:30 2026 -0400
[#13405] fix(core): null-guard and defensively copy uris in the
ModelVersionInfo map constructor (#13406)
### What changes were proposed in this pull request?
The `Map`-based `ModelVersionInfo` constructor now stores `uris` as
`Collections.emptyMap()` when the argument is null and
`ImmutableMap.copyOf(uris)` otherwise, matching the `uri`-based
constructor and the adjacent `properties` field.
### Why are the changes needed?
The constructor stored `uris` by reference, so a null `uris` made
`uris()` return null (while siblings return empty) and `uri()` throw
`NullPointerException`, and the aliased map exposed the event payload to
later mutation by the caller.
Fix: #13405
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added `TestModelVersionInfo.testNullUrisYieldEmptyMap` and
`testUrisAreDefensivelyCopied`, pinning the null handling and the
defensive copy. They fail against the pre-fix code.
---
.../listener/api/info/ModelVersionInfo.java | 2 +-
.../listener/api/info/TestModelVersionInfo.java | 56 ++++++++++++++++++++++
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/java/org/apache/gravitino/listener/api/info/ModelVersionInfo.java
b/core/src/main/java/org/apache/gravitino/listener/api/info/ModelVersionInfo.java
index c11b8414c4..493c31184e 100644
---
a/core/src/main/java/org/apache/gravitino/listener/api/info/ModelVersionInfo.java
+++
b/core/src/main/java/org/apache/gravitino/listener/api/info/ModelVersionInfo.java
@@ -108,7 +108,7 @@ public class ModelVersionInfo {
Map<String, String> properties,
String[] aliases,
Audit auditInfo) {
- this.uris = uris;
+ this.uris = uris == null ? Collections.emptyMap() :
ImmutableMap.copyOf(uris);
this.properties = properties == null ? ImmutableMap.of() :
ImmutableMap.copyOf(properties);
this.comment = Optional.ofNullable(comment);
this.audit = Optional.ofNullable(auditInfo);
diff --git
a/core/src/test/java/org/apache/gravitino/listener/api/info/TestModelVersionInfo.java
b/core/src/test/java/org/apache/gravitino/listener/api/info/TestModelVersionInfo.java
new file mode 100644
index 0000000000..d64b345a3b
--- /dev/null
+++
b/core/src/test/java/org/apache/gravitino/listener/api/info/TestModelVersionInfo.java
@@ -0,0 +1,56 @@
+/*
+ * 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.listener.api.info;
+
+import com.google.common.collect.ImmutableMap;
+import java.util.HashMap;
+import java.util.Map;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/** Tests for {@link ModelVersionInfo}. */
+public class TestModelVersionInfo {
+
+ @Test
+ public void testNullUrisYieldEmptyMap() {
+ ModelVersionInfo info =
+ new ModelVersionInfo((Map<String, String>) null, null, null, null,
null);
+
+ // Mirrors the uri-based constructor and the properties handling: absent
uris
+ // surface as an empty map, not null, and uri() must not throw.
+ Assertions.assertNotNull(info.uris());
+ Assertions.assertTrue(info.uris().isEmpty());
+ Assertions.assertNull(info.uri());
+ }
+
+ @Test
+ public void testUrisAreDefensivelyCopied() {
+ Map<String, String> uris = new HashMap<>();
+ uris.put("unknown", "gs://bucket/model/v1");
+ ModelVersionInfo info = new ModelVersionInfo(uris, null, null, null, null);
+
+ uris.put("unknown", "gs://bucket/model/MUTATED");
+ uris.put("extra", "gs://bucket/model/EXTRA");
+
+ // Event payloads are read-only; mutating the caller's map must not change
them.
+ Assertions.assertEquals(
+ ImmutableMap.of("unknown", "gs://bucket/model/v1"),
ImmutableMap.copyOf(info.uris()));
+ Assertions.assertEquals("gs://bucket/model/v1", info.uri());
+ }
+}
