Bug 706659 part 2: Support empty keypaths on objectStores. r=bent
authorJonas Sicking <jonas@sicking.cc>
Fri, 02 Dec 2011 18:32:46 -0800
changeset 82006 1fddf8667d2180f6a347ccff45cc7504fa18d4b1
parent 82005 1b9ca56d4aab132ab290aeecafbcf3740894aaa5
child 82007 513428a937004e6de2abdffefb18339a8c6e4c5f
push idunknown
push userunknown
push dateunknown
reviewersbent
bugs706659
milestone11.0a1
Bug 706659 part 2: Support empty keypaths on objectStores. r=bent
dom/indexedDB/IDBDatabase.cpp
dom/indexedDB/IDBFactory.cpp
dom/indexedDB/IDBObjectStore.cpp
dom/indexedDB/IDBObjectStore.h
dom/indexedDB/IndexedDatabase.h
dom/indexedDB/OpenDatabaseHelper.cpp
dom/indexedDB/OpenDatabaseHelper.h
dom/indexedDB/test/test_complex_keyPaths.html
dom/indexedDB/test/test_create_objectStore.html
dom/indexedDB/test/test_transaction_abort.html
--- a/dom/indexedDB/IDBDatabase.cpp
+++ b/dom/indexedDB/IDBDatabase.cpp
@@ -459,21 +459,18 @@ IDBDatabase::CreateObjectStore(const nsA
 
   if (!transaction ||
       transaction->Mode() != nsIIDBTransaction::VERSION_CHANGE) {
     return NS_ERROR_DOM_INDEXEDDB_NOT_ALLOWED_ERR;
   }
 
   DatabaseInfo* databaseInfo = Info();
 
-  if (databaseInfo->ContainsStoreName(aName)) {
-    return NS_ERROR_DOM_INDEXEDDB_CONSTRAINT_ERR;
-  }
-
   nsString keyPath;
+  keyPath.SetIsVoid(true);
   bool autoIncrement = false;
 
   if (!JSVAL_IS_VOID(aOptions) && !JSVAL_IS_NULL(aOptions)) {
     if (JSVAL_IS_PRIMITIVE(aOptions)) {
       // XXX This isn't the right error
       return NS_ERROR_DOM_TYPE_ERR;
     }
 
@@ -509,17 +506,21 @@ IDBDatabase::CreateObjectStore(const nsA
     JSBool boolVal;
     if (!JS_ValueToBoolean(aCx, val, &boolVal)) {
       NS_WARNING("JS_ValueToBoolean failed!");
       return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
     }
     autoIncrement = !!boolVal;
   }
 
-  if (!IDBObjectStore::IsValidKeyPath(aCx, keyPath)) {
+  if (databaseInfo->ContainsStoreName(aName)) {
+    return NS_ERROR_DOM_INDEXEDDB_CONSTRAINT_ERR;
+  }
+
+  if (!keyPath.IsVoid() && !IDBObjectStore::IsValidKeyPath(aCx, keyPath)) {
     return NS_ERROR_DOM_SYNTAX_ERR;
   }
 
   nsAutoPtr<ObjectStoreInfo> newInfo(new ObjectStoreInfo());
 
   newInfo->name = aName;
   newInfo->id = databaseInfo->nextObjectStoreId++;
   newInfo->keyPath = keyPath;
@@ -795,36 +796,38 @@ IDBDatabase::PostHandleEvent(nsEventChai
   return NS_OK;
 }
 
 nsresult
 CreateObjectStoreHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
 {
   nsCOMPtr<mozIStorageStatement> stmt =
     mTransaction->GetCachedStatement(NS_LITERAL_CSTRING(
-    "INSERT INTO object_store (id, name, key_path, auto_increment) "
-    "VALUES (:id, :name, :key_path, :auto_increment)"
+    "INSERT INTO object_store (id, auto_increment, name, key_path) "
+    "VALUES (:id, :auto_increment, :name, :key_path)"
   ));
   NS_ENSURE_TRUE(stmt, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
   mozStorageStatementScoper scoper(stmt);
 
   nsresult rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("id"),
                                        mObjectStore->Id());
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
+  rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("auto_increment"),
+                             mObjectStore->IsAutoIncrement() ? 1 : 0);
+  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+
   rv = stmt->BindStringByName(NS_LITERAL_CSTRING("name"), mObjectStore->Name());
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
-  rv = stmt->BindStringByName(NS_LITERAL_CSTRING("key_path"),
-                              mObjectStore->KeyPath());
-  NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
-
-  rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("auto_increment"),
-                             mObjectStore->IsAutoIncrement() ? 1 : 0);
+  rv = mObjectStore->HasKeyPath() ?
+    stmt->BindStringByName(NS_LITERAL_CSTRING("key_path"),
+                           mObjectStore->KeyPath()) :
+    stmt->BindNullByName(NS_LITERAL_CSTRING("key_path"));
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
   rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
   return NS_OK;
 }
 
--- a/dom/indexedDB/IDBFactory.cpp
+++ b/dom/indexedDB/IDBFactory.cpp
@@ -242,18 +242,28 @@ IDBFactory::LoadDatabaseInformation(mozI
 
     ObjectStoreInfo* info = element->get();
 
     rv = stmt->GetString(0, info->name);
     NS_ENSURE_SUCCESS(rv, rv);
 
     info->id = stmt->AsInt64(1);
 
-    rv = stmt->GetString(2, info->keyPath);
+    PRInt32 columnType;
+    nsresult rv = stmt->GetTypeOfIndex(2, &columnType);
     NS_ENSURE_SUCCESS(rv, rv);
+    if (columnType == mozIStorageStatement::VALUE_TYPE_NULL) {
+      info->keyPath.SetIsVoid(true);
+    }
+    else {
+      NS_ASSERTION(columnType == mozIStorageStatement::VALUE_TYPE_TEXT,
+                   "Should be a string");
+      rv = stmt->GetString(2, info->keyPath);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
 
     info->autoIncrement = !!stmt->AsInt32(3);
     info->databaseId = aDatabaseId;
 
     ObjectStoreInfoMap* mapEntry = infoMap.AppendElement();
     NS_ENSURE_TRUE(mapEntry, NS_ERROR_OUT_OF_MEMORY);
 
     mapEntry->id = info->id;
--- a/dom/indexedDB/IDBObjectStore.cpp
+++ b/dom/indexedDB/IDBObjectStore.cpp
@@ -894,34 +894,30 @@ IDBObjectStore::GetAddInfo(JSContext* aC
                            Key& aKey,
                            nsTArray<IndexUpdateInfo>& aUpdateInfoArray,
                            PRUint64* aOffsetToKeyProp)
 {
   nsresult rv;
 
   // Return DATA_ERR if a key was passed in and this objectStore uses inline
   // keys.
-  if (!JSVAL_IS_VOID(aKeyVal) && !mKeyPath.IsEmpty()) {
+  if (!JSVAL_IS_VOID(aKeyVal) && HasKeyPath()) {
     return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
   }
 
   JSAutoRequest ar(aCx);
 
-  if (mKeyPath.IsEmpty()) {
+  if (!HasKeyPath()) {
     // Out-of-line keys must be passed in.
     rv = aKey.SetFromJSVal(aCx, aKeyVal);
     NS_ENSURE_SUCCESS(rv, rv);
   }
   else {
     // Inline keys live on the object. Make sure that the value passed in is an
     // object.
-    if (JSVAL_IS_PRIMITIVE(aValue)) {
-      return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
-    }
-
     rv = GetKeyFromValue(aCx, aValue, mKeyPath, aKey);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   // Return DATA_ERR if no key was specified this isn't an autoIncrement
   // objectStore.
   if (aKey.IsUnset() && !mAutoIncrement) {
     return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
@@ -936,18 +932,22 @@ IDBObjectStore::GetAddInfo(JSContext* aC
   rv = GetIndexUpdateInfo(info, aCx, aValue, aUpdateInfoArray);
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
   const jschar* keyPathChars =
     reinterpret_cast<const jschar*>(mKeyPath.get());
   const size_t keyPathLen = mKeyPath.Length();
   JSBool ok = JS_FALSE;
 
-  if (!mKeyPath.IsEmpty() && aKey.IsUnset()) {
+  if (HasKeyPath() && aKey.IsUnset()) {
     NS_ASSERTION(mAutoIncrement, "Should have bailed earlier!");
+    
+    if (JSVAL_IS_PRIMITIVE(aValue)) {
+      return NS_ERROR_DOM_INDEXEDDB_DATA_ERR;
+    }
 
     JSObject* obj = JS_NewObject(aCx, &gDummyPropClass, nsnull, nsnull);
     NS_ENSURE_TRUE(obj, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
     jsval key = OBJECT_TO_JSVAL(obj);
  
     ok = JS_DefineUCProperty(aCx, JSVAL_TO_OBJECT(aValue), keyPathChars,
                              keyPathLen, key, nsnull, nsnull,
--- a/dom/indexedDB/IDBObjectStore.h
+++ b/dom/indexedDB/IDBObjectStore.h
@@ -140,16 +140,21 @@ public:
     return mId;
   }
 
   const nsString& KeyPath() const
   {
     return mKeyPath;
   }
 
+  const bool HasKeyPath() const
+  {
+    return !mKeyPath.IsVoid();
+  }
+
   IDBTransaction* Transaction()
   {
     return mTransaction;
   }
 
   nsresult ModifyValueForNewKey(JSAutoStructuredCloneBuffer& aBuffer,
                                 Key& aKey,
                                 PRUint64 aOffsetToKeyProp);
--- a/dom/indexedDB/IndexedDatabase.h
+++ b/dom/indexedDB/IndexedDatabase.h
@@ -46,17 +46,17 @@
 #include "jsapi.h"
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "nsDebug.h"
 #include "nsDOMError.h"
 #include "nsStringGlue.h"
 #include "nsTArray.h"
 
-#define DB_SCHEMA_VERSION 6
+#define DB_SCHEMA_VERSION 7
 
 #define BEGIN_INDEXEDDB_NAMESPACE \
   namespace mozilla { namespace dom { namespace indexedDB {
 
 #define END_INDEXEDDB_NAMESPACE \
   } /* namespace indexedDB */ } /* namepsace dom */ } /* namespace mozilla */
 
 #define USING_INDEXEDDB_NAMESPACE \
--- a/dom/indexedDB/OpenDatabaseHelper.cpp
+++ b/dom/indexedDB/OpenDatabaseHelper.cpp
@@ -117,19 +117,19 @@ CreateTables(mozIStorageConnection* aDBC
     ");"
   ));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Table `object_store`
   rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "CREATE TABLE object_store ("
       "id INTEGER PRIMARY KEY, "
+      "auto_increment INTEGER NOT NULL DEFAULT 0, "
       "name TEXT NOT NULL, "
-      "key_path TEXT NOT NULL, "
-      "auto_increment INTEGER NOT NULL DEFAULT 0, "
+      "key_path TEXT, "
       "UNIQUE (name)"
     ");"
   ));
   NS_ENSURE_SUCCESS(rv, rv);
 
   // Table `object_data`
   rv = aDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "CREATE TABLE object_data ("
@@ -749,16 +749,82 @@ UpgradeSchemaFrom5To6(mozIStorageConnect
 
   rv = transaction.Commit();
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 nsresult
+UpgradeSchemaFrom6To7(mozIStorageConnection* aConnection)
+{
+  mozStorageTransaction transaction(aConnection, false,
+                                 mozIStorageConnection::TRANSACTION_IMMEDIATE);
+
+  // Turn off foreign key constraints before we do anything here.
+  nsresult rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "PRAGMA foreign_keys = OFF;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TEMPORARY TABLE temp_upgrade ("
+      "id, "
+      "name, "
+      "key_path, "
+      "auto_increment, "
+    ");"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "INSERT INTO temp_upgrade "
+      "SELECT id, name, key_path, auto_increment "
+      "FROM object_store;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TABLE object_store;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TABLE object_store ("
+      "id INTEGER PRIMARY KEY, "
+      "auto_increment INTEGER NOT NULL DEFAULT 0, "
+      "name TEXT NOT NULL, "
+      "key_path TEXT, "
+      "UNIQUE (name)"
+    ");"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "INSERT INTO object_store "
+      "SELECT id, auto_increment, name, nullif(key_path, '') "
+      "FROM temp_upgrade;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TABLE temp_upgrade;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = aConnection->SetSchemaVersion(7);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return NS_OK;
+}
+
+nsresult
 CreateDatabaseConnection(const nsAString& aName,
                          nsIFile* aDBFile,
                          mozIStorageConnection** aConnection)
 {
   NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
 
   NS_NAMED_LITERAL_CSTRING(quotaVFSName, "quota");
 
@@ -799,31 +865,32 @@ CreateDatabaseConnection(const nsAString
     NS_ENSURE_SUCCESS(rv, rv);
 
     NS_ASSERTION(NS_SUCCEEDED(connection->GetSchemaVersion(&schemaVersion)) &&
                  schemaVersion == DB_SCHEMA_VERSION,
                  "CreateTables set a bad schema version!");
   }
   else if (schemaVersion != DB_SCHEMA_VERSION) {
     // This logic needs to change next time we change the schema!
-    PR_STATIC_ASSERT(DB_SCHEMA_VERSION == 6);
+    PR_STATIC_ASSERT(DB_SCHEMA_VERSION == 7);
 
 #define UPGRADE_SCHEMA_CASE(_from, _to)                                        \
   if (schemaVersion == _from) {                                                \
     rv = UpgradeSchemaFrom##_from##To##_to (connection);                       \
     NS_ENSURE_SUCCESS(rv, rv);                                                 \
                                                                                \
     rv = connection->GetSchemaVersion(&schemaVersion);                         \
     NS_ENSURE_SUCCESS(rv, rv);                                                 \
                                                                                \
     NS_ASSERTION(schemaVersion == _to, "Bad upgrade function!");               \
   }
 
     UPGRADE_SCHEMA_CASE(4, 5)
     UPGRADE_SCHEMA_CASE(5, 6)
+    UPGRADE_SCHEMA_CASE(6, 7)
 
 #undef UPGRADE_SCHEMA_CASE
 
     if (schemaVersion != DB_SCHEMA_VERSION) {
       NS_WARNING("Unable to open IndexedDB database, schema doesn't match");
       return NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
     }
   }
--- a/dom/indexedDB/OpenDatabaseHelper.h
+++ b/dom/indexedDB/OpenDatabaseHelper.h
@@ -55,18 +55,18 @@ class OpenDatabaseHelper : public Helper
 public:
   OpenDatabaseHelper(IDBOpenDBRequest* aRequest,
                      const nsAString& aName,
                      const nsACString& aASCIIOrigin,
                      PRUint64 aRequestedVersion,
                      bool aForDeletion)
     : HelperBase(aRequest), mOpenDBRequest(aRequest), mName(aName),
       mASCIIOrigin(aASCIIOrigin), mRequestedVersion(aRequestedVersion),
-      mForDeletion(aForDeletion), mCurrentVersion(0),
-      mDataVersion(DB_SCHEMA_VERSION), mDatabaseId(0), mLastObjectStoreId(0),
+      mForDeletion(aForDeletion), mDatabaseId(nsnull), mCurrentVersion(0),
+      mDataVersion(DB_SCHEMA_VERSION), mLastObjectStoreId(0),
       mLastIndexId(0), mState(eCreated), mResultCode(NS_OK)
   {
     NS_ASSERTION(!aForDeletion || !aRequestedVersion,
                  "Can't be for deletion and request a version!");
   }
 
   NS_DECL_ISUPPORTS
   NS_DECL_NSIRUNNABLE
--- a/dom/indexedDB/test/test_complex_keyPaths.html
+++ b/dom/indexedDB/test/test_complex_keyPaths.html
@@ -27,16 +27,21 @@
         { keyPath: "id",      value: { id: /x/ } },
         { keyPath: "id",      value: 2 },
         { keyPath: "id",      value: undefined },
         { keyPath: "foo.id",  value: { foo: { id: 7 } },             key: 7 },
         { keyPath: "foo.id",  value: { id: 7, foo: { id: "asdf" } }, key: "asdf" },
         { keyPath: "foo.id",  value: { foo: { id: undefined } } },
         { keyPath: "foo.id",  value: { foo: 47 } },
         { keyPath: "foo.id",  value: {} },
+        { keyPath: "",        value: "foopy",                        key: "foopy" },
+        { keyPath: "",        value: 2,                              key: 2 },
+        { keyPath: "",        value: undefined },
+        { keyPath: "",        value: { id: 12 } },
+        { keyPath: "",        value: /x/ },
         { keyPath: "foo.bar", value: { baz: 1, foo: { baz2: 2, bar: "xo" } },     key: "xo" },
         { keyPath: "foo.bar.baz", value: { foo: { bar: { bazz: 16, baz: 17 } } }, key: 17 },
         { keyPath: "foo..id", exception: true },
         { keyPath: "foo.",    exception: true },
         { keyPath: "fo o",    exception: true },
         { keyPath: "foo ",    exception: true },
         { keyPath: "foo[bar]",exception: true },
         { keyPath: "$('id').stuff", exception: true },
@@ -56,16 +61,17 @@
       // Test creating object stores and inserting data
       for (let i = 0; i < keyPaths.length; i++) {
         let info = keyPaths[i];
         test = " for " + JSON.stringify(info);
         if (!stores[info.keyPath]) {
           try {
             let objectStore = db.createObjectStore(info.keyPath, { keyPath: info.keyPath });
             ok(!("exception" in info), "expected exception behavior observed" + test);
+            is(objectStore.keyPath, info.keyPath, "objectStore has correct keyPath property" + test);
             stores[info.keyPath] = objectStore;
           } catch (e) {
             ok("exception" in info, "expected exception behavior observed" + test);
             ok(e instanceof DOMException, "Got a DOM Exception" + test);
             is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error" + test);
             continue;
           }
         }
@@ -98,16 +104,17 @@
       let store = db.createObjectStore("indexStore");
       let indexes = {};
       for (let i = 0; i < keyPaths.length; i++) {
         let info = keyPaths[i];
         if (!indexes[info.keyPath]) {
           try {
             let index = store.createIndex(info.keyPath, info.keyPath);
             ok(!("exception" in info), "expected exception behavior observed");
+            is(index.keyPath, info.keyPath, "index has correct keyPath property");
             indexes[info.keyPath] = index;
           } catch (e) {
             ok("exception" in info, "expected exception behavior observed");
             ok(e instanceof DOMException, "Got a DOM Exception");
             is(e.code, DOMException.SYNTAX_ERR, "expect a syntax error");
             continue;
           }
         }
--- a/dom/indexedDB/test/test_create_objectStore.html
+++ b/dom/indexedDB/test/test_create_objectStore.html
@@ -81,17 +81,17 @@
             found = true;
             break;
           }
         }
         is(found, true, "objectStoreNames contains name");
 
         is(objectStore.name, name, "Bad name");
         is(objectStore.keyPath, info.options && info.options.keyPath ?
-                                info.options.keyPath : "",
+                                info.options.keyPath : null,
            "Bad keyPath");
         if(objectStore.indexNames.length, 0, "Bad indexNames");
 
         ok(event.target.transaction, "event has a transaction");
         ok(event.target.transaction.db === db, "transaction has the right db");
         is(event.target.transaction.readyState, nsIIDBTransaction.LOADING,
            "transaction has the correct readyState");
         is(event.target.transaction.mode, nsIIDBTransaction.VERSION_CHANGE,
--- a/dom/indexedDB/test/test_transaction_abort.html
+++ b/dom/indexedDB/test/test_transaction_abort.html
@@ -52,17 +52,17 @@
       is(transaction.mode, VERSION_CHANGE, "Correct mode");
       is(transaction.objectStoreNames.length, 1, "Correct names length");
       is(transaction.objectStoreNames.item(0), "foo", "Correct name");
       is(transaction.objectStore("foo"), objectStore, "Can get stores");
       is(transaction.oncomplete, null, "No complete listener");
       is(transaction.onabort, null, "No abort listener");
 
       is(objectStore.name, "foo", "Correct name");
-      is(objectStore.keyPath, "", "Correct keyPath");
+      is(objectStore.keyPath, null, "Correct keyPath");
 
       is(objectStore.indexNames.length, 1, "Correct indexNames length");
       is(objectStore.indexNames[0], "fooindex", "Correct indexNames name");
       is(objectStore.index("fooindex"), index, "Can get index");
 
       // Wait until it's complete!
       transaction.oncomplete = grabEventAndContinueHandler;
       event = yield;
@@ -78,17 +78,17 @@
         is(transaction.objectStore("foo").name, "foo", "Can't get stores");
         ok(false, "Should have thrown");
       }
       catch (e) {
         ok(true, "Out of scope transaction can't make stores");
       }
 
       is(objectStore.name, "foo", "Correct name");
-      is(objectStore.keyPath, "", "Correct keyPath");
+      is(objectStore.keyPath, null, "Correct keyPath");
 
       is(objectStore.indexNames.length, 1, "Correct indexNames length");
       is(objectStore.indexNames[0], "fooindex", "Correct indexNames name");
 
       try {
         objectStore.add({});
         ok(false, "Should have thrown");
       }