RocketChat / Rocket.Chat.Java.SDK

[DEPRECATED, NOT MAINTAINED] Java/Android SDK for Rocket.Chat
MIT License
29 stars 23 forks source link

[WIP] Move to OkHttp and add REST calls #26

Closed luciofm closed 7 years ago

luciofm commented 7 years ago

Work in Progress

Moving the SDK to OkHttp. Since we are going to use OkHttp for the REST calls, and we will use it on the App, doesn't make sense to have 2 websocket stacks consuming resources (size and methods). Adding REST calls for various calls. On the new App, we will start to use REST calls on many methods, including login, list channels, list channel members, send messages, pin/unpin messages, star/unstar messages and more.

Use mockwebserver (from OkHttp) for testing the SDK

sacOO7 commented 7 years ago

Hey I'm not quite sure, how reliable OkHTTP websockets are. As far as REST is concerned, there are other libraries available which provides pure REST API's. I tested OkHTTP websockets and it gives problem for maintaining connection with server. For now lets do one thing, keep both libraries separate and test them on various platforms. I will create a separate release for SDK based on OkHTTP for native app, keep your branch sync with develop branch.

sacOO7 commented 7 years ago

I will test the current implementation for 2-3 days. I will see if it works great on various devices 👍 . Then I can merge it with develop. Till then try pushing your updates to OkHTTP branch.

luciofm commented 7 years ago

This is still a working in progress, far from merge state yet.

sacOO7 commented 7 years ago

@luciofm still great work 👍

sacOO7 commented 7 years ago

Please do not change lots of method names. Users will need to alter their code if new artifact is published with changed method names. Try to update docs in sync with updated names.

luciofm commented 7 years ago

@sacOO7 now is the time to break things... once we got a stable API and release 1.0, then we cannot break compatibility with previous versions...

sacOO7 commented 7 years ago

ohk, I think I need to test the API on your branch fully. Lot of code has been changed from last time.

sacOO7 commented 7 years ago

Hey please do not remove APIs for realtime, lets make them deprecated. Let's add a builder class for RocketChatAPI which can build both REST and realtime client (same as OkHTTP). I think it's better to separate both from each other. It's easy to manage that way and makes code more modular. I truly believe it is a better approach.

sacOO7 commented 7 years ago

Give it a new name like RocketChatRestClient. All rest APIs can reside in there. Maybe you can rename RocketChatAPI to RocketChatRealtimeClient (two different classes). That way code is more easy to handle and understand 👍 . Great work 💯

sacOO7 commented 7 years ago

Hey @luciofm please add your suggestions here

sacOO7 commented 7 years ago

Hey any thought on above suggestions? Please let me know about it. We can have a discussion on this.

sacOO7 commented 7 years ago

I will add here points that will cover tasks that need to be covered before PR is merged. I can see lot of code been changed. I think it is ok to say that every functionality needs to be covered that was available before changing the code. E.g. Attachment types, DbManager, ConnectivityManager, reconnection, manual and auto reconnection etc

sacOO7 commented 7 years ago

I will add some code for LiveChat SDK that will cover changes to subscriptions middleware similar to core middleware. I will also provide interface implementation for DbManager, so that it is configurable to store on disk by extending interface.

sacOO7 commented 7 years ago

TODO

  1. Working Attachment Models
  2. Interface implementation of DbManager (I think it should be present in websocketimpl since data will be stored from subscriptions which comes under realtime API)
  3. ConnectivityManager, reconnection, manual and auto reconnection are already available, just need accessor methods.
  4. All APIs to be well tested since models are available from new library, program may crash in runtime due to absense of particular key.
  5. Discussion related to abstract class callback, since I cannot implement it, I cannot have callback defined as a class member. If it is a interface, it introduces reusability by passing instance of a class as a callback inside holder.
  6. Make sure all models are correct and getting filled with all data that is being received from server.
  7. Merge ChatFactory and Improvements inside this PR and test each functionality. Refactor the code i.e. remove unnecessary code. **8. Advanced functionality : Add Task schedular to run certain tasks after reconnection i.e. subscriptions to rooms, authenticating again etc. Tasks can be assigned manually. Will be executed after each reconnection.
luciofm commented 7 years ago

Merging to development. Still lots of stuff to break before 1.0