Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/*
* 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.hugegraph.api.traversers;

import static org.apache.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_ELEMENTS_LIMIT;
import static org.apache.hugegraph.traversal.algorithm.HugeTraverser.DEFAULT_MAX_DEGREE;

import java.util.ArrayList;
import java.util.List;
import java.util.Set;

import org.apache.hugegraph.util.E;
import org.apache.hugegraph.util.Log;
import org.slf4j.Logger;

import org.apache.hugegraph.HugeGraph;
import org.apache.hugegraph.api.API;
import org.apache.hugegraph.api.graph.EdgeAPI;
import org.apache.hugegraph.core.GraphManager;
import org.apache.hugegraph.backend.id.Id;
import org.apache.hugegraph.server.RestServer;
import org.apache.hugegraph.structure.HugeVertex;
import org.apache.hugegraph.traversal.algorithm.SameNeighborTraverser;
import org.apache.hugegraph.type.define.Directions;
import com.codahale.metrics.annotation.Timed;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ Critical: Import organization issue

The import statements are not properly ordered. Standard Java convention and most projects require imports to be grouped and sorted:

  1. java/javax imports
  2. Third-party imports
  3. Project imports

Current order mixes Apache HugeGraph imports with third-party libraries (Codahale, Jackson, Swagger, Jakarta). This violates standard Java conventions.

Suggestion: Reorder imports to follow standard conventions:

// Java standard
import java.util.ArrayList;
import java.util.List;
import java.util.Set;

// Third-party
import com.codahale.metrics.annotation.Timed;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.inject.Singleton;
import jakarta.ws.rs.POST;
// ... other jakarta imports

// Apache HugeGraph
import org.apache.hugegraph.HugeGraph;
import org.apache.hugegraph.api.API;
// ... other project imports


import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.inject.Singleton;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.Context;

@Path("graphspaces/{graphspace}/graphs/{graph}/traversers/sameneighborsbatch")
@Singleton
@Tag(name = "SameNeighborsBatchAPI")
public class SameNeighborsBatchAPI extends API {

Check warning on line 55 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L55

Added line #L55 was not covered by tests

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ Critical: Wrong Logger class usage

Line 56 creates a logger for RestServer.class instead of the current class SameNeighborsBatchAPI.class. This is incorrect and will make debugging difficult as log messages will appear to come from the wrong class.

Fix:

private static final Logger LOG = Log.logger(SameNeighborsBatchAPI.class);

private static final Logger LOG = Log.logger(RestServer.class);

Check warning on line 57 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L57

Added line #L57 was not covered by tests

@POST
@Timed
@Produces(APPLICATION_JSON_WITH_CHARSET)
public String sameNeighborsBatch(@Context GraphManager manager,
@PathParam("graphspace") String graphSpace,
@PathParam("graph") String graph,
Request req) {
LOG.debug("Graph [{}] get same neighbors batch, '{}'", graph, req.toString());

Check warning on line 66 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L66

Added line #L66 was not covered by tests

Directions dir = Directions.convert(EdgeAPI.parseDirection(req.direction));

Check warning on line 68 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L68

Added line #L68 was not covered by tests

ApiMeasurer measure = new ApiMeasurer();
HugeGraph g = graph(manager, graph);
SameNeighborTraverser traverser = new SameNeighborTraverser(g);

Check warning on line 72 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L70-L72

Added lines #L70 - L72 were not covered by tests

List<Set<Id>> result = new ArrayList<>();

Check warning on line 74 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L74

Added line #L74 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Important: Missing validation

No null check for req.vertexList before iterating. If the request body is malformed or missing vertex_list, this will throw a NullPointerException.

Add validation:

E.checkArgumentNotNull(req.vertexList, "vertex_list cannot be null");
E.checkArgument(!req.vertexList.isEmpty(), "vertex_list cannot be empty");

for (List<Object> vertexPair : req.vertexList) {
    E.checkArgumentNotNull(vertexPair, "vertex pair cannot be null");
    E.checkArgument(vertexPair.size() == 2, "vertex_list size must be 2");
    // ...
}

for (List<Object> vertexPair : req.vertexList) {
E.checkArgument(vertexPair.size() == 2,
"vertex_list size must be 2");
Id sourceId = HugeVertex.getIdValue(vertexPair.get(0));
Id targetId = HugeVertex.getIdValue(vertexPair.get(1));
Set<Id> neighbors = traverser.sameNeighbors(sourceId, targetId, dir,

Check warning on line 80 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L78-L80

Added lines #L78 - L80 were not covered by tests
req.label, req.maxDegree, req.limit);
result.add(neighbors);
}
measure.addIterCount(traverser.vertexIterCounter.get(),
traverser.edgeIterCounter.get());

Check warning on line 85 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L82-L85

Added lines #L82 - L85 were not covered by tests

return manager.serializer(g, measure.measures()).writeList("same_neighbors", result);

Check warning on line 87 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L87

Added line #L87 was not covered by tests
}

private static class Request {

Check warning on line 90 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L90

Added line #L90 was not covered by tests
@JsonProperty("vertex_list")
private List<List<Object>> vertexList;
@JsonProperty("direction")
private String direction;
@JsonProperty("label")
private String label;
@JsonProperty("max_degree")
public long maxDegree = Long.parseLong(DEFAULT_MAX_DEGREE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Important: Inefficient parsing of constants

Lines 97-98 use Long.parseLong() and Integer.parseInt() on constants that are already defined as strings. This is inefficient and error-prone.

If DEFAULT_MAX_DEGREE and DEFAULT_ELEMENTS_LIMIT are string constants like "10000", consider:

  1. Defining them as numeric constants directly, OR
  2. Parsing them once in a static initializer, OR
  3. If they must remain as strings elsewhere, create numeric constant versions for API usage

Current:

public long maxDegree = Long.parseLong(DEFAULT_MAX_DEGREE);
public int limit = Integer.parseInt(DEFAULT_ELEMENTS_LIMIT);

Better approach:

public long maxDegree = DEFAULT_MAX_DEGREE_VALUE; // Define as long constant
public int limit = DEFAULT_ELEMENTS_LIMIT_VALUE;   // Define as int constant

@JsonProperty("limit")
public int limit = Integer.parseInt(DEFAULT_ELEMENTS_LIMIT);

Check warning on line 100 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L97-L100

Added lines #L97 - L100 were not covered by tests

@Override
public String toString() {
ObjectMapper om = new ObjectMapper();
String vertexStr = "";

Check warning on line 105 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L104-L105

Added lines #L104 - L105 were not covered by tests
try {
vertexStr = om.writeValueAsString(this.vertexList);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Important: Silent exception swallowing

Lines 106-107 catch an exception but do nothing with it - not even logging. This is a bad practice that makes debugging impossible.

Issues:

  1. No logging of the error
  2. Returns empty string on failure, which may cause confusing behavior
  3. No indication to caller that serialization failed

Fix:

try {
    vertexStr = om.writeValueAsString(this.vertexList);
} catch (Exception e) {
    LOG.warn("Failed to serialize vertex list: {}", e.getMessage());
    vertexStr = "<serialization failed>";
}

} catch (Exception e) {
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty catch block silently swallows exceptions. Consider logging the error or at minimum adding a comment explaining why the exception is intentionally ignored. For example: LOG.debug(\"Failed to serialize vertex list: {}\", e.getMessage());

Suggested change
} catch (Exception e) {
} catch (Exception e) {
LOG.debug("Failed to serialize vertex list: {}", e.getMessage());

Copilot uses AI. Check for mistakes.
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Minor: Typo in format string

Line 109 has an incomplete format string - missing closing brace } after limit=%d.

Fix:

return String.format("SameNeighborsBatchRequest{vertex=%s,direction=%s,label=%s," +
                     "max_degree=%d,limit=%d}", vertexStr, this.direction,
        this.label, this.maxDegree, this.limit);

return String.format("SameNeighborsBatchRequest{vertex=%s,direction=%s,label=%s," +

Check warning on line 110 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L107-L110

Added lines #L107 - L110 were not covered by tests
"max_degree=%d,limit=%d", vertexStr, this.direction,
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing closing brace in format string. The string should end with } after limit=%d. Current output will be malformed like limit=10 instead of limit=10}.

Suggested change
"max_degree=%d,limit=%d", vertexStr, this.direction,
"max_degree=%d,limit=%d}", vertexStr, this.direction,

Copilot uses AI. Check for mistakes.
this.label, this.maxDegree, this.limit);

Check warning on line 112 in hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java

View check run for this annotation

Codecov / codecov/patch

hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/traversers/SameNeighborsBatchAPI.java#L112

Added line #L112 was not covered by tests
}
}
}
Loading