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

Implement thaliManager #507

yaronyg opened this issue Feb 3, 2016 · 3 comments

Implement thaliManager #507

yaronyg opened this issue Feb 3, 2016 · 3 comments


Copy link

@yaronyg yaronyg commented Feb 3, 2016

  • Clone the vNext_yarong_417_1 branch then 'git checkout -B vNext_andrew-aladev_507'
  • Implement #773, you'll need it
  • Update comment on PouchDB argument to ThaliManager to point at the new utility function from #773 and let the user know that unless they have a good reason not to, they need to use it to wrap the version of PouchDB they are using.
  • Change the code (and the comments) so that if the user doesn't submit their own thailPeerPoolInterface we fail.
  • In the constructor make sure that we set up salti as the first router configured on our express router object so we know all requests will go through it first. Then set up the expressPouchDB object that was passed in for the BASE_DB_PATH with 'minimumForPouchDB'. The second step is already in the code.
  • The constructor also needs to setup thaliSendNotificationBasedOnReplication and thaliPullReplicationFromNotification. Both are already in the constructor. thaliPullReplicationFromNotification will handle setting up thaliNotificationClient. And thaliSendNotificationBasedOnReplication will handle setting up thaliNotificationServer. That is why we only need the two top level replication classes. They handle everything else.
  • The start function has a bunch of jobs to do. First it will start up pull replication from notification. This doesn't do anything because the underlying network isn't started yet but we don't want a race condition so we start it first. Then we can start the network layer by calling ThaliMobile.start followed by starting listening for Advertisements (e.g. people announcing themselves) and startUpdateAdvertisingAndListening (where we advertise ourselves) finally followed by starting thaliSendNotificationBasedOnReplication which depends on things like advertising ourselves being turned on (since it's whole purpose is to update stuff like that).
  • Stop is easier because we just shut down the two replication objects and we have a single stop that shuts everything down for ThaliMobile.
  • We can do a local test with a bogus PSK request to make sure Salti is on the job.
  • Do a test with a legal PSK request (e.g. a PSK from an actual authorized peer) but for a URL they shouldn't have access to just to make sure we are using the right config of salti.
  • If you feel like it you can use proxyquire to mock out the underlying objects and write unit tests to make sure we call the right objects for start and stop but I'm not sure that is really worth doing.
  • For this class the much more interesting tests are the coordinated ones. The first test is the repeat write test. In this test during setup we will share our public key via the data option (see line 29 of testThaliPullReplicationFromNotificationCoordinated). In the test itself create a local DB with some well known name and write to a record of the form { _id: (our public key), test1: true }. Then register for changes on the DB and make sure to include the doc. Then call start on the thaliManager and pass in the public keys of all the other peers using the testUtils.turnParticipantsIntoBufferArray (line 120 of testThaliPullReplicationFromNotificationCoordinated). What you are looking for is that you see changes with docs with the IDs of all your peers with test1: true. Once that happens then you start the second stage of the test. In the second stage you will update your local record in the DB with { _id: (out public key), test1: true, test2: true }. Now create a new changes tracker (make sure to cancel the previous one!!!) and this one is looking for records of all the peers again (make sure to set your new changes tracker with the PouchDB option "since: 0") but you are looking to see test2: true. Once you see that then you are done with the test and can exit. In teardown make sure we call stop.
  • For the next test you are actually going to continue the previous test. You MUST use the exact same thaliManager object you used from the first test. That is the whole point of this test. You are going to use the same DB and you are going to update our favorite record with { _id: (out public key), test1: true, test2: true, test3: true }. The rest of the test looks like the first one except you are just waiting to find a record for each of the other peers with test3: true. What this test shows is that we can stop the whole system (which happened in teardown of the first test) and then restart the whole system and everything still works with the exactly same thaliManager object. This shows that the object properly cleans itself up and restarts itself.
  • Make sure both of the coordinated tests have a timer that will kill the test if it runs too long.

Note that the previous two tests are just variants on the testThaliPullReplicationFromNotificationCoordinated. There are lots of subtleties I don't really go into above such as how the changes tracker works, about including docs, live changes tracking, since 0 (which I at least mention), etc. All of that is in the testThaliPullReplicationFromNotificationCoordinated test.

@yaronyg yaronyg added this to the New Infra milestone Feb 3, 2016
@yaronyg yaronyg added 0 - Icebox and removed 0 - Icebox labels Feb 3, 2016
@yaronyg yaronyg self-assigned this Mar 22, 2016
@yaronyg yaronyg added 2 - Ready and removed 1 - Backlog labels Mar 22, 2016
@yaronyg yaronyg added 4 - Done and removed 2 - Ready labels Jun 24, 2016
@yaronyg yaronyg changed the title Implement thaliReplicationManager Implement thaliManager Jul 12, 2016
@yaronyg yaronyg assigned andrew-aladev and unassigned yaronyg Jul 15, 2016
@yaronyg yaronyg added 2 - Ready and removed 3 - Working labels Jul 20, 2016
@yaronyg yaronyg added 3 - Working and removed 2 - Ready labels Jul 25, 2016
Copy link
Member Author

@yaronyg yaronyg commented Jul 29, 2016

Code should be done and testing is at 40% complete.

Copy link
Member Author

@yaronyg yaronyg commented Aug 3, 2016

I should have a PR for this tomorrow.

Copy link
Member Author

@yaronyg yaronyg commented Aug 8, 2016

30 minutes until the pr

@yaronyg yaronyg added 4 - Done and removed 3 - Working labels Aug 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.