Changeset 105891 in webkit


Ignore:
Timestamp:
Jan 25, 2012, 10:01:13 AM (13 years ago)
Author:
jsbell@chromium.org
Message:

IndexedDB: Need to distinguish key paths that don't yield value vs. yield invalid key
​https://bugs.webkit.org/show_bug.cgi?id=76487

Source/WebCore:

Implement the precondition checks for IDBObjectStore.add/put operations: raise an error
if there is a key generator (autoIncrement) and the path yields a value and the value
is not a valid key; raise an error if any of the index key paths yield a value which
is not a valid key.

Reviewed by Tony Chang.

Tests: storage/indexeddb/keypath-edges.html

storage/indexeddb/objectstore-basics.html

  • storage/IDBObjectStoreBackendImpl.cpp:

(WebCore::IDBObjectStoreBackendImpl::put):

Source/WebKit/chromium:

Added a NullType to represent a null IDBKey pointer. This is needed to distinguish the
cases in the spec where the key resolution algorithm returns no value (null) versus
returns a value but that value is not a valid key (invalid).

Reviewed by Tony Chang.

  • public/WebIDBKey.h:
  • src/WebIDBKey.cpp:

(WebKit::WebIDBKey::createNull): Added.
(WebKit::WebIDBKey::createFromValueAndKeyPath): Now returns null if value is null.
(WebKit::convertFromWebIDBKeyArray): Null keys should never exist within arrays.
(WebKit::WebIDBKey::assignInvalid):
(WebKit::WebIDBKey::assignNull):
(WebKit::WebIDBKey::type):

LayoutTests:

Reviewed by Tony Chang.

  • storage/indexeddb/keypath-edges-expected.txt: Added.
  • storage/indexeddb/keypath-edges.html: Added.
  • storage/indexeddb/objectstore-basics-expected.txt:
  • storage/indexeddb/objectstore-basics.html:
