ohmage / android

This is the repository for the ohmage Android application.
Apache License 2.0
7 stars 10 forks source link

synch to /people/<user_id>/current does not handle users being removed from ohmlets #83

Closed jshslsky closed 10 years ago

jshslsky commented 10 years ago

The synch needs to update local state to account for ohmlets that the user may no longer be a part of.

cketcham commented 10 years ago

So this is an interesting bug.. If you leave an ohmlet from anything other than the phone, the first thing the client will do on a sync is join the user to the ohmlet again. The problem is I try to join the user to any ohmlets which are set on the phone but not the server. This works when a user joins an ohmlet on their phone since that state will exist only on their phone and not on the server.

I think this is indicative of a larger problem. Here is an example of the problem: If a user joins an ohmlet on their phone, and immediately joins the same ohmlet on the web, and then leaves the ohmlet on their phone or on the web, the next time the sync ran on the phone, it would synchronize the phones state. So if the user left the ohmlet on their phone, even if the last thing they did was join using the web, they would be removed from the ohmlet.

I think we need a way to know when an action happened and to only apply it if nothing happened after it so we always synchronize to the latest thing that happened.

jshslsky commented 10 years ago

So if the user left the ohmlet on their phone, even if the last thing they did was join using the web, they would be removed from the ohmlet.

Won't the synch with the server include the ohmlet they joined on the web?

cketcham commented 10 years ago

Actually I messed up when I wrote that case. The real case that is a problem is if the user joins on the phone, and before that syncs, they join and leave on the web. If the last thing they did was leave from the web, the phone won't know that it's state is old, so it will join the user to the ohmlet. Since it will see the user is part of the ohmlet on the phone, but not on the server. It won't see that they were part of the ohmlet and left.

jshslsky commented 10 years ago

OK, so it's a kind of race condition. When they join from the phone, it is via the join link right?

cketcham commented 10 years ago

yeah exactly, it's not too difficult to do because the phone could take a few minutes before it synchronizes, but I guess this kind of behavior isn't likely.

The issue I'm having now is if a user is removed from an ohmlet via the web, the phone still can't tell the difference between having the ohmlet added on the phone, or having the ohmlet left over because the user used to be in it. So when the phone syncs the first thing it will do is add the user back to the ohmlet. I'm pretty sure this is what you experienced when you put in this issue.

I could fix it on the phone by including some state so I know that I modified it so it should sync from the phone to the server, or not so it should accept the state of the server, but this doesn't cover the "race condition" case we have been talking about.

jshslsky commented 10 years ago

It's not likely, but it's also a classic "multiple device" problem that would be good to fix. It will keep coming up for us as we test as well.

In the new front-end, is the same "intercept and join" behavior that occurs on the phone present?

cketcham commented 10 years ago

No, I didn't implement the ability to join existing ohmlets, streams, surveys.

jshslsky commented 10 years ago

OK, so then today this issue can only occur manually with EasyPost. I will try and recreate it.

At some point the web front end should definitely be updated to handle the join process.

Going all the way back up to the original post upthread, it seems like the fix for this is to keep track of when a user joined or was added to an ohmlet as well as when they were removed. If a user was removed and a client attempts to sync, the server can return a 409 Conflict response and the client can just remove the ohmlet reference from its internal cache.

cketcham commented 10 years ago

That still doesn't catch the case where the user intentionally joined the ohmlet on the phone again. If a user was removed using easypost and then they do something like remove themselves using the phone (before it syncs) and then immediately join again. The last thing they did would be to join the ohmlet on the phone, but the server would return a 409 and they wouldn't be added back.

jshslsky commented 10 years ago

Instead of implementing a really complex solution, here's what we have decided to do here.

The phone will store the state that a user clicked an ohmlet's join link. When a successful synchronization of the ohmlet happens, the phone will remove the "join" state. On a future synchronization, if an ohmlet is removed on the server and the join state does not exist for the ohmlet, the ohmlet will be removed from the phone.