RE: [IndexedDB] New version API checked in

On Wednesday, September 21, 2011 7:11 PM, Jonas Sicking wrote:
> On Mon, Sep 12, 2011 at 1:56 PM, Israel Hilerio <israelh@microsoft.com>
> wrote:
> > On Sunday, September 04, 2011 3:33 AM, Jonas Sicking wrote:
> >> Hi Everyone,
> >>
> >> I finally got around to updating the IndexedDB spec to the new version
> API!
> >> Definitely a non-trivial change, so I'd love for people to have a
> >> look at it to see if I messed anything up.
> >>
> >> I decided to go with the name "upgradeneeded" for the event fired
> >> when a version upgrade is needed. I'm not terribly happy with the
> >> name, but it does feel descriptive.
> >>
> >> I also went with integers (long long) for the version number. The
> >> reason was that I wanted to avoid people passing strings like "1.10"
> >> to the API since JS will automatically and silently convert that to
> >> the number 1.1. This could lead to confusion since people might think
> >> that "1.10" is a higher version than "1.9".
> >>
> >>
> >> There were a few issues that came up during editing, mostly related
> >> to edge cases that doesn't matter terribly much:
> >>
> >> * What to do if multiple different versions are opened at the same time.
> >> Consider the following scenario. A database with name "mydb" has
> >> version 3. The following two calls happen almost at the same time:
> >>
> >> req1 = indexedDB.open("mydb", 4);
> >> and
> >> req2 = indexedDB.open("mydb", 5);
> >>
> >> It's clear that we should here fire a "upgradeneeded" event on req2
> >> and let it run a VERSION_CHANGE transaction to upgrade the database to
> version 5.
> >> There are however two possible things we could do to req1.
> >>
> >> A) Immediately fail by firing an "error" event on req1.
> >> B) Wait for req2 to attempt to upgrade the database version to 5.
> >> Only once that succeeds fail req1 by firing an "error" event at it.
> >> If req2 failed to upgrade the database (due to a aborted
> >> transaction), then fire a "upgradeneeded" transaction on req1.
> >>
> >> This seems like a really rare edge case and I don't think it matters
> >> much what we do. I chose to go with option B since it results in the
> >> least amount of errors and it doesn't seem particularly important to
> >> optimize for failing open calls quickly in this rare situation.
> >>
> >> I don't think it matters much what we choose here. I think it's very
> >> unlikely to matter in any real-world scenarios. I might even be fine
> >> with letting implementations choose to go with either solution here.
> >>
> >>
> >> * What to do if indexedDB.open is called while a VERSION_CHANGE
> >> transaction is pending, but the new call is for a higher version.
> >> Consider the following scenario:
> >>
> >> 1. A database with name "mydb" and version 1 is currently open in tab 1.
> >> 2. Someone calls indexedDB.open("mydb", 2) in tab 2.
> >> 3. The indexedDB implementation fires a "versionchange" event on the
> >> open connection in tab 1 and waits for it to close. The newVersion
> >> property of the event is set to 2.
> >> 4. Someone calls indexedDB.open("mydb", 3) in tab 3.
> >>
> >> At this point there are at least two options:
> >> A) Simply let the call in step 4 wait for the call in step 2 to
> >> finish. Only after it has finished will we fire new events to attempt
> >> an upgrade to version 3
> >> B) Stall the upgrade two version 2 and instead start attempting an
> >> upgrade to version 3. I.e. fire a new "versionchange" event on the
> >> open connection in tab 1 (now with newVersion set to 3), and once
> >> that connection is closed, fire a "upgradeneeded" event and start a
> VERSION_CHANGE transaction in tab 3.
> >>
> >> Option A basically makes us behave as if the call in step 4 happened
> >> after the VERSION_CHANGE transaction for the call in step 2 had
> >> started. Option B almost makes us behave as if the calls in step 2
> >> and step 4 had happened at the same time (with the exception that two
> >> "versionchange" events are fired).
> >>
> >> As with the previous issue I don't think it matters much what we
> >> choose here. I think it's very unlikely to matter in any real-world
> >> scenarios. I might even be fine with letting implementations choose
> >> to go with either solution here.
> >>
> >>
> >> * What to do if db.close() is called during the VERSION_CHANGE
> >> transaction Calling db.close() during a VERSION_CHANGE transaction
> >> somewhat similar to calling transaction.abort(). At least in the
> >> sense that in neither case does it make sense for
> >> IDBFactorySync.open/IDBFactory.open to complete successfully. I.e. it
> >> would seem strange to let IDBFactorySync.open return a closed
> >> database, or to fire a "success" event on the request returned by
> IDBFactory.open and then deliver a closed database.
> >>
> >> We could make db.close() throw an exception in this case, but that
> >> seems like a odd behavior for db.close() compared to how it usually
> >> interacts with running transactions (i.e. it usually lets them finish).
> >>
> >> I'm instead leaning towards letting the VERSION_CHANGE transaction
> >> continue running, but make IDBFactorySync.open throw an exception and
> >> fire an "error" event on the request returned from IDBFactory.open.
> >>
> >> In fact, after thinking about this some more I checked in a change to
> >> the spec to make it define that behavior. The main problem was that
> >> we don't have a really good error for this situation. I decided to
> >> return a ABORT_ERR error, but if anyone has other suggestions, or
> >> think we should use should use a dedicated error, I'm fine with that too.
> >>
> >> If people feel that we should use another solution, please do speak
> >> up. It's easy to back out my patch :-)
> >>
> >> / Jonas
> >
> > Thanks for making these changes to the spec so quickly.  Here is some
> feedback:
> >
> > * In order to support Section 4.1 step #4,  it seems we want to follow the
> same advance number logic and not allow any zero or negative numbers to
> be defined by the developer for the version.  This will allow this step to
> happen without any issues.  Otherwise, we will be creating a default db with
> version "0" and the value would be negative or "0" which would cause step
> #5 to execute.  It seems we want to throw an exception with a
> NON_TRANSIENT_ERR if the developer specifies any zero or negative version
> values (similar to advance).
> 
> Agreed. We can do that using the new [EnforceRange] WebIDL syntax. We
> should use that for cursor.advance too.

