Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(client): kneigbor-api has unmatched edge type with server #644

Closed
wants to merge 3 commits into from
Closed

fix(client): kneigbor-api has unmatched edge type with server #644

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e00fec5
fix(client): When calling the 'kneigbor' interface and returning the …
Nov 26, 2024
34522c1
fix(client): When calling the 'kneigbor' interface and returning the …
Nov 26, 2024
16b2526
Merge branch 'fix_client_kneighbor' of https://github.com/LiJie201901…
Nov 26, 2024
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
4 changes: 2 additions & 2 deletions hugegraph-client/src/main/java/org/apache/hugegraph/structure/traverser/Kneighbor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class Kneighbor {
@JsonProperty
private Set<Vertex> vertices;
@JsonProperty
private Set<Edge> edges;
private Set<?> edges;
Copy link
Member

@imbajin imbajin Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Although using <?> may take effect(no add/update operation here), it is still strange why the restriction of generics has been modified to arbitrary here.

It is possible that the server-side changes did not meet expectations?

Wanna to know in what case do I need to pass in a non-Edge Edge?(Is it because the definition of server-side generics is too broad?)

Copy link
Author

Choose a reason for hiding this comment

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

server code
org.apache.hugegraph.api.traversers.KneighborAPI#post
image

Copy link
Member

Choose a reason for hiding this comment

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

server code org.apache.hugegraph.api.traversers.KneighborAPI#post image

I mean the change in server maybe not suitable.

A better approach could be to adjust the <> type of server? You can take a look at it
image

Copy link
Author

@LiJie20190102 LiJie20190102 Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

How about this：If 'with_idge' is' false ', we will not return any information about the edge; If 'with-vertex' is' false 'and does not return vertex information, the' kneigbor 'parameter has already returned a set of vertex IDs
image

Copy link
Member

Choose a reason for hiding this comment

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

ow about this：If 'with_idge' is' false ', we will not return any information about the edge; If 'with-vertex' is' false 'and does not return vertex information, the' kneigbor 'parameter has already returned a set of vertex IDs

Looks okay, but what effect does the iterator returned earlier? Do you have to test what the potential effect is there? See if the test can pass rightly

Copy link
Author

@LiJie20190102 LiJie20190102 Nov 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

ow about this：If 'with_idge' is' false ', we will not return any information about the edge; If 'with-vertex' is' false 'and does not return vertex information, the' kneigbor 'parameter has already returned a set of vertex IDs

Looks okay, but what effect does the iterator returned earlier? Do you have to test what the potential effect is there? See if the test can pass rightly

if with_edge is false:
{ "kneighbor": [ "2:car1!2", "2:car2!4" ], "size": 2, "paths": [ { "objects": [ "1:zhangsan", "2:car1!2" ] }, { "objects": [ "1:zhangsan", "2:car2!4" ] } ], "vertices": [ { "id": "2:car2!4", "label": "car", "type": "vertex", "properties": { "age": 4, "qsdi_task_id": "car2_task_id_test", "qsdi_record_id": "test_record_id", "name": "car2", "qsdi_data_source": "1" } }, { "id": "1:zhangsan", "label": "people", "type": "vertex", "properties": { "age": 55, "qsdi_record_id": "926133154860048384", "name": "zhangsan", "qsdi_data_source": "1" } }, { "id": "2:car1!2", "label": "car", "type": "vertex", "properties": { "age": 2, "qsdi_task_id": "car1_task_id_test", "qsdi_record_id": "car1_log_id_test", "name": "car1", "qsdi_data_source": "1" } } ], "edges": [ "S1:zhangsan>3>3>>S2:car2!4", "S1:zhangsan>3>3>>S2:car1!2" ] }

if with_edge is true:
{ "kneighbor": [ "2:car1!2", "2:car2!4" ], "size": 2, "paths": [ { "objects": [ "1:zhangsan", "2:car1!2" ] }, { "objects": [ "1:zhangsan", "2:car2!4" ] } ], "vertices": [ { "id": "2:car2!4", "label": "car", "type": "vertex", "properties": { "age": 4, "qsdi_task_id": "car2_task_id_test", "qsdi_record_id": "test_record_id", "name": "car2", "qsdi_data_source": "1" } }, { "id": "1:zhangsan", "label": "people", "type": "vertex", "properties": { "age": 55, "qsdi_record_id": "926133154860048384", "name": "zhangsan", "qsdi_data_source": "1" } }, { "id": "2:car1!2", "label": "car", "type": "vertex", "properties": { "age": 2, "qsdi_task_id": "car1_task_id_test", "qsdi_record_id": "car1_log_id_test", "name": "car1", "qsdi_data_source": "1" } } ], "edges": [ { "id": "S1:zhangsan>3>3>>S2:car2!4", "label": "driver_single", "type": "edge", "outV": "1:zhangsan", "outVLabel": "people", "inV": "2:car2!4", "inVLabel": "car", "properties": { "qsdi_task_id": "zhangsan_car2_rask_id_test", "qsdi_record_id": "926133802515111937", "name": "ride12", "driver_count": 23, "qsdi_data_source": "1" } }, { "id": "S1:zhangsan>3>3>>S2:car1!2", "label": "driver_single", "type": "edge", "outV": "1:zhangsan", "outVLabel": "people", "inV": "2:car1!2", "inVLabel": "car", "properties": { "qsdi_record_id": "926133804008284160", "name": "ride88", "driver_count": 88, "qsdi_data_source": "1" } } ] }

Copy link
Member

Choose a reason for hiding this comment

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

THX, could try to submit a PR in server (or in your own forked repo to test the CI)

BTW, test/ in local env is also fine:)

Copy link
Contributor

Choose a reason for hiding this comment

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

after apache/incubator-hugegraph#2699 merged, we don't need to change Set<Edge> edges?

@JsonProperty
private ApiMeasure measure;

Expand All @@ -57,7 +57,7 @@ public Set<Vertex> vertices() {
return this.vertices;
}

public Set<Edge> edges() {
public Set<?> edges() {
return this.edges;
}

Expand Down
Loading