RE: [indexeddb] openCursor optional parameters issue

On Tuesday, June 28, 2011 7:31 PM, Jonas Sicking wrote:
> On Tue, Jun 28, 2011 at 4:59 PM, Israel Hilerio <israelh@microsoft.com>
> wrote:
> > On Tuesday, June 28, 2011 12:49 PM, Jonas Sicking wrote:
> >> On Tue, Jun 28, 2011 at 10:53 AM, Israel Hilerio
> >> <israelh@microsoft.com>
> >> wrote:
> >> > On Monday, June 27, 2011 8:21 PM, Jonas Sicking wrote:
> >> >> On Mon, Jun 27, 2011 at 11:42 AM, Israel Hilerio
> >> >> <israelh@microsoft.com>
> >> >> wrote:
> >> >> > The IDBObjectStore.openCursor method is defined to have two
> >> >> > optional
> >> >> parameters:
> >> >> > * IDBRequest openCursor (in optional any range, in optional
> >> >> > unsigned short direction) raises (IDBDatabaseException);
> >> >> >
> >> >> > Based on the examples in the spec, it seems we're envisioning
> >> >> > the method
> >> >> to be used in the following ways:
> >> >> > * objStore.openCursor();
> >> >> > * objStore.openCursor(keyRange);
> >> >> > * objStore.openCursor(keyRange, IDBCursor.PREV);
> >> >> > * objStore.openCursor(IDBCursor.PREV);
> >> >>
> >> >> No, that's not how optional parameters work in WebIDL. In order to
> >> >> specify an optional parameter, you always have to specify all
> >> >> preceding optional parameters. So only the following syntaxes are
> >> >> valid:
> >> >>
> >> >> * objStore.openCursor();
> >> >> * objStore.openCursor(keyRange);
> >> >> * objStore.openCursor(keyRange, IDBCursor.PREV);
> >> >>
> >> >> > Having "any" for the keyRange type makes it difficult to detect
> >> >> > the correct
> >> >> overloaded parameter for openCursor.
> >> >>
> >> >> The reason the first parameter is of type 'any' is so that you can
> >> >> pass either a IDBKeyRange or a value. So for example:
> >> >>
> >> >> req = objStore.openCursor("hello"); req = index.openCursor(4);
> >> >>
> >> >> are valid. When called with a simple value on an object store the
> >> >> cursor will obviously always return 0 or 1 rows. For indexes it
> >> >> could return any number of rows though.
> >> >>
> >> >> This is actually already specified if you look at the steps for
> >> >> opening a
> >> cursor.
> >> >> The same holds true for many other functions, such as .get and .delete.
> >> >>
> >> >> However it's a very subtle feature that's easy to miss. If you
> >> >> have suggestions for how to make this more clear in the spec I'd
> >> >> love to hear them. I've been thinking that we should add
> >> >> non-normative, easy-to-understand text to explain each function,
> >> >> similar to what the
> >> >> HTML5 spec does when defining APIs.
> >> >>
> >> >> / Jonas
> >> >
> >> > What you're saying makes a lot of sense.  That was what I
> >> > originally thought
> >> but what confused me was some of the examples in the current spec
> >> which suggest we want to do the following (Section 3.3.5):
> >> > * objStore.openCursor(IDBCursor.PREV);
> >>
> >> I don't think we should allow this. The benefit of saving the author
> >> from writing objStore.openCursor(nulll, IDBCursor.PREV) isn't worth
> >> the complexity that is introduced. IMHO. We should just fix the example
> instead.
> >>
> >> > Independent of how up to date the examples are, the issue with the
> >> > way it is
> >> currently spec'ed is that there is an implied dependency between
> >> keyRange and Cursor direction.  In other words, you can't open a
> >> cursor without any keyRange and just a direction.  One possible way
> >> to resolve this is to allow the keyRange to be nullable.  This will
> >> allow us to define a cursor without a keyRange and with a direction:
> >> > * objStore.openCursor(null, IDBCursor.PREV);
> >> >
> >> > Without something like this, it is not easy to get a list of all
> >> > the records on
> >> the store going in the opposite direction from IDBCursor.NEXT.
> >>
> >> Indeed, it was the intent that this should be allowed. I suspect we
> >> simply haven't kept up to date with WebIDL changing under us. But I
> >> do think that the text in the algorithm does say to do the right
> >> thing when no keyrange (or key
> >> value) is supplied.
> >>
> >> / Jonas
> >
> > My concern is not having a clean mechanism to retrieve a regular cursor
> with an inverted order without knowing any records (first or last) in the
> list.  This seems like a common operation that is not supported today.
> >
> > These are some of the alternatives that I believe we have:
> > * Support a null value for IDBKeyRange:
> >        -IDBRequest objStore.openCursor(null, IDBCursor.PREV);
> > * Introduce a new specialized method to handle this scenario:
> >        -IDBRequest objStore.openDirectionalCursor(IDBCursor.PREV);
> >         * This will default internally to an IDBKeyRange with the properties
> defined below.
> >         * One advantage of this approach is that we don't have to expose a new
> IDBKeyRange constructor.
> > * Define a new static keyRange constructor that is a catch all:
> >        -static IDBKeyRange.all();
> >         * The values for the new constructor would be:
> >                IDBKeyRange.lower = undefined
> >                IDBKeyRange.upper = undefined
> >                IDBKeyRange.lowerOpen = false
> >                IDBKeyRange.upperOpen = false
> >         * I believe these satisfy the conditions for a key is in a key range section
> [1].
> >
> >         * We could pass this new keyRange to the existing openCursor method:
> >            -objStore.openCursor(IDBKeyRange.all(), IDBCursor.PREV);
> >
> > Let me know if you see any other alternatives or if you like any of these
> options.
> >
> > Israel
> > [1]
> > http://dvcs.w3.org/hg/IndexedDB/raw-file/tip/Overview.html#dfn-in-a-ke
> > y-range
> 
> I think we're talking past each other. I was trying to say that when I wrote the
> current spec text, my intent was for
> 
> req = objStore.openCursor(null, IDBCursor.PREV);
> 
> to work and return a cursor which iterates the whole object store.
> Same for indexes of course. The "Cursor Iteration Operation" should already
> support this unless I've made a mistake somewhere.
> 
> What's happened is that since I wrote the text WebIDL changed in such a way
> that it now requires types to be explicitly nullable. However I don't think that
> applies to the 'any' type used for most cursor-open functions.
> 
> However I do now see that the type defined for the open-cursor functions on
> indexes use "IDBKeyRange" rather than "any" as type. That needs to be fixed
> (In both sync and async) such that you can do things like
> 
> req = myindex.openCursor(4);
> and
> req = myindex.openKeyCursor("hello", IDBCursor.PREV);
> 
> / Jonas

Great, thanks for clarifying!  Yes, I believe we are in violent agreement :-)

To avoid future confusion, I believe we should update the WebIDL table associated with IDBObjectStore.openCursor and IDBIndex.openCursor to add a checkmark in the Nullable field instead of an x.  Plus as you pointed out, we need to ensure the type arguments are the same for both of these two methods (add the any and replace the IDBKeyRange).

I can work with Eliot on this.

Israel

Received on Thursday, 30 June 2011 20:20:10 UTC