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

Split gdax-java into library and client #56

Open
irufus opened this issue Nov 14, 2018 · 14 comments
Open

Split gdax-java into library and client #56

irufus opened this issue Nov 14, 2018 · 14 comments
Assignees
Labels

Comments

@irufus
Copy link
Owner

irufus commented Nov 14, 2018

There are three separate projects within this repo.

  • The library which facilitates connecting to the Coinbase Pro
  • The spring boot application the injects the configuration for each rest api call
  • The order book application that loads market data into a viewable interface
@irufus irufus self-assigned this Nov 14, 2018
@irufus irufus added the up-next label Nov 14, 2018
@robevansuk
Copy link
Collaborator

Attempting to break this apart this evening into a multi project build - will publish any commits here. Might not get all the way through but certainly feasible to break it apart.

@robevansuk
Copy link
Collaborator

robevansuk commented Oct 30, 2019

@stokito
Copy link
Contributor

stokito commented Apr 13, 2020

It also would be nice to split feed API. For example I need only it in my project

@robevansuk
Copy link
Collaborator

robevansuk commented Apr 20, 2020

I have a working branch here: https://github.com/irufus/gdax-java/tree/%2356_Split_gdax-java_into_library_and_client for splitting the GUI and API apart. I haven't looked at it in a while but I think (if I remember correctly) that it removes some spring boot dependency from the lib part of the codebase and puts it on the desktop side of things. It may be useful to try get this merged as a major version some day soon. I probably won't get around to it any time soon but worth a look.

@stokito
Copy link
Contributor

stokito commented Apr 20, 2020

please take alook on my PR #60 which also have some dependencies cleanup

@robevansuk
Copy link
Collaborator

reviewed - just one item I can suggest. Good to see some library bumps

@irufus
Copy link
Owner Author

irufus commented Apr 24, 2020

I will etch out some time this weekend to review

@irufus
Copy link
Owner Author

irufus commented May 25, 2020

@robevansuk Does @stokito 's changes address similar things you were fixing in that branch?

@robevansuk
Copy link
Collaborator

I have a huge commit but somethings broken on the live orderbook which I've been looking at for the past week or two... I haven't managed to fix it yet but am working on this branch #62

@robevansuk
Copy link
Collaborator

Once this all works, we can then look at "publish to maven" plugins for the module that contains the library code

@robevansuk
Copy link
Collaborator

robevansuk commented Jun 3, 2020

I found the problems and am in the process of fixing them. in a nut shell, the new JSON parsing doesn't deserialise the type field for websocket feed messages. This breaks the live orderbook which depends on this information. I'll repair the deserialisation code and test it again. If all works this can be merged and more incremental changes used to update the repo.

@stokito
Copy link
Contributor

stokito commented Jun 3, 2020

Yes, Jackson by default don't populate the type property. The logic still works because then we'll use class info itself and we don't need the type. If you want the type to be populated then we can add a flag into the Jackson annotation. But IMHO it would be better just to remove the field for two reasons:

  1. Performance consideration: the string field takes quite a lot of memory and it's deserialization wastes resources.
  2. Code stability: To avoid misusing of the field e.g. in if msg.type == 'MATCH', which can't be checked by compiler and force developed to use OOP i.e. if msg instance of MessageMatch

@robevansuk
Copy link
Collaborator

I've added changes to populate it but in all honesty my exposure to jackson isn't that great. It works for now but I need to look at the live orderbook before I can merge my changes. Once its working I'll merge it and look at simplifying afterwards

robevansuk added a commit that referenced this issue Jul 27, 2020
* Initial steps to break client / desktop app out into separate modules and create a separate spring boot implementation of the client lib

* update websocket and REST urls in application-*.yml files to point to new pro.coinbase.com urls

* re-add application-*.yml to gitignore to prevent anyone committing their private API keys/secrets by mistake

* rename tests that need to run against the live sandbox as "IntegrationTest" so they can be run only when we actually want to test the codebase against a live implementation of the exchange. Also updated the gradle script to use submodules

* move config to spring boot application module

* Some rearrangement, remove RestTemplate as the spring context/container provides this anyway, and some renaming of GDAX -> CoinbasePro

* decoupling orderbook and websocketfeed by making use of the addMessageHandler method

* Begin decoupling spring from the coinbase pro library and using it only in the desktop client

* Remove spring from api integration project.
Remove spring boot application plugin from the api project
Some renaming in a vain attempt to fix the multiproject build issues

* some documentation updates

* update the documentation and the coinbase exchange class name

* update the banner

* update config to use exchange as the config var name

* re-ignore application*.yml files

* use correct .yml file extension in references

* Successful separation of gui from library. Updated the gui to integrate with the latest websocket feed implementation - now using "full" channel. Alsoo updates to the README (some amendments still necessary) and more updates to pull apart the 'library' from the desktop GUI

* adding desktop gui application.yml file for configuring the application for your trading account

* clean up

* re-jig to have a gradle multiproject build that has the api as a proper library and the gui and websocket feed as individual modules

* remove redundant imports

* remove redundant gson lib

* minor improvements suggested by IntelliJ

* amend confusing javadoc message

* attempting to merge all changes together following some merge mayhem

* refactoring the live orderbook in the Gui and the tests of OrderBookModelTest to be more concise, less repetitive. A lot of tidying up assertions. Updated CONTRIBUTING.md (minor). Getting the gradle multiproject build working. Lots of minor fixes

* Large refactor. Need to fix orderbook as its currently broken

* fix compilation issues with market data tests

* refactor a order message insertion based on sequence id

* extract constants so they can be used in tests and live orderbook code

* fix json deserialising to fix missing message `type` information which has broken the live orderbook.

* Updates to ensure parsing from JSON sets the `type` for L2UpdateMessages, ErrorMessages, SnapshotMessage

* Fix TickerMessage definition and addition of Test to ensure its parsed correctly from Json

* Fix for json parsing to set the message type. This should fix the live orderbook

* Fix datatype for product ID

* removing some redundant code,editing comments,renaming variables. A bit of clarity added. Test also added to test a live scenario which appeared to be failing.

* Removed CompletableFuture for the websocket. The websocket connection needs to be sequential for the orderbookto work properly. Update code to remove deprecated value.

* introduce sleep after websocket connects to allow enough of a queue to form for the GUI that we can reliably reconstruct a live orderbook on top of the full orderbook snapshot from the MarketData endpoint.

* price entries were appearing twice when we had orders with prices like 3500.23 and 3500.23000. The comparison wasn't equivalent. setting the scale on the OrderBookMessage should fix this.

* rename orderbookview to orderbookpresentation as it will host the presentation layer only

* Remove Gui ready for a rebuild

* revert removal of websocket feed and remove all references to previously implemented GUI. Also introduced a security package so that the Signature class can be shared between the websocketfeed module and the api module. CONTRIBUTING.md also updated

* add build.gradle for new security package

* updated README with latest changes

Co-authored-by: robevans <[email protected]>
@robevansuk
Copy link
Collaborator

The GUI now removed. I'll work on publishing the libs to a maven repo next so that the lib can be used more freely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants
@irufus @stokito @robevansuk and others