[JanusGraph] Fixed - should contain id property

[Tin-Nguyen]

Description

This is the fix related to my comment in https://github.com/microsoft/spring-data-gremlin/issues/230#issuecomment-536178388 about the issue I'm facing in my Java application using spring-data-gremlin working with janusgraph database

java.lang.IllegalArgumentException: should contain id property

Gremlin-driver: 3.2.4 spring-data-gremlin: 2.1.7 Spring Boot: 2.1.8.RELEASE

Root cause

• The issue happened after the library finished inserting the new data into JanusGraph database.
• gremlin-driver returns a different data structure, like as below That's why validate() function raises IllegalArgumentException when it identifies there is no id, label, type, etc... contained in the Result instance.

I'm going to write some Tests to cover the logics and test it via my application shortly today.

Related PRs

N/A

Todos

• [ ] Tests
• [ ] Documentation

Steps to Test

Steps to test code change

[msftclas]

All CLA requirements met.

[codecov-io]

Codecov Report

Merging #243 into master will decrease coverage by 0.46%. The diff coverage is 73.33%.

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
- Coverage   86.24%   85.77%   -0.47%     
==========================================
  Files          60       60              
  Lines        1352     1364      +12     
  Branches      231      234       +3     
==========================================
+ Hits         1166     1170       +4     
- Misses         74       80       +6     
- Partials      112      114       +2

Impacted Files	Coverage Δ
...icrosoft/spring/data/gremlin/common/Constants.java	87.5% <ø> (ø)	:arrow_up:
...lin/conversion/result/GremlinResultEdgeReader.java	87.5% <100%> (-0.38%)	:arrow_down:
...n/conversion/result/GremlinResultVertexReader.java	95.83% <100%> (-0.17%)	:arrow_down:
...conversion/result/AbstractGremlinResultReader.java	55% <42.85%> (-32.5%)	:arrow_down:
...ta/gremlin/config/GremlinConfigurationSupport.java	95.83% <0%> (+0.37%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b249bae...1e043e5. Read the comment docs.

[Tin-Nguyen]

I found another issue related Edge id field returned from JanusGraph id value isn't Integer type, it's the map with relationId: <String>

[Tin-Nguyen]

I'm facing another issue when getting the data from JanusGraph using findAll() function. The data structure returned from gremlin-driver is quite different as the code is doing Need to fix it as well

[superrdean]

@Tin-Nguyen Travis-ci seems not to pass, please check it and fix it.

[Tin-Nguyen]

@neuqlz I'm going to fix the PR within next week. I will let you know once it fixed soon. Thanks.

[manjurhassan]

Hi everyone, I am new to open source development but I have a fair enough problem solving background in Graphs, this looks like an interesting project to start my open source contribution journey. It would be great if you could point me in the right direction to get started.

[Tin-Nguyen]

sorry guys, I was busy with some urgent works. it's time to back to get this change done within next week.

[Incarnation-p-lee]

Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/main/java/com/microsoft/spring/data/gremlin/conversion/result/AbstractGremlinResultReader.java  4

Clones removed
==============
+ src/main/java/com/microsoft/spring/data/gremlin/conversion/result/GremlinResultEdgeReader.java  -1
+ src/main/java/com/microsoft/spring/data/gremlin/conversion/result/GremlinResultVertexReader.java  -1

See the complete overview on Codacy

[arvind-das]

is anyone going to merge this fix ? This is a blocker in integrating this orm framework.

[codecov-commenter]

Codecov Report

Merging #243 into master will decrease coverage by 0.44%. The diff coverage is 73.33%.

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
- Coverage   86.21%   85.77%   -0.45%     
==========================================
  Files          60       60              
  Lines        1357     1364       +7     
  Branches      232      234       +2     
==========================================
  Hits         1170     1170              
- Misses         74       80       +6     
- Partials      113      114       +1     

Impacted Files	Coverage Δ
...icrosoft/spring/data/gremlin/common/Constants.java	87.50% <ø> (ø)
...conversion/result/AbstractGremlinResultReader.java	55.00% <42.85%> (-32.50%)	:arrow_down:
...lin/conversion/result/GremlinResultEdgeReader.java	87.50% <100.00%> (-0.38%)	:arrow_down:
...n/conversion/result/GremlinResultVertexReader.java	95.83% <100.00%> (-0.17%)	:arrow_down:
...soft/spring/data/gremlin/common/GremlinConfig.java	100.00% <0.00%> (ø)
...ta/gremlin/config/GremlinConfigurationSupport.java	95.83% <0.00%> (+0.37%)	:arrow_up:
...oft/spring/data/gremlin/common/GremlinFactory.java	91.30% <0.00%> (+2.41%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a1c230f...c24ed3b. Read the comment docs.

[arvind-das]

is this branch ever going to merge ? this is a highly needed issue fix, one can not get started with it since it does not assure it will save.

[Tin-Nguyen]

@arvind-das sorry for the delay, I will try to improve the test coverage and merge the PR this weekend.

[arvind-das]

@arvind-das sorry for the delay, I will try to improve the test coverage and merge the PR this weekend.

Thanks for the update Tim. I know you are busy in your work office work too. Meanwhile I created some generic methods to do my job. They solved the problem for a short time , still need to handle a lot of conditions. Once you merge this, I will be integrating thins into my app.