Location:
trunk
Files:
2 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r105890 r105891  
     12012-01-25  Joshua Bell  <jsbell@chromium.org>
     2
     3        IndexedDB: Need to distinguish key paths that don't yield value vs. yield invalid key
     4        https://bugs.webkit.org/show_bug.cgi?id=76487
     5
     6        Reviewed by Tony Chang.
     7
     8        * storage/indexeddb/keypath-edges-expected.txt: Added.
     9        * storage/indexeddb/keypath-edges.html: Added.
     10        * storage/indexeddb/objectstore-basics-expected.txt:
     11        * storage/indexeddb/objectstore-basics.html:
     12
    1132012-01-25  Tony Chang  <tony@chromium.org>
    214

  • trunk/LayoutTests/storage/indexeddb/objectstore-basics-expected.txt

    r105329 r105891  
    9999db.transaction(['storeName'], webkitIDBTransaction.READ_WRITE)
    100100store = transaction.objectStore('storeName')
    101 store.add({x: null}, 'validkey')
    102 PASS event.cancelable is true
    103 addWithNullIndexFailure():
    104 PASS event.target.errorCode is webkitIDBDatabaseException.DATA_ERR
    105 event.preventDefault()
     101Expecting exception from store.add({x: null}, 'validkey')
     102PASS Exception was thrown.
     103PASS code is webkitIDBDatabaseException.DATA_ERR
    106104db.transaction(['storeName'], webkitIDBTransaction.READ_WRITE)
    107105store = transaction.objectStore('storeName')
    … …  
    165163PASS Exception was thrown.
    166164PASS code is webkitIDBDatabaseException.DATA_ERR
     165If there are any indexes referencing this object store whose key path is a string, evaluating their key path on the value parameter yields a value, and that value is not a valid key.
     166Expecting exception from storeWithIndex.put({indexKey: null}, 'key')
     167PASS Exception was thrown.
     168PASS code is webkitIDBDatabaseException.DATA_ERR
    167169
    168170IDBObjectStore.add()
    … …  
    187189PASS Exception was thrown.
    188190PASS code is webkitIDBDatabaseException.DATA_ERR
     191If there are any indexes referencing this object store whose key path is a string, evaluating their key path on the value parameter yields a value, and that value is not a valid key.
     192Expecting exception from storeWithIndex.add({indexKey: null}, 'key')
     193PASS Exception was thrown.
     194PASS code is webkitIDBDatabaseException.DATA_ERR
    189195PASS successfullyParsed is true
    190196

  • trunk/LayoutTests/storage/indexeddb/objectstore-basics.html

    r105329 r105891  
    216216    var store = evalAndLog("store = transaction.objectStore('storeName')");
    217217
    218     request = evalAndLog("store.add({x: null}, 'validkey')");
    219     request.onsuccess = unexpectedSuccessCallback;
    220     request.onerror = addWithNullIndexFailure;
    221 }
    222 
    223 function addWithNullIndexFailure()
    224 {
    225     shouldBeTrue("event.cancelable");
    226     debug("addWithNullIndexFailure():");
    227     shouldBe("event.target.errorCode", "webkitIDBDatabaseException.DATA_ERR");
    228 
    229     evalAndLog("event.preventDefault()");
     218    evalAndExpectException("store.add({x: null}, 'validkey')", "webkitIDBDatabaseException.DATA_ERR");
    230219
    231220    transaction = evalAndLog("db.transaction(['storeName'], webkitIDBTransaction.READ_WRITE)");
    … …  
    319308        evalAndExpectException("storeWithOutOfLineKeys.put({}, null)", "webkitIDBDatabaseException.DATA_ERR");
    320309
    321         // FIXME: Add precondition checks for put() with index key paths that yield invalid keys.
    322         // https://bugs.webkit.org/show_bug.cgi?id=76487
     310        debug("If there are any indexes referencing this object store whose key path is a string, evaluating their key path on the value parameter yields a value, and that value is not a valid key.");
     311        evalAndExpectException("storeWithIndex.put({indexKey: null}, 'key')", "webkitIDBDatabaseException.DATA_ERR");
    323312
    324313        debug("");
    … …  
    339328        evalAndExpectException("storeWithOutOfLineKeys.add({}, null)", "webkitIDBDatabaseException.DATA_ERR");
    340329
    341         // FIXME: Add precondition checks for add() with index key paths that yield invalid keys.
    342         // https://bugs.webkit.org/show_bug.cgi?id=76487
     330        debug("If there are any indexes referencing this object store whose key path is a string, evaluating their key path on the value parameter yields a value, and that value is not a valid key.");
     331        evalAndExpectException("storeWithIndex.add({indexKey: null}, 'key')", "webkitIDBDatabaseException.DATA_ERR");
    343332
    344333        done();

  • trunk/Source/WebCore/ChangeLog

    r105885 r105891  
     12012-01-25  Joshua Bell  <jsbell@chromium.org>
     2
     3        IndexedDB: Need to distinguish key paths that don't yield value vs. yield invalid key
     4        https://bugs.webkit.org/show_bug.cgi?id=76487
     5
     6        Implement the precondition checks for IDBObjectStore.add/put operations: raise an error
     7        if there is a key generator (autoIncrement) and the path yields a value and the value
     8        is not a valid key; raise an error if any of the index key paths yield a value which
     9        is not a valid key.
     10
     11        Reviewed by Tony Chang.
     12
     13        Tests: storage/indexeddb/keypath-edges.html
     14               storage/indexeddb/objectstore-basics.html
     15
     16        * storage/IDBObjectStoreBackendImpl.cpp:
     17        (WebCore::IDBObjectStoreBackendImpl::put):
     18
    1192012-01-25  Yong Li  <yoli@rim.com>
    220

  • trunk/Source/WebCore/storage/IDBObjectStoreBackendImpl.cpp

    r105329 r105891  
    132132    RefPtr<IDBTransactionBackendInterface> transaction = transactionPtr;
    133133
    134     if (key && (key->type() == IDBKey::InvalidType)) {
    135         ec = IDBDatabaseException::DATA_ERR;
    136         return;
    137     }
    138 
    139     const bool autoIncrement = objectStore->autoIncrement();
    140     const bool hasKeyPath = !objectStore->m_keyPath.isNull();
    141 
    142134    if (putMode != CursorUpdate) {
     135        if (key && !key->valid()) {
     136            ec = IDBDatabaseException::DATA_ERR;
     137            return;
     138        }
     139        const bool autoIncrement = objectStore->autoIncrement();
     140        const bool hasKeyPath = !objectStore->m_keyPath.isNull();
    143141        if (!key && !autoIncrement && !hasKeyPath) {
    144142            ec = IDBDatabaseException::DATA_ERR;
    … …  
    146144        }
    147145        if (hasKeyPath) {
    148             if (key && key->valid()) {
     146            if (key) {
    149147                ec = IDBDatabaseException::DATA_ERR;
    150148                return;
    151149            }
     150
     151            RefPtr<IDBKey> keyPathKey = fetchKeyFromKeyPath(value.get(), objectStore->m_keyPath);
    152152            if (!autoIncrement) {
    153                 RefPtr<IDBKey> keyPathKey = fetchKeyFromKeyPath(value.get(), objectStore->m_keyPath);
    154153                if (!keyPathKey || !keyPathKey->valid()) {
    155154                    ec = IDBDatabaseException::DATA_ERR;
    156155                    return;
    157156                }
     157            } else if (keyPathKey && !keyPathKey->valid()) {
     158                ec = IDBDatabaseException::DATA_ERR;
     159                return;
    158160            }
    159161        }
    160         // FIXME: Add precondition checks for index key paths that yield invalid keys.
    161         // https://bugs.webkit.org/show_bug.cgi?id=76487
    162      }
     162        for (IndexMap::iterator it = m_indexes.begin(); it != m_indexes.end(); ++it) {
     163            const RefPtr<IDBIndexBackendImpl>& index = it->second;
     164            RefPtr<IDBKey> indexKey = fetchKeyFromKeyPath(value.get(), index->keyPath());
     165            if (indexKey && !indexKey->valid()) {
     166                ec = IDBDatabaseException::DATA_ERR;
     167                return;
     168            }
     169        }
     170    }
    163171
    164172    if (!transaction->scheduleTask(createCallbackTask(&IDBObjectStoreBackendImpl::putInternal, objectStore, value, key, putMode, callbacks, transaction)))
    … …  
    173181    const bool autoIncrement = objectStore->autoIncrement();
    174182    const bool hasKeyPath = !objectStore->m_keyPath.isNull();
    175 
    176     if (hasKeyPath && key && putMode != CursorUpdate) {
    177         callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "A key was supplied for an objectStore that has a keyPath."));
    178         return 0;
    179     }
    180183
    181184    if (autoIncrement && key) {
    … …  
    208211        RefPtr<IDBKey> keyPathKey = fetchKeyFromKeyPath(value.get(), objectStore->m_keyPath);
    209212
    210         if (!keyPathKey) {
    211             callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "The key could not be fetched from the keyPath."));
    212             return 0;
    213         }
    214 
     213        // FIXME: This check should be moved to put() and raise an exception. WK76952
    215214        if (putMode == CursorUpdate && !keyPathKey->isEqual(key)) {
    216215            callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "The key fetched from the keyPath does not match the key of the cursor."));
    … …  
    235234    if (!key)
    236235        return;
    237 
    238     if (key->type() == IDBKey::InvalidType) {
    239         callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "Not a valid key."));
    240         return;
    241     }
     236    ASSERT(key->valid());
    242237
    243238    Vector<RefPtr<IDBKey> > indexKeys;
    … …  
    250245            continue;
    251246        }
    252         if (indexKey->type() == IDBKey::InvalidType) {
    253             callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::DATA_ERR, "One of the derived (from a keyPath) keys for an index is not valid."));
    254             return;
    255         }
     247        ASSERT(indexKey->valid());
    256248
    257249        if ((!index->multiEntry() || indexKey->type() != IDBKey::ArrayType) && !index->addingKeyAllowed(indexKey.get(), key.get())) {
    … …  
    340332    RefPtr<IDBKey> key = prpKey;
    341333    RefPtr<IDBCallbacks> callbacks = prpCallbacks;
    342     if (key->type() == IDBKey::InvalidType) {
     334    if (!key || !key->valid()) {
    343335        ec = IDBDatabaseException::DATA_ERR;
    344336        return;

  • trunk/Source/WebKit/chromium/ChangeLog

    r105846 r105891  
     12012-01-25  Joshua Bell  <jsbell@chromium.org>
     2
     3        IndexedDB: Need to distinguish key paths that don't yield value vs. yield invalid key
     4        https://bugs.webkit.org/show_bug.cgi?id=76487
     5
     6        Added a NullType to represent a null IDBKey pointer. This is needed to distinguish the
     7        cases in the spec where the key resolution algorithm returns no value (null) versus
     8        returns a value but that value is not a valid key (invalid).
     9
     10        Reviewed by Tony Chang.
     11
     12        * public/WebIDBKey.h:
     13        * src/WebIDBKey.cpp:
     14        (WebKit::WebIDBKey::createNull): Added.
     15        (WebKit::WebIDBKey::createFromValueAndKeyPath): Now returns null if value is null.
     16        (WebKit::convertFromWebIDBKeyArray): Null keys should never exist within arrays.
     17        (WebKit::WebIDBKey::assignInvalid):
     18        (WebKit::WebIDBKey::assignNull):
     19        (WebKit::WebIDBKey::type):
     20
    1212012-01-24  Vsevolod Vlasov  <vsevik@chromium.org>
    222

  • trunk/Source/WebKit/chromium/public/WebIDBKey.h

    r101122 r105891  
    5050    WEBKIT_EXPORT static WebIDBKey createNumber(double);
    5151    WEBKIT_EXPORT static WebIDBKey createInvalid();
     52    WEBKIT_EXPORT static WebIDBKey createNull();
    5253    WEBKIT_EXPORT static WebIDBKey createFromValueAndKeyPath(const WebSerializedScriptValue&, const WebIDBKeyPath&);
    5354    WEBKIT_EXPORT static WebSerializedScriptValue injectIDBKeyIntoSerializedValue(const WebIDBKey&, const WebSerializedScriptValue&, const WebIDBKeyPath&);
    … …  
    6667    WEBKIT_EXPORT void assignNumber(double);
    6768    WEBKIT_EXPORT void assignInvalid();
     69    WEBKIT_EXPORT void assignNull();
    6870    WEBKIT_EXPORT void reset();
    6971
    … …  
    7375        StringType,
    7476        DateType,
    75         NumberType
     77        NumberType,
     78        NullType,
    7679    };
    7780

  • trunk/Source/WebKit/chromium/src/WebIDBKey.cpp

    r102044 r105891  
    7777}
    7878
     79WebIDBKey WebIDBKey::createNull()
     80{
     81    WebIDBKey key;
     82    key.assignNull();
     83    return key;
     84}
     85
    7986WebIDBKey WebIDBKey::createFromValueAndKeyPath(const WebSerializedScriptValue& serializedScriptValue, const WebIDBKeyPath& idbKeyPath)
    8087{
     88    // FIXME: If key path is empty string, this should return invalid key instead
    8189    if (serializedScriptValue.isNull())
    82         return WebIDBKey::createInvalid();
     90        return WebIDBKey::createNull();
    8391    return createIDBKeyFromSerializedValueAndKeyPath(serializedScriptValue, idbKeyPath);
    8492}
    … …  
    113121            break;
    114122        case WebIDBKey::InvalidType:
     123        case WebIDBKey::NullType:
    115124            ASSERT_NOT_REACHED();
    116125            break;
    … …  
    171180void WebIDBKey::assignInvalid()
    172181{
     182    m_private = IDBKey::createInvalid();
     183}
     184
     185void WebIDBKey::assignNull()
     186{
    173187    m_private = 0;
    174188}
    … …  
    182196{
    183197    if (!m_private.get())
    184         return InvalidType;
     198        return NullType;
    185199    return Type(m_private->type());
    186200}
Note: See TracChangeset for help on using the changeset viewer.