leaguevine / leaguevine-ultistats

MIT License
18 stars 4 forks source link

Replace Backbone.Sync with WebSQL #6

Closed cboulay closed 12 years ago

cboulay commented 12 years ago

This is a big one. I will come back to it later. If anyone wants to start working on it now you may. I imagine the following steps. 1 - Use a Backbone + WebSQL tutorial (there are a few) to get started. 2 - Start replicating Leaguevine's data model in the local DB. 3 - Let the database module, but not the app's views/routers, communicate with the leaguevine API to keep their data in sync.

Obviously 3 is difficult to do. One take on this kind of problem can be found here: http://code.google.com/p/webstorageportabilitylayer/

mliu7 commented 12 years ago

I noticed in the code you started using a localStorage plugin. Were you still hoping to use WebSQL or is localStorage now the plan?

mliu7 commented 12 years ago

Also, are you sure you want to use WebSQL? It has been deprecated for over a year now, so that worries me a bit:

http://dev.w3.org/html5/webdatabase/

cboulay commented 12 years ago

Right, but no mobile browsers support the alternative. http://caniuse.com/indexeddb

The other alternative, localStorage, is possible and won't be deprecated but I prefer to use a relational model. From what I've seen of the Backbone localStorage solutions you have to load in an entire table before you can extract your instance, which seems rather inefficient. If you can find a localStorage solution that uses indexes for fast access then I would like to see that.

In either case, the app won't change much as long as we implement this by overwriting the Backbone.sync and pass the URL through to the ajax layer. The majority of the work will be setting up the interface between the data storage and the Leaguevine API. A lot of that work can be reused even if we have to change storage engines a year from now.

mliu7 commented 12 years ago

Thanks for the thoughts on localStorage. I think you're right about needing an index for quick lookups. If the user is entering a lot of events it could become unmanageable in localStorage after a while. I just did a bit of reading, and found this post as a pretty useful comparison:

http://stackoverflow.com/questions/7734371/developing-a-html5-offline-storage-solution-for-ios-android-in-2011

cboulay commented 12 years ago

I've been working on this. I'm almost done. The only thing left to do is having the queue of ajax requests wait to process request x+1 until x has returned successfully.

cboulay commented 12 years ago

This is done but I have to cleanup ultistats first before it'll work. Issue #66 addresses some of the problems.

Another problem is in authentication. Should we make it necessary for a user to authenticate at least once even if they plan on using the app entirely offline? I think that's reasonable. They need to be online to download the app in the first place, so clearly that device has to be online capable. I'm just worried about users who browse around the app but never get to a screen requiring authentication (and are thus never prompted for it), then they head out to the fields with their wifi-only ipad and suddenly they can't use anything because they've never logged in.

Maybe we can put a red exclamation mark over settings if they are not authenticated?

mliu7 commented 12 years ago

Ah, good point. I think we should just make people authenticate as soon as they visit the app. This way we avoid all these issues that you mention here.

A second issue that this would resolve is the issue of rate limiting. Right now the Leaguevine API does not rate limit read requests but this will change soon. The API will be rate limited by IP address. If an unauthenticated user is accessing the Leaguevine API over a 3G/4G connection, their IP address is shared with other users on the network, and this could cause their API limit to behave pretty unexpectedly if a bunch of other people in the same location are also accessing the API. The industry standard for overcoming this 3G/4G connection is to require authentication when accessing an API over this medium, so I think it's good if we begin doing this as well.

Obviously from the Leaguevine end we could just make an exception to the rate limiting for people using the Leaguevine-Ultistats app hosted at ultistats.leaguevine.com, but I don't think this is good because if people fork the app and want to host it themselves it won't work properly for them.

cboulay commented 12 years ago

(For my reference)

  1. type in search bar
  2. search.js -> new_collection.options.name & new_collection.fetch()
  3. backbone.js -> set callback & this.sync('read',options)
  4. websqlajax -> store.findAll(model) -> get all models from local db
  5. backbone.js -> result is parsed and add or rest onto the collection
  6. websqlajax -> then request is prequeued
    • TODO: process relation ids
    • local_id is stripped because it is useless/confusing to API
    • create success callback (see later)
  7. websqlajax -> request is queued and (TODO: ) if online it is transmitted
  8. websqlajax -> ajax success callback
    • Build an array from the result(s)
    • Go through each result and see if we have a match in our collection already (the newly generated search collection)
    • If no match then add it to our collection and to the local store.create (no callback)
    • Trigger "reset"

There are a couple todos up there, also the event triggering on the ajax callback is a bit awkward.

Currently I'm getting an SQL error when trying to insert the search fetch results into the db because the same results already exist in the db. I'm not sure why.

cboulay commented 12 years ago

Can you take a look at the "titlebar" branch? The WebSQL Ajax plugin is mostly working. One important exception are the model's that don't have an id by themselves. I didn't plan for that, and fixing that will be rather difficult. Eventually I think we can make a custom table for each model and use foreign keys etc but that's a bit far in the future.

If you think the branch is good enough, then please go ahead and merge it. Note that I had to disable the cache manifest because I use node.js as my server and it isn't configured for serving the manifest correctly.

mliu7 commented 12 years ago

Awesome, good work on all your progress. Roger and I are leaving for the weekend for a friend's wedding so I'm going to wait until after this weekend to merge the titlebar branch so I don't mess things up when I'm not here to fix them.

I'll be sure to fix that cache manifest problem as well when I return.

cboulay commented 12 years ago

I'm going to close this in favor of separate events for the remaining changes.