This makes sense.  I believe this will change the current signatures to:
*IDBFactory: IDBOpenDBRequest open (DOMString name, [EnforceRange] unsigned long long version);
*IDBCursor:  void       advance ([EnforceRange] unsigned long count) raises (IDBDatabaseException);

We will also throw an exception if the value for version or count equals 0.
Correct?

> 
> > * We also believe that the version number should be optional.  We see
> scenarios where the developer would want to use the latest db and not have
> to worry about the version number.  If a version number is not specified and
> the db doesn't exists, we will throw a NOT_FOUND_ERR.  We only way to
> create a new db when the version is specified.  This will prevent the
> onupgradeneeded event handler from being called when there is no version
> specified.
> 
> This was a tough one. After thinking about it for some time, and discussing
> with others, I agree that we need this. One use-case is for wrapping libraries
> which doesn't care about versions but instead lazily instantiates object stores
> and indexes as needed. One example is here:
> 
> http://nparashuram.com/IndexedDB/jquery/index.html
> 
> Such a library would want to open the database in its current version, but
> likely doesn't know what version the database has.
> 
> There is however a edge case that we need to deal with somehow. What do
> we do if someone calls .open and doesn't specify a version, but the database
> doesn't yet exist? I could see a few options:
> 
> 1. Fire the "success" event (and thus not start a VERSION_CHANGE
> transaction) and provide a database with version 0 which doesn't have any
> object stores.
> 2. Fire the "success" event (and thus not start a VERSION_CHANGE
> transaction) and set .result to null.
> 3. Fire a "error" event and set the error code of the request to some new
> error.
> 4. Fire a "upgradeneeded" event (and thus *do* start a VERSION_CHANGE
> transaction) and provide a database with version 1 which doesn't yet have
> any object stores.
> 
> Option 1 is somewhat useless since the page won't be able to do anything
> with such a database. However arguably option 2 and 3 isn't much more
> useful.
> 
> Option 4 scares me a little since it's somewhat inconsistent. I.e. in all other
> instances, not specifying a version never results in "upgradeneeded" being
> fired.
> 
> I'm leaning towards 2 or 4, but I'm very interested in hearing what others
> think. Or if I've missed some additional option.

We feel that developers should be specific when they want to create a database and therefore should provide a version.  Adopting this philosophy would allow us to determine developers didn't intend for a database to be created when there is no version.  It will also signal libraries that the database doesn't exists.  Therefore, option 4 doesn't seem ideal.

We believe that Option 2 could be misleading and will introduce inconsistencies on how the onsuccess handler is coded.  Devs would have to do additional checks inside their onsuccess handler (.result != null) to validate a successful db open.

Therefore, we would like to vote for option 3.

> 
> > * We don't believe developers shouldn't be allowed to start a new
> transaction from the onabort handler associated with a VERSION_CHANGE
> transaction.  The reason is that we don't consider the db to be in a valid state
> because the open operation associated with the db handle failed to execute
> properly.
> 
> Agreed. My intent was that step 8 in the steps for opening a database
> requires this (http://dvcs.w3.org/hg/IndexedDB/raw-
> file/tip/Overview.html#opening)
> 
> > * Similarly, if the VERSION_CHANGE transaction fails to commit (db failure
> or abort method is called) we believe the db version should be set to
> undefined on the onabort handler.  We don't see the point of going back to
> the server to get the last committed version number because the intent of
> the developer was to operate on the new schema which failed to commit.  It
> seems that if they want to use the latest schema they could open the db
> without specifying a version.
> 
> I don't really feel strongly here, but you'll already have the last committed
> version since that is what the "oldVersion" property on the "upgradeneeded"
> event contains this value.

By setting the current version to undefined in a db transaction failure all we're saying is that the current upgrade operation failed and the requested version wasn't set.  Devs can still go ahead and obtain the last good version of their database from the IDBVersionChangeEvent as you pointed out.

> 
> > * It seems that if a user aborts the VERSION_CHANGE transaction, we
> should call the onerror handler of the open database method.  This will
> ensure that the onsuccess logic that follows a clean transaction commit will
> not be executed, possibly generating additional errors.
> 
> Agreed. Both implicit and explicit aborts of the VERSION_CHANGE transaction
> should cause the "error" event to fire on the request returned from the
> .open call.
> 
> The second paragraph of the description of the IDBFactory.open call requires
> this. For the IDBFactorySync.open call an exception is thrown as defined by
> the first paragraph after the first Note.
> 
> / Jonas

Let us know what you think.

Israel

Received on Tuesday, 27 September 2011 21:42:31 UTC