Uuid to Guid

[espiegelberg]

This PR piggy backs off the changes merged from PR https://github.com/graphaware/neo4j-uuid/pull/31.

Due to the merging of the above PR https://github.com/graphaware/neo4j-uuid/pull/31, the UUID module now allows other generators to be made use of and UUID’s are not necessarily the format of the generated ID. The SequenceGenerator, for example, will instead generate an incremental numerical series. Users, through configuration, are now able to implement generates that generate ID’s as they see fit as well, and there are no guarantees what what is generated will be a UUID.

The substance of this PR is within the generator package where I have created a GuidGenerator interface, now used in favor of the UuidGenerator. This interface allows implementors to return Object, rather than UuidGenerator’s String, so that generators who return a numerical value can have their value stored in Neo4j as a number rather than a String. In other words, the UuidGenerator interface forced all generated id’s to be stored in Neo4j as a String while this change allows generators to have their id’s stored as Strings or Numbers (or whatever these they choose to return).

If you accept that, due to PR https://github.com/graphaware/neo4j-uuid/pull/31, this module can now generate id’s other than UUID’s then it stands to reason that this module could/would/should be renamed and rebranded from UUID module to something such as GUID module. Accordingly, I have updated the entire project to make use of guid rather than uuid (such as in the pom.xml, package structures, README, etc). Admittedly, this is perhaps (likely?) larger changes than GraphAware is willing to accept, given that it essentially rebrands the module. Just the same, I offer the changes as food for thought. Should GraphAware decide you’re not ready to accept such a sweeping change, I would then be willing to submit a PR with just the changes needed to support the change from the UuidGenerator’s interface to the GuidGenerator, as I feel the ability for a generator that returns numerical id's, such as the SequenceGenerator, to have their value stored in Neo4j as numbers rather than Strings is worthwhile.

[ikwattro]

@espiegelberg I'll leave this decision to merge to all the team and especially to @bachmanm .

For my pov, I'm afraid I'll have to say no, mostly for historical reasons and the community cognitive relation to the neo4j-uuid keyword. But we'll discuss internally.

[bachmanm]

Thanks for the PR @espiegelberg. I don't think we should rename the whole project or anything else from UUID to GUID. UUID doesn't imply String, does it? We can imho still entertain the idea of relaxing the String type to Object without renaming a whole lot of things.

To make sure there is no major impact of this type change, I think I'd like to see a few tests for the procedures and REST APIs that use different types, such as long and int, and also tests for scenarios where the user intends to do the right thing but uses wrong types as arguments.

I would also like to see a test of what happens when the UUID generator actually does return an Object which Neo4j can't store as a property. Thinking about it, I believe we should do a check at startup time, generate a "test" UUID and verify that its type is something that can be stored as a Neo4j property.

Thanks!

[Joshfindit]

Is there any issue with having both "uuid" (classic) and "guid" (this PR)? That type of approach may alleviate the friction about "rebranding".

On Nov 27, 2016, at 11:36 AM, Michal Bachman notifications@github.com wrote:

Thanks for the PR @espiegelberg. I don't think we should rename the whole project or anything else from UUID to GUID. UUID doesn't imply String, does it? We can imho still entertain the idea of relaxing the String type to Object without renaming a whole lot of things.

To make sure there is no major impact of this type change, I think I'd like to see a few tests for the procedures and REST APIs that use different types, such as long and int, and also tests for scenarios where the user intends to do the right thing but uses wrong types as arguments.

I would also like to see a test of what happens when the UUID generator actually does return an Object which Neo4j can't store as a property. Thinking about it, I believe we should do a check at startup time, generate a "test" UUID and verify that its type is something that can be stored as a Neo4j property.

Thanks!

― You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[bachmanm]

@Joshfindit what exactly do you mean by "both"?

[Joshfindit]

As in respond to 'uuid' with the uuid, and 'guid' with the various new options.

On Nov 27, 2016, at 3:06 PM, Michal Bachman notifications@github.com wrote:

@Joshfindit what exactly do you mean by "both"?

― You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[bachmanm]

Sorry @Joshfindit, still unclear what you mean. "Respond" to what?

@espiegelberg what in your view is the actual difference between UUID and GUID? I'm just trying to understand the motivation behind the renaming. Is the accepted answer here correct or not in your opinion? http://stackoverflow.com/questions/246930/is-there-any-difference-between-a-guid-and-a-uuid

Thanks!

[Joshfindit]

@bachmanm Apologies, I was pressed for time and hoped it was clear by context.

Currently, neo4j-uuid assigns UUID in the format xxxxxxxx-xxxx-Mxxx-Nxxx-xxxxxxxxxxxx. This makes sense, is clearly defined, and on-trend.

The proposed PR essentially moves the entire project to neo4j-guid which brings more functionality and power as well as being able to assign UUID, making it an upgrade, but it also diverges from the clarity, trend, and branding making it a reasonable hesitation if people are generally looking for "neo4j UUID" when they find this project (full disclosure: this is how I first found it as well).

So, my thought experiment is not technical, but branding-centric:

Does it help implement the GUID PR if the proposed solution continues to use the current code incom.graphaware.common.uuid.EaioUuidGenerator to generate a UUID, and com.graphaware.common.guid.EaioGuidGenerator uses the proposed GUID generator code?

[espiegelberg]

Thanks @Joshfindit for your support of my PR. Although I am the originator of this PR, I have come to have a divided mind on the topic.

In favor of the PR and, to answer @bachmanm question, I do think that UUID implies String, due to UUID’s being alphanumeric. With the recently introduced functionality to facilitate the creation of custom id generators, specifically such as the now included SequenceIdGenerator that generates an id that is ascending numerical value, generated ids may be in a format of than UUID. This leads me to think that the use of uuid in the project’s name is no longer apt. From the view of a community member who has zero commitment or investment in the neo4j-uuid “brand”, I would be in favor of a rebranding of the project to neo4j-guid.

On the other hand, I can appreciate that from the view of GraphAware — who owns the project and has undoubtedly invested significant time, energy, and possibly financial resources to the neo4j-uuid brand — the above rationale that the project name is no longer on point is undoubtedly fairly academic and there simply isn’t enough tangible ROI to make the change. Taking a step back, I also think the chances that project end users have widely adopted or dramatically shifted to make use of custom id generators rather than the default UUID generation is very low and so for the vast majority of the people familiar with the project the neo4j-uuid name still rings true.

So, despite being the initiator of this PR, I ultimately fall on the side of acquiescing with GraphAware, particularly since @bachmanm has weighed in with (reasonable) opposition against the idea of rebranding. Ultimately, if I were in his/their shoes, I’d reach the same conclusion.

I propose that this PR be closed but that, as a compromise, GraphAware keep this topic in mind should the project ever reach a significant milestone, such as version 2.0, where a rebranding may be more appropriate or timely.

[bachmanm]

agree