RE: [Bug 15434] New: [IndexedDB] Detail steps for assigning a key to a value

On Tuesday, January 24, 2012 11:38 AM, Jonas Sicking wrote:
> On Tue, Jan 24, 2012 at 8:43 AM, Joshua Bell <jsbell@chromium.org> wrote:
> > On Tue, Jan 24, 2012 at 2:21 AM, Jonas Sicking <jonas@sicking.cc> wrote:
> >>
> >> On Mon, Jan 23, 2012 at 5:34 PM, Joshua Bell <jsbell@chromium.org>
> wrote:
> >> > There's another edge case here - what happens on a put (etc)
> >> > request to an object store with a key generator when the object
> >> > store's key path does not yield a value, yet the algorithm below
> >> > exits without changing the value.
> >> >
> >> > Sample:
> >> >
> >> > store = db.createObjectStore("my-store", {keyPath: "a.b",
> autoIncrement:
> >> > true});
> >> > request = store.put("value");
> >> >
> >> > 3.2.5 for "put" has this error case "The object store uses in-line
> >> > keys and the result of evaluating the object store's key path
> >> > yields a value and that value is not a valid key." resulting in a
> >> > DataError.
> >>
> >> The intent here was for something like:
> >>
> >> store = db.createObjectStore("my-store", {keyPath: "a.b", autoIncrement:
> >> true});
> >> request = store.put({ a: { b: { hello: "world" } });
> >>
> >> In this case "4.7 Steps for extracting a key from a value using a key
> >> path" will return the { hello: "world" } object which is not a valid
> >> key and hence a DataError is thrown.
> >>
> >> > In this case, "4.7
> >> > Steps for extracting a key from a value using a key path" says "no
> >> > value is returned", so that error case doesn't apply.
> >>
> >> Yes, in your example that error is not applicable.
> >>
> >> > "5.1 Object Store Storage Operation" step 2 is: "If store uses a
> >> > key generator and key is undefined, set key to the next generated
> >> > key. If store also uses in-line keys, then set the property in
> >> > value pointed to by store's key path to the new value for key."
> >> >
> >> > Per the algorithm below, the value would not change. (Another
> >> > example would be a keyPath of "length" and putting [1,2,3])
> >> >
> >
> >
> > Although it's unimportant to the discussion below, I realized after
> > the fact that my Array/length example was lousy since |length| is of
> > course assignable.
> 
> Just to be perfectly clear and avoid any misunderstanding, the same thing
> happens for non-assignable properties. For example:
> 
> store1 = db.createObjectStore("store1", { keyPath: "a.length",
> autoIncrement: true});
> store1.put({ a: "str" }); // stores an entry with key 3
> store2 = db.createObjectStore("store2", { keyPath: "a.size",
> autoIncrement: true});
> store2.put({ a: my10KbBlob }); // stores an entry with key 10240
> 
> >> So, with that in mind we still need to figure out the various edge
> >> cases and write a detailed set of steps for modifying a value using a
> >> keyPath. In all these examples i'll assume that the key 1 is
> >> generated. I've included the Firefox behavior in all cases, not
> >> because I think it's obviously correct, but as a data point. I'm
> >> curious to hear what you guys do too.
> >>
> >> What happens if a there are missing objects "higher up" in the keyPath:
> >> store = db.createObjectStore("os", { keyPath: "a.b.c", autoIncrement:
> >> true }); store.put({ x: "str" }); Here there is nowhere to directly
> >> store the new value since there is no "a" property.
> >> What we do in Firefox is to insert objects as needed. In this case
> >> we'd modify the value such that we get the following:
> >> { x: "str", a: { b: { c: 1 } } }
> >> Same thing goes if part of the "object chain" is there:
> >> store = db.createObjectStore("os", { keyPath: "a.b.c", autoIncrement:
> >> true }); store.put({ x: "str", a: {} }); Here Firefox will again
> >> store { x: "str", a: { b: { c: 1 } } }
> >>
> >
> > Per this thread/bug, I've landed a patch in Chromium to follow this
> > behavior. Should be in Chrome Canary already and show up in 18.
> 
> Cool.

IE follows the same behavior as FF.

> 
> >> What happens if a value "higher up" in the keyPath is not an object:
> >> store = db.createObjectStore("os", { keyPath: "a.b.c", autoIncrement:
> >> true }); store.put({ a: "str" }); Here there not only is nowhere to
> >> directly store the new value. We also can't simply insert the missing
> >> objects since we can't add a "b"
> >> property to the value "str". Exact same scenario appears if you
> >> replace "str" with a 1 or null.
> >> What we do in Firefox is to throw a DataError exception.
> >> Another example of this is simply
> >> store = db.createObjectStore("os", { keyPath: "a", autoIncrement:
> >> true }); store.put("str");
> >
> > Chrome currently defers setting the new value until the transaction
> > executes the asynchronous request, and thus doesn't raise an exception
> > but fails the request. I agree that doing this at the time of the call
> > makes more sense and is more consistent and predictable. If there's
> > consensus here I'll file a bug against Chromium.
> 
> Awesome!
> 

IE follows the same behavior as FF.

> >> What happens if a value "higher up" in the keyPath is a host object:
> >> store = db.createObjectStore("os", { keyPath: "a.b", autoIncrement:
> >> true }); store.put({ a: myBlob }); While we can set a 'b' property on
> >> the blob, the structured clone algorithm wouldn't copy this property
> >> and so it'd be pretty useless.
> >> The exact same question applies if a is set to a Date or a RegExp too.
> >> In Firefox I actually think that we set a useless 'b' property on the
> >> blob which I would say is a bug.
> >
> > I believe Chrome does the same as Firefox here - assign a property
> > which will then fail to be serialized.
> >
> >>
> >> Whatever we decide here, we should probably be consistent with the
> >> previous case of "a" being a string rather than a blob.
> >
> >
> > ... and I agree that being consistent with the "string" case. If the
> > value is not going to be serialized, we should probably fail.
> 
> Awesome.
> 

IE successfully adds the record only when autoInc is set.  When getting the record, the Blob has the property that contains the key.  We retrieve the key before the Blob is serialized and store it.  Later, we add the key to the record after the Blob is de-serialized from the database.  That's why this works in our implementation even though SCA doesn't clone the property.  If a developer manually defines the key on the keyPath without autInc we will throw a DataError exception type.

However, we agree that we should be consistent with the "string" case and fail.

> >> What happens if a value "higher up" in the keyPath is an Array object:
> >> store = db.createObjectStore("os", { keyPath: "a.b", autoIncrement:
> >> true }); store.put({ a: [1, 2] }); We can simply set a 'b' property
> >> on the array since that property should get copied during the
> >> structured clone. But since it's non-obvious I wanted to point it
> >> out.
> >> In Firefox we set a 'b' property on the array.
> >
> >
> > Good example. I'll have to test, but I'm pretty sure Chrome will
> > behave the same as Firefox here.
> 
> Sounds good.

IE follows the same behavior as FF.

> 
> >> There's also the somewhat ambiguous situation where a keyPath points
> >> to a property which does exist, but where the property has the value
> >> 'undefined':
> >> store = db.createObjectStore("os", { keyPath: "a", autoIncrement:
> >> true }); store.put({ a: undefined }); I know I above said that the
> >> steps for modifying a value doesn't even run if evaluating "4.7 Steps
> >> for extracting a key from a value using a key path" had yielded a
> >> value. The question is if yielding 'undefined'
> >> counts as yielding a value.
> >> If it does count as yielding a value then the above code throws since
> >> undefined isn't a valid key. I.e. we hit the "The object store uses
> >> in-line keys and the result of evaluating the object store's key path
> >> yields a value and that value is not a valid key." case which throws
> >> a DataError.
> >> In Firefox we do count it as yielding a value and so we throw an
> >> exception.
> >

IE also throws a DataError exception type.

> >
> > I'm in the process of landing a patch which brings Chrome's behavior
> > on put/add calls in line with with the spec (hence the thread!); the
> > current state of the patch give the same behavior as you describe for
> Firefox:
> > |undefined| is considered a value, and so an exception is thrown.
> 
> I'm very much unsure what the better behavior is here. And note that this
> situation also appears in the following scenario:
> 
> store = db.createObjectStore("os", { keyPath: "a.b", autoIncrement: true });
> store.put({ a: undefined });
> 
> Considering undefined a value, and thus throwing, is the more conservative
> choice and does let us change the behavior later if we want to later.
> 
> Based on this (pending details from microsoft of course) I suggest the
> following set of steps:
> 
> 1. If /keyPath/ is the empty string, skip the remaining steps and /value/ is not
> modified.
> 2. Let /remainingKeypath/ be /keyPath/ and /object/ be /value/.
> 3. if /object/ is not an Object object or an Array object (see structured clone
> algorithm), then throw a DOMException of type DataError.
> 4. If /remainingKeypath/ has a period in it, assign /remainingKeypath/ to be
> everything after the first period and assign /attribute/ to be everything
> before that first period. Otherwise, go to step 8.
> 5. If /object/ does not have an attribute named /attribute/, then create the
> attribute and assign it an empty object.
> 6. Assign /object/ to be the value of the attribute named /attribute/ on
> /object/.
> 7. Go to step 3.
> 8. NOTE: The steps leading here ensure that /remainingKeyPath/ is a single
> attribute name (i.e. string without periods) by this step. They also ensure
> that /object/ is an Object or an Array, and not a Date, RegExp, Blob etc.
> 9. Let /attribute/ be /remainingKeyPath/ 10. Set an attribute named
> /attribute/ on /object/ with the value /key/.
> 
> Note that the intent is that these steps are only executed if evaluating the
> keyPath did not yield a value. I.e. if we know that we need to modify the
> stored value. Because of this we know that at step
> 10 the object does not have an attribute with name /attribute/.
> 
> Let me know if these steps sound ok?
> 
> / Jonas
> 

The steps look good to me.

Israel

Received on Wednesday, 25 January 2012 00:17:22 UTC