Re: [IndexedDB] Implicit transactions

On Thu, Aug 5, 2010 at 8:56 PM, Jonas Sicking <jonas@sicking.cc> wrote:

> Ok, I'm going to start by taking a step back here.
>
> There is no such thing as implicit transactions.
>
> db.objectStore("foo", mode)
>
> is just syntactic sugar for
>
> db.transaction(["foo"], mode).objectStore("foo")
>
> so it always starts a new transaction. I think for now, lets take
> db.objectStore(..) out of the discussion and focus on how we want
> db.transaction() to work. In the end we may or may not want to keep
> db.objectStore() if it causes too much confusion.
>
> One thing that we have to first realize is that every IDBObjectStore
> instance is tied to a specific transaction. This is required to avoid
> ambiguity in what transaction a request is made against. Consider the
> following code
>
> trans1 = db.transaction(["foo", "bar"], READ_WRITE);
> trans2 = db.transaction(["foo", "students"], READ_ONLY);
> os1 = trans1.objectStore("foo");
> os2 = trans2.objectStore("foo");
> alert(os1 === os2);
> os1.get("someKey").onsuccess = ...;
>
> In this code, the alert will always display "false". The os1 and os2
> are two distinct objects. They have to be, otherwise we wouldn't know
> which transaction to place the get() request against.
>
> Once a transaction has been committed or aborted, using any of the
> IDBObjectStore objects connected with it will throw an error. So the
> example mentioned earlier in the thread (i'll use different syntax
> than used previously in the thread):
>
> var gMyos = null;
> function fun1() {
>  gMyos = db.transaction(["foo"]).objectStore("foo");
>  gMyos.get("someKey").onsuccess = ...;
> }
> function fun2() {
>  gMyos.get("someOtherKey");
> }
>
> If we return to the main even loop between calling fun1 and fun2, the
> .get() call in fun2 will *always* throw. IMHO it's a good thing that
> this consistently throws. Consider also
>
> function fun3() {
>  var trans = db.transaction(["foo", "bar"], READ_WRITE);
>  trans.objectStore("bar").openCursor(...).onsuccess = ...;
> }
>
> It would IMHO be a bad thing if calling fun3 right before calling fun2
> all of a sudden made fun2 not throw and instead place a request
> against the transaction created in fun3.
>
> While I definitely think it can be confusing that there are several
> IDBObjectStore instances referring to the same underlying objectStore,
> I think this is ultimately a good thing as it reduces the risk of
> accidentally placing a request against the wrong transaction. It means
> that in order to place a request against a transaction, you must
> either have a reference to that transaction, or a reference to an
> objectStore retrieved from that transaction.
>
> Another way to think of it is this. You generally don't place requests
> against an objectStore or index. You place them against a transaction.
> By tying IDBObjectStores to a given transaction, it's always explicit
> which transaction you are using.
>
> On Thu, Aug 5, 2010 at 3:04 AM, Jeremy Orlow <jorlow@chromium.org> wrote:
> > On Wed, Aug 4, 2010 at 7:47 PM, Shawn Wilsher <sdwilsh@mozilla.com>
> wrote:
> >>
> >>  On 8/4/2010 10:53 AM, Jeremy Orlow wrote:
> >>>
> >>>
> >>> Whoa....transaction() is synchronous?!?  Ok, so I guess the entire
> >>> premise
> >>> of my question was super confused.  :-)
> >>>
> >> It is certainly spec'd that way [1].  The locks do not get acquired
> until
> >> the first actual bit of work is done though.
> >
> > I fully understand how the trick works.  I just didn't comprehend the
> fact
> > that the Mozilla proposal (what's now in the spec) was removing any way
> to
> > get into an IDBTransactionEvent handler besides doing an initial data
> > access.  I wouldn't have agreed to the proposal had I realized this.
> > Lets say I had the following bit of initialization code in my program:
> > var myDB = ...
> > var myObjectStore = myDB.objectStore("someObjectStore");
> > var myIndex = myObjectStore.index("someIndex");
> > var anotherObjectStore = myDB.objectStore("anotherObjectStore");
>
> As described above, grabbing references like this is not what you want
> to do. If we were to allow this I think we would run a severe risk of
> making it very hard to understand which transaction you are placing
> requests against.
>
> > And then I wanted to start a transaction that'd access some key and then
> > presumably do some other work.  As currently specced, here's what I'd
> need
> > to do:
> >
> myDB.transaction().objectStore("someObjectStore").index("someIndex").get("someKey").onsuccess(function()
> > {
> >     anotherObjectStore.get("someOtherKey").onsuccess(...);
> > });
> > vs doing something like this:
> > myDB.asyncTransaction().onsuccess(function() {
> >     myIndex.get("someKey").onsuccess(function() {
> >         anotherObjectStore.get("someOtherKey").onsuccess(...);
> >     });
> > });
> > With the former, we actually have more typing and the code is harder to
> > read.  Sure, when I'm writing short code snipits, the synchronous form
> can
> > be more convenient and readable, but forcing this upon every situation is
> > going to be a hinderance.
> > Please, lets add back in a transaction method that returns an IDBRequest.
>
> The current design of the spec is that which IDBObjectStore you place
> a request against determines which transaction you are using.
>
> It sounds to me like you want a design where each IDBObjectStore
> instance represents just a objectStore name, and depending on *when*
> you place a request against it determines which transaction is used.
>
> To me the latter is a much more subtle and risky. Especially if we
> *also* want to have the ability to synchronously start transactions.
> Which transaction would the following code snippets use for the .get()
> request?
>
> Example 1:
> trans1 = db.transaction(["foo", "bar"], READ_WRITE);
> trans2 = db.transaction(["foo", "fish"]);
> myStashedFooObjectStoreReference.get("someKey").onsuccess = ...;
>
> Example 2:
> trans = db.transaction(["foo"], READ_WRITE);
> var fooStore = trans.objectStore("foo");
> fooStore.add("someKey", someValue).onsuccess = function(e) {
>  trans2 = db.transaction(["foo", "bar"]);
>  fooStore.get("someOtherKey");
>  fooStore = trans2.objectStore("foo");
>  fooStore.get("aThirdKey");
> }
>
> Basically if we want to make *when* you place a request determine
> which transaction is used, rather than *which* IDBObjectStore you use,
> then we can't allow transactions to be synchronously created. That
> reason alone is enough to make me think that we should stick with the
> current model in the spec.
>
> Here is what your example above would look like:
> trans = myDB.transaction(...);
>
> trans.objectStore("someObjectStore").index("someIndex").get("someKey").onsuccess
> = ...;
> trans.objectStore("anotherObjectStore").get("someOtherKey").onsuccess =
> ...;
>
> Which I think is simpler than either of your examples. Yes, it's a bit
> more typing, but it's much less asynchronous nesting which is IMHO a
> bigger advantage.
>

Ok, I think I now understand how your transaction model was intended to
work.  It scares me some that neither I nor Andrei understood the model.
 I'm curious to know whether it was just us or Pablo and Nikunj were also
confused by it.

Personally, I think the model of ObjectStores, Indexes, etc not being tied
to just one transaction and instead using knowledge of which
IDBTransactionEvent is currently firing to know which transaction an action
is tied to is more elegant.  But I agree such a model is kind of at odds
with your model and that just doing one or the other is probably for the
best.  I worry that your model will only increase the pressure for people to
wrap IndexedDB for syntactic sugar.  Yet I also think more thrashing of the
spec is a bad idea and clearly you guys feel pretty strongly about this.  So
I guess we should continue down our current path.

I'll file a bug for clarification of what's currently there.

J