This is an automated email from the ASF dual-hosted git repository.
SYaoJun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-graphar.git
The following commit(s) were added to refs/heads/main by this push:
new 0a51cd20 fix(java): align edge metadata paths with GraphAr v1 (#946)
0a51cd20 is described below
commit 0a51cd20657cc85435b46b16e752fdb9c836b363
Author: alex <[email protected]>
AuthorDate: Tue Aug 18 02:41:26 2026 +0300
fix(java): align edge metadata paths with GraphAr v1 (#946)
* fix(java): align edge metadata paths with GraphAr v1
Restore GraphAr v1 edge counts, offsets, topology, and property chunk
suffixes in the pure-Java metadata API.
Tests: mvn --no-transfer-progress clean verify -Dspotless.check.skip=true
(devcontainer; 124 tests)
Relates-to: #943
* fix(java): resolve edge paths with URI semantics and reject a null prefix
Review feedback on apache/incubator-graphar#946: the private path helper
concatenated strings, so a null adjacent-list or property-group prefix
silently produced a path segment spelled "null", and separators were not
handled as URI references.
Resolve the child path against the base URI with RFC 3986 reference
resolution, keep the trailing-separator normalization that resolution needs
so the last base segment survives, reject a null child path with
IllegalArgumentException, and reject a child path that parses as an absolute
URI. The child path is parsed rather than re-escaped because prefixes are
already stored in URI form.
Tests: mvn -pl info test, 126 tests, 0 failures/errors/skips.
---------
Co-authored-by: keksmd <[email protected]>
---
.../java/org/apache/graphar/info/EdgeInfo.java | 82 +++++++++++++++++---
.../org/apache/graphar/info/GraphInfoTest.java | 87 +++++++++++++++-------
.../org/apache/graphar/info/GraphInfoUriTest.java | 80 ++++++++++++++++++++
3 files changed, 209 insertions(+), 40 deletions(-)
diff --git
a/maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java
b/maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java
index 91cb48c4..5e546779 100644
--- a/maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java
+++ b/maven-projects/info/src/main/java/org/apache/graphar/info/EdgeInfo.java
@@ -26,6 +26,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
@@ -462,38 +463,95 @@ public class EdgeInfo {
return propertyGroups.getPropertyGroup(property);
}
- public URI getPropertyGroupUri(PropertyGroup propertyGroup) {
+ public URI getPropertyGroupUri(PropertyGroup propertyGroup, AdjListType
adjListType) {
checkPropertyGroupExist(propertyGroup);
- return getBaseUri().resolve(propertyGroup.getBaseUri());
+ return resolvePath(getAdjacentListBaseUri(adjListType),
propertyGroup.getPrefix());
}
- public URI getPropertyGroupChunkUri(PropertyGroup propertyGroup, long
chunkIndex) {
- // PropertyGroup will be checked in getPropertyGroupPrefix
- return getPropertyGroupUri(propertyGroup).resolve("chunk" +
chunkIndex);
+ public URI getPropertyGroupChunkUri(
+ PropertyGroup propertyGroup,
+ AdjListType adjListType,
+ long vertexChunkIndex,
+ long edgeChunkIndex) {
+ return resolvePath(
+ getPropertyGroupUri(propertyGroup, adjListType),
+ "part" + vertexChunkIndex + "/chunk" + edgeChunkIndex);
}
public URI getAdjacentListUri(AdjListType adjListType) {
- return
getBaseUri().resolve(getAdjacentList(adjListType).getBaseUri()).resolve("adj_list/");
+ return resolvePath(getAdjacentListBaseUri(adjListType), "adj_list/");
}
- public URI getAdjacentListChunkUri(AdjListType adjListType, long
vertexChunkIndex) {
- return getAdjacentListUri(adjListType).resolve("chunk" +
vertexChunkIndex);
+ public URI getAdjacentListChunkUri(
+ AdjListType adjListType, long vertexChunkIndex, long
edgeChunkIndex) {
+ return resolvePath(
+ getAdjacentListUri(adjListType),
+ "part" + vertexChunkIndex + "/chunk" + edgeChunkIndex);
}
public URI getOffsetUri(AdjListType adjListType) {
- return getAdjacentListUri(adjListType).resolve("offset/");
+ return resolvePath(getAdjacentListBaseUri(adjListType), "offset/");
}
public URI getOffsetChunkUri(AdjListType adjListType, long
vertexChunkIndex) {
- return getOffsetUri(adjListType).resolve("chunk" + vertexChunkIndex);
+ return resolvePath(getOffsetUri(adjListType), "chunk" +
vertexChunkIndex);
}
public URI getVerticesNumFileUri(AdjListType adjListType) {
- return getAdjacentListUri(adjListType).resolve("vertex_count");
+ return resolvePath(getAdjacentListBaseUri(adjListType),
"vertex_count");
}
public URI getEdgesNumFileUri(AdjListType adjListType, long
vertexChunkIndex) {
- return getAdjacentListUri(adjListType).resolve("edge_count" +
vertexChunkIndex);
+ return resolvePath(getAdjacentListBaseUri(adjListType), "edge_count" +
vertexChunkIndex);
+ }
+
+ private URI getAdjacentListBaseUri(AdjListType adjListType) {
+ return resolvePath(getBaseUri(),
getAdjacentList(adjListType).getPrefix());
+ }
+
+ /**
+ * Resolves a GraphAr child path against a base URI with RFC 3986
reference resolution instead
+ * of raw string concatenation, so that path separators, escaping and
normalization follow URI
+ * semantics.
+ *
+ * @param baseUri the directory the child path is relative to
+ * @param childPath a relative GraphAr path such as a prefix or a chunk
file name
+ * @return the resolved absolute-or-relative URI of the child
+ * @throws IllegalArgumentException if {@code childPath} is null, is not a
valid URI path, or is
+ * not a relative reference
+ */
+ private static URI resolvePath(URI baseUri, String childPath) {
+ Objects.requireNonNull(baseUri, "baseUri must not be null");
+ if (childPath == null) {
+ throw new IllegalArgumentException("childPath must not be null");
+ }
+ return asDirectoryUri(baseUri).resolve(asRelativeReference(childPath));
+ }
+
+ /**
+ * Returns the base URI in the directory form URI resolution requires,
because resolving against
+ * a URI whose path has no trailing separator replaces its last segment.
+ */
+ private static URI asDirectoryUri(URI baseUri) {
+ String rawPath = baseUri.getRawPath();
+ if (rawPath == null) {
+ throw new IllegalArgumentException("baseUri must be hierarchical,
but was " + baseUri);
+ }
+ return rawPath.endsWith("/") ? baseUri : URI.create(baseUri + "/");
+ }
+
+ /**
+ * Parses a GraphAr path fragment as a relative URI reference. Prefixes
are already stored in
+ * URI form, so the fragment is parsed rather than re-escaped, which would
double-encode an
+ * escaped prefix.
+ */
+ private static URI asRelativeReference(String childPath) {
+ URI reference = URI.create(childPath);
+ if (reference.isAbsolute()) {
+ throw new IllegalArgumentException(
+ "childPath must be a relative reference, but was " +
childPath);
+ }
+ return reference;
}
public void dump(Writer output) {
diff --git
a/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoTest.java
b/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoTest.java
index fc931b60..fe5bcbce 100644
---
a/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoTest.java
+++
b/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoTest.java
@@ -21,6 +21,8 @@ package org.apache.graphar.info;
import java.io.IOException;
import java.net.URI;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -306,31 +308,31 @@ public class GraphInfoTest {
Assert.assertEquals("ordered_by_source/",
adjOrderBySource.getPrefix());
Assert.assertEquals(URI.create("ordered_by_source/"),
adjOrderBySource.getBaseUri());
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/vertex_count"),
+
URI.create("edge/person_knows_person/ordered_by_source/vertex_count"),
knowsEdgeInfo.getVerticesNumFileUri(AdjListType.ordered_by_source));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/edge_count0"),
+
URI.create("edge/person_knows_person/ordered_by_source/edge_count0"),
knowsEdgeInfo.getEdgesNumFileUri(AdjListType.ordered_by_source, 0));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/edge_count4"),
+
URI.create("edge/person_knows_person/ordered_by_source/edge_count4"),
knowsEdgeInfo.getEdgesNumFileUri(AdjListType.ordered_by_source, 4));
Assert.assertEquals(
URI.create("edge/person_knows_person/ordered_by_source/adj_list/"),
knowsEdgeInfo.getAdjacentListUri(AdjListType.ordered_by_source));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/chunk0"),
-
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_source, 0));
+
URI.create("edge/person_knows_person/ordered_by_source/adj_list/part0/chunk0"),
+
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_source, 0, 0));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/chunk4"),
-
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_source, 4));
+
URI.create("edge/person_knows_person/ordered_by_source/adj_list/part4/chunk2"),
+
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_source, 4, 2));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/offset/"),
+
URI.create("edge/person_knows_person/ordered_by_source/offset/"),
knowsEdgeInfo.getOffsetUri(AdjListType.ordered_by_source));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/offset/chunk0"),
+
URI.create("edge/person_knows_person/ordered_by_source/offset/chunk0"),
knowsEdgeInfo.getOffsetChunkUri(AdjListType.ordered_by_source,
0));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/offset/chunk4"),
+
URI.create("edge/person_knows_person/ordered_by_source/offset/chunk4"),
knowsEdgeInfo.getOffsetChunkUri(AdjListType.ordered_by_source,
4));
// test ordered by destination adjacency list
@@ -341,31 +343,31 @@ public class GraphInfoTest {
Assert.assertEquals("ordered_by_dest/",
adjOrderByDestination.getPrefix());
Assert.assertEquals(URI.create("ordered_by_dest/"),
adjOrderByDestination.getBaseUri());
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/vertex_count"),
+
URI.create("edge/person_knows_person/ordered_by_dest/vertex_count"),
knowsEdgeInfo.getVerticesNumFileUri(AdjListType.ordered_by_dest));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/edge_count0"),
+
URI.create("edge/person_knows_person/ordered_by_dest/edge_count0"),
knowsEdgeInfo.getEdgesNumFileUri(AdjListType.ordered_by_dest,
0));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/edge_count4"),
+
URI.create("edge/person_knows_person/ordered_by_dest/edge_count4"),
knowsEdgeInfo.getEdgesNumFileUri(AdjListType.ordered_by_dest,
4));
Assert.assertEquals(
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/"),
knowsEdgeInfo.getAdjacentListUri(AdjListType.ordered_by_dest));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/chunk0"),
-
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_dest, 0));
+
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/part0/chunk0"),
+
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_dest, 0, 0));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/chunk4"),
-
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_dest, 4));
+
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/part4/chunk2"),
+
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_dest, 4, 2));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/offset/"),
+ URI.create("edge/person_knows_person/ordered_by_dest/offset/"),
knowsEdgeInfo.getOffsetUri(AdjListType.ordered_by_dest));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/offset/chunk0"),
+
URI.create("edge/person_knows_person/ordered_by_dest/offset/chunk0"),
knowsEdgeInfo.getOffsetChunkUri(AdjListType.ordered_by_dest,
0));
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_dest/adj_list/offset/chunk4"),
+
URI.create("edge/person_knows_person/ordered_by_dest/offset/chunk4"),
knowsEdgeInfo.getOffsetChunkUri(AdjListType.ordered_by_dest,
4));
}
@@ -377,7 +379,9 @@ public class GraphInfoTest {
IllegalArgumentException illegalArgumentException =
Assert.assertThrows(
IllegalArgumentException.class,
- () -> knowsEdgeInfo.getPropertyGroupUri(notExistPg));
+ () ->
+ knowsEdgeInfo.getPropertyGroupUri(
+ notExistPg,
AdjListType.ordered_by_source));
Assert.assertEquals(
"Property group "
+ notExistPg
@@ -399,14 +403,16 @@ public class GraphInfoTest {
Assert.assertEquals(URI.create("creationDate/"),
propertyGroup.getBaseUri());
Assert.assertEquals(FileType.CSV, propertyGroup.getFileType());
Assert.assertEquals(
- URI.create("edge/person_knows_person/creationDate/"),
- knowsEdgeInfo.getPropertyGroupUri(propertyGroup));
+
URI.create("edge/person_knows_person/ordered_by_source/creationDate/"),
+ knowsEdgeInfo.getPropertyGroupUri(propertyGroup,
AdjListType.ordered_by_source));
Assert.assertEquals(
- URI.create("edge/person_knows_person/creationDate/chunk0"),
- knowsEdgeInfo.getPropertyGroupChunkUri(propertyGroup, 0));
+
URI.create("edge/person_knows_person/ordered_by_source/creationDate/part0/chunk0"),
+ knowsEdgeInfo.getPropertyGroupChunkUri(
+ propertyGroup, AdjListType.ordered_by_source, 0, 0));
Assert.assertEquals(
- URI.create("edge/person_knows_person/creationDate/chunk4"),
- knowsEdgeInfo.getPropertyGroupChunkUri(propertyGroup, 4));
+
URI.create("edge/person_knows_person/ordered_by_dest/creationDate/part4/chunk2"),
+ knowsEdgeInfo.getPropertyGroupChunkUri(
+ propertyGroup, AdjListType.ordered_by_dest, 4, 2));
// edge properties in group 1
Assert.assertNotNull(propertyGroup.getPropertyList());
Assert.assertEquals(1, propertyGroup.getPropertyList().size());
@@ -546,7 +552,32 @@ public class GraphInfoTest {
Assert.assertEquals(AdjListType.ordered_by_source,
adjOrderBySource.getType());
Assert.assertEquals("ordered_by_source/",
adjOrderBySource.getPrefix());
Assert.assertEquals(
-
URI.create("edge/person_knows_person/ordered_by_source/adj_list/offset/"),
+
URI.create("edge/person_knows_person/ordered_by_source/offset/"),
knowsEdgeInfo.getOffsetUri(AdjListType.ordered_by_source));
+
+ URI adjacencyChunkUri =
+
knowsEdgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_source, 2, 1);
+ URI propertyChunkUri =
+ knowsEdgeInfo.getPropertyGroupChunkUri(
+ creationDate, AdjListType.ordered_by_source, 2, 1);
+ URI edgeCountUri =
knowsEdgeInfo.getEdgesNumFileUri(AdjListType.ordered_by_source, 2);
+
+ Assert.assertEquals(
+
URI.create("edge/person_knows_person/ordered_by_source/adj_list/part2/chunk1"),
+ adjacencyChunkUri);
+ Assert.assertEquals(
+
URI.create("edge/person_knows_person/ordered_by_source/creationDate/part2/chunk1"),
+ propertyChunkUri);
+ Assert.assertEquals(
+
URI.create("edge/person_knows_person/ordered_by_source/edge_count2"),
edgeCountUri);
+ Assert.assertTrue(
+ Files.isRegularFile(
+
Paths.get(graphInfo.getBaseUri().resolve(adjacencyChunkUri).getPath())));
+ Assert.assertTrue(
+ Files.isRegularFile(
+
Paths.get(graphInfo.getBaseUri().resolve(propertyChunkUri).getPath())));
+ Assert.assertTrue(
+ Files.isRegularFile(
+
Paths.get(graphInfo.getBaseUri().resolve(edgeCountUri).getPath())));
}
}
diff --git
a/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoUriTest.java
b/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoUriTest.java
index 78fb5616..f4ca2056 100644
---
a/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoUriTest.java
+++
b/maven-projects/info/src/test/java/org/apache/graphar/info/GraphInfoUriTest.java
@@ -20,6 +20,10 @@
package org.apache.graphar.info;
import java.net.URI;
+import java.util.List;
+import org.apache.graphar.info.type.AdjListType;
+import org.apache.graphar.info.type.DataType;
+import org.apache.graphar.info.type.FileType;
import org.apache.graphar.info.yaml.VertexYaml;
import org.junit.Assert;
import org.junit.Test;
@@ -92,4 +96,80 @@ public class GraphInfoUriTest {
URI.create("file:///tmp/vertex/person/firstName_lastName_gender/chunk0"),
vertexInfo.getPropertyGroupChunkUri(vertexInfo.getPropertyGroups().get(1), 0));
}
+
+ @Test
+ public void testEdgePathsNormalizePrefixesWithoutTrailingSlashes() {
+ PropertyGroup propertyGroup =
+ new PropertyGroup(
+ List.of(new Property("created", DataType.STRING,
false, false)),
+ FileType.PARQUET,
+ "created");
+ EdgeInfo edgeInfo =
+ new EdgeInfo(
+ "person",
+ "knows",
+ "person",
+ 1024,
+ 100,
+ 100,
+ false,
+ "edge/person_knows_person",
+ "gar/v1",
+ List.of(
+ new AdjacentList(
+ AdjListType.ordered_by_source,
+ FileType.PARQUET,
+ "ordered_by_source")),
+ List.of(propertyGroup));
+
+ Assert.assertEquals(
+
URI.create("edge/person_knows_person/ordered_by_source/edge_count2"),
+ edgeInfo.getEdgesNumFileUri(AdjListType.ordered_by_source, 2));
+ Assert.assertEquals(
+
URI.create("edge/person_knows_person/ordered_by_source/adj_list/part2/chunk1"),
+
edgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_source, 2, 1));
+ Assert.assertEquals(
+
URI.create("edge/person_knows_person/ordered_by_source/created/part2/chunk1"),
+ edgeInfo.getPropertyGroupChunkUri(
+ propertyGroup, AdjListType.ordered_by_source, 2, 1));
+ }
+
+ @Test
+ public void testEdgePathsRejectMissingAdjacentListPrefix() {
+ EdgeInfo edgeInfo = edgeInfoWithAdjacentListPrefix(null);
+
+ IllegalArgumentException failure =
+ Assert.assertThrows(
+ IllegalArgumentException.class,
+ () ->
edgeInfo.getAdjacentListUri(AdjListType.ordered_by_source));
+ Assert.assertEquals("childPath must not be null",
failure.getMessage());
+ }
+
+ @Test
+ public void testEdgePathsKeepEscapedPrefixesUnchanged() {
+ EdgeInfo edgeInfo =
edgeInfoWithAdjacentListPrefix("ordered%20by%20source");
+
+ Assert.assertEquals(
+
URI.create("edge/person_knows_person/ordered%20by%20source/adj_list/part0/chunk0"),
+
edgeInfo.getAdjacentListChunkUri(AdjListType.ordered_by_source, 0, 0));
+ }
+
+ private static EdgeInfo edgeInfoWithAdjacentListPrefix(String prefix) {
+ return new EdgeInfo(
+ "person",
+ "knows",
+ "person",
+ 1024,
+ 100,
+ 100,
+ false,
+ "edge/person_knows_person",
+ "gar/v1",
+ List.of(new AdjacentList(AdjListType.ordered_by_source,
FileType.PARQUET, prefix)),
+ List.of(
+ new PropertyGroup(
+ List.of(new Property("created",
DataType.STRING, false, false)),
+ FileType.PARQUET,
+ "created")));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
