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.

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

Support uuid id type #618

Merged
merged 5 commits into from
Jul 30, 2019
Merged

Support uuid id type #618

merged 5 commits into from
Jul 30, 2019

Conversation

javeme
Copy link
Contributor

@javeme javeme commented Jul 19, 2019

implement #617

Change-Id: Ie05d7a3609dd1695728f0a2f4a322448ac8aabeb

@codecov
Copy link

codecov bot commented Jul 19, 2019

Codecov Report

Merging #618 into master will decrease coverage by 0.9%.
The diff coverage is 81.37%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #618      +/-   ##
============================================
- Coverage     71.89%   70.98%   -0.91%     
+ Complexity     3615     3574      -41     
============================================
  Files           218      219       +1     
  Lines         16802    16882      +80     
  Branches       2389     2399      +10     
============================================
- Hits          12079    11983      -96     
- Misses         3467     3630     +163     
- Partials       1256     1269      +13
Impacted Files Coverage Δ Complexity Δ
...n/java/com/baidu/hugegraph/schema/VertexLabel.java 100% <ø> (ø) 8 <0> (ø) ⬇️
...m/baidu/hugegraph/backend/tx/GraphTransaction.java 82.55% <ø> (ø) 210 <0> (ø) ⬇️
...du/hugegraph/backend/cache/CachedBackendStore.java 10% <0%> (ø) 0 <0> (ø) ⬇️
...in/java/com/baidu/hugegraph/io/HugeGryoModule.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...a/com/baidu/hugegraph/traversal/optimize/Text.java 100% <100%> (ø) 3 <1> (+1) ⬆️
...java/com/baidu/hugegraph/structure/HugeVertex.java 77.19% <100%> (+0.4%) 64 <0> (+3) ⬆️
.../java/com/baidu/hugegraph/structure/HugeIndex.java 68.91% <100%> (ø) 20 <0> (ø) ⬇️
...va/com/baidu/hugegraph/type/define/IdStrategy.java 80.95% <100%> (+0.95%) 9 <4> (+1) ⬆️
...gegraph/backend/serializer/BinaryBackendEntry.java 60.81% <100%> (ø) 18 <0> (ø) ⬇️
...c/main/java/com/baidu/hugegraph/backend/id/Id.java 100% <100%> (ø) 6 <6> (?)
... and 21 more

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 b01549a...4e26368. Read the comment docs.

@javeme javeme force-pushed the support-uuid branch 2 times, most recently from d5ce26d to 59c4fae Compare July 19, 2019 13:29
@@ -143,6 +143,7 @@ public static void loadSchema(final HugeGraph graph) {
schema.vertexLabel("reviewer").properties("name", "timestamp")
.primaryKeys("name").create();


Copy link
Contributor

Choose a reason for hiding this comment

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

delete empty line

@@ -744,6 +743,75 @@ public void testAddVertexWithCustomizeNumberIdStrategyWithoutValidId() {
});
}

@Test
public void testAddVertexWithCustomizeUuidStrategy() {
Copy link
Contributor

Choose a reason for hiding this comment

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

testAddVertexWithCustomizeUuidIdStrategy()

}

@Test
public void testAddVertexWithCustomizeUuidStrategyWithoutValidId() {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -318,6 +326,7 @@ public BytesBuffer writeId(Id id, boolean big) {
len -= 1;
int high = len >> 8;
int low = len & 0xff;
assert high != 0x7f; // The tail 7 bits 1 reserved for UUID
Copy link
Contributor

Choose a reason for hiding this comment

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

'len <= BIG_ID_MAX_LEN' can't ensure 'high != 0x7f',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

id len must be in range [0, 126]

public static final int ID_MAX_LEN = UINT8_MAX & 0x7f + 1; // 128
public static final int BIG_ID_MAX_LEN = UINT16_MAX & 0x7fff + 1; // 32768
public static final int ID_MAX_LEN = UINT8_MAX & 0x7f; // 127
public static final int BIG_ID_MAX_LEN = UINT16_MAX & 0x7fff; // 32767
Copy link
Contributor

Choose a reason for hiding this comment

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

max length is 32767-256

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, 1byte id len must be in range [0, fe], 2byte id len must be in range [0, feff]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, 1byte id len must be in range [0, 7e], 2byte id len must be in range [0, 7eff]

implement #617

Change-Id: Ie05d7a3609dd1695728f0a2f4a322448ac8aabeb
Change-Id: Ia8046307813f21104c35e74afcb36cff2e3694dc
Change-Id: I0174d1ace1f193124cc37f4f1689ca087b91f525
Change-Id: I6f45a5a709fad866642073ddeb231b7500acf902
}

public enum IdType {
UNKONW,
Copy link
Contributor

Choose a reason for hiding this comment

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

spell error UNKONW -> UNKNOWN

return of(LongEncoding.decode(id));
case UUID:
byte[] bytes = Base64.getDecoder().decode(id);
return of(bytes, type);
Copy link
Contributor

Choose a reason for hiding this comment

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

just return of(bytes, UUID);

Change-Id: I3ab8a9e7bb25dc05f990a0cebf5341644f49728d
@Linary Linary merged commit 674a5e7 into master Jul 30, 2019
@Linary Linary deleted the support-uuid branch July 30, 2019 10:41
@javeme javeme mentioned this pull request Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants