search

stellasia / neomap

A Neo4j Desktop application to visualize nodes with geographic attributes on a map.
GNU General Public License v3.0
106 stars 13 forks source link

Singleton neo4j #79

Closed okjodom closed 3 years ago

okjodom commented 4 years ago

A singleton neo4jService that encapsulates driver. Layers don't need to have a knowledge of underlying neo4j driver. Composition proposed here allows us to smoothly swap out drivers in future #65, and remove driver instantiation on critical boot path #78. Additionally, this PR formalizes a result API on neo4jService endpoints. Success from service calls return as { status: 200, result: someResult } while failures return { status: 500, error: errorWithStack }

Next: This pattern opens up an opportunity for better error ux, most immediately, a path for solving #77 [ don't rerender if we hit error on create/update new layer ]

PS: Builds on top of changes in PR #75

okjodom commented 4 years ago

cc @wagnerjt for reviews on service changes

okjodom commented 4 years ago

Not ready until I add tests for the driver... :)

okjodom commented 4 years ago

Not ready until I add tests for the driver... :)

done. @stellasia, @wagnerjt I'd love your reviews on pr

okjodom commented 4 years ago

A singleton neo4jService that encapsulates driver. Layers don't need to have a knowledge of underlying neo4j driver. Composition proposed here allows us to smoothly swap out drivers in future #65, and remove driver instantiation on critical boot path #78. Additionally, this PR formalizes a result API on neo4jService endpoints. Success from service calls return as { status: 200, result: someResult } while failures return { status: 500, error: errorWithStack }

Next: This pattern opens up an opportunity for better error ux, most immediately, a path for solving #77 [ don't rerender if we hit error on create/update new layer ]

PS: Builds on top of changes in PR #75

@stellasia, let's review and close this PR when possible.

stellasia commented 4 years ago

Sure, will do during the coming week!

stellasia commented 3 years ago

Thank you very much for this @jodom , merging :100: