DRIVERS-2630: add e2e testing for search index commands and clarifica…

…tions to search index spec (#1442)

source/client-side-operations-timeout/client-side-operations-timeout.rst

@@ -131,7 +131,7 @@ specifications:
 - `Enumerating Collections <../enumerate-collections.rst>`__
 - `Enumerating Databases <../enumerate-databases.rst>`__
 - `GridFS <../gridfs/gridfs-spec.rst>`__
-- `Index Management <../index-management.rst>`__
+- `Index Management <../index-management/index-management.rst>`__
 - `Transactions <../transactions/transactions.rst>`__
 - `Convenient API for Transactions <../transactions-convenient-api/transactions-convenient-api.rst>`__
 

source/index-management/index-management.rst

@@ -414,6 +414,10 @@ Index View API
      * For drivers that cannot make IndexView iterable, they MUST implement this method to
      * return a list of indexes. In the case of async drivers, this MAY return a Future<Cursor>
      *  or language/implementation equivalent.
+     * 
+     *  If drivers are unable to make the IndexView iterable, they MAY opt to provide the options for 
+     *  listing search indexes via the `list` method instead of the `Collection.indexes` method.
+
      */
     list(): Cursor;
 
@@ -873,8 +877,8 @@ search index management helpers.
 options as outline in the `CRUD specification <https://github.com/mongodb/specifications/blob/master/source/crud/crud.rst#read>`_.  Drivers MAY combine the aggregation options with
 any future ``listSearchIndexes`` stage options, if that is idiomatic for a driver's language.
 
-Notes
------
+Asynchronicity
+--------------
 
 The search index commands are asynchronous and return from the server before the index is successfully updated, created or dropped.
 In order to determine when an index has been created / updated, users are expected to run the ``listSearchIndexes`` repeatedly
@@ -888,7 +892,34 @@ An example, from Javascript:
   while (!(await collection.listSearchIndexes({ name }).hasNext())) {
     await setTimeout(1000);
   }
- 
+
+Where are read concern and write concern?
+-----------------------------------------
+
+These commands internally proxy the search index management commands to a separate process that runs alongside an Atlas cluster.  As such, read concern and 
+write concern are not relevant for the search index management commands.
+
+Consistency with Existing APIs
+------------------------------
+
+Drivers SHOULD strive for a search index management API that is as consistent as possible with their existing index management API.
+
+NamespaceNotFound Errors
+------------------------
+
+Some drivers suppress NamespaceNotFound errors for CRUD helpers.  Drivers MAY suppress NamespaceNotFound errors from 
+the search index management helpers.
+
+Drivers MUST suppress NamespaceNotFound errors for the ``dropSearchIndex`` helper.  Drop operations should be idempotent:
+
+.. code:: typescript
+
+  await collection.dropSearchIndex('my-test-index');
+  // subsequent calls should behave the same for the user as the first call
+  await collection.dropSearchIndex('my-test-index');
+  await collection.dropSearchIndex('my-test-index');
+
+
 Common Interfaces
 -----------------
 
@@ -902,6 +933,11 @@ Common Interfaces
     name: Optional<string>;
   }
 
+  interface SearchIndexOptions {
+    // The name for this index, if present.
+    name: Optional<string>;
+  }
+
   /**
    * The following interfaces are empty but are provided as placeholders for drivers that cannot 
    * add options in a non-breaking manner, if options are added in the future.
@@ -925,8 +961,10 @@ Standard API for Search Indexes
      * @note Drivers MAY opt to implement this method signature, the signature that
      *   takes an SearchIndexModel as a parameter, or for those languages with method
      *   overloading MAY decide to implement both.
+     *   
+     * @note Drivers MAY combine the `indexOptions` with the `createSearchIndexOptions`, if that is idiomatic for their language.
      */
-    createSearchIndex(name: String, definition: Document, options: Optional<CreateSearchIndexOptions>): String;
+    createSearchIndex(definition: Document, indexOptions: Optional<SearchIndexOptions>, createSearchIndexOptions: Optional<CreateSearchIndexOptions>): String;
 
     /**
      * Convenience method for creating a single index.
@@ -974,7 +1012,7 @@ Index View API for Search Indexes
     /**
      * Returns the search index view for this collection.
      */
-    searchIndexes(name: Optional<String>, aggregateOptions: Optional<AggregationOptions>, options: Optional<ListSearchIndexesOptions>): SearchIndexView;
+    searchIndexes(name: Optional<String>, aggregateOptions: Optional<AggregationOptions>, options: Optional<ListSearchIndexOptions>): SearchIndexView;
   }
 
   interface SearchIndexView extends Iterable<Document> {
@@ -990,6 +1028,9 @@ Index View API for Search Indexes
      * For drivers that cannot make SearchIndexView iterable, they MUST implement this method to
      * return a list of indexes. In the case of async drivers, this MAY return a Future<Cursor>
      *  or language/implementation equivalent.
+     *  
+     *  If drivers are unable to make the SearchIndexView iterable, they MAY opt to provide the options for 
+     *  listing search indexes via the `list` method instead of the `Collection.searchIndexes` method.
      */
     list(): Cursor<Document>;
 
@@ -1002,8 +1043,10 @@ Index View API for Search Indexes
      * @note Drivers MAY opt to implement this method signature, the signature that
      *   takes an SearchIndexModel as a parameter, or for those languages with method
      *   overloading MAY decide to implement both.
+     *   
+     * @note Drivers MAY combine the `indexOptions` with the `createSearchIndexOptions`, if that is idiomatic for their language.
      */
-    createOne(name: String, definition: Document, options: Optional<CreateSearchIndexOptions>): String;
+    createOne(definition: Document, indexOptions: Optional<SearchIndexOptions>, createSearchIndexOptions: Optional<CreateSearchIndexOptions>): String;
 
     /**
      * This is a convenience method for creating a single index.
@@ -1031,7 +1074,7 @@ Index View API for Search Indexes
     /**
      * Updates a single search index from the collection by the index name.
      */
-    updateOne(name: String, options: Optional<UpdateSearchIndexOptions>): Result;
+    updateOne(name: String, definition: Document, options: Optional<UpdateSearchIndexOptions>): Result;
   }
 
 ---------
@@ -1093,3 +1136,4 @@ Changelog
 :2023-05-10:  Merge index enumeration and index management specs and get rid of references 
              to legacy server versions.
 :2023-05-18:  Add the search index management API.
+:2023-07-27:  Add search index management clarifications.

source/index-management/tests/README.rst

@@ -31,7 +31,6 @@ For each of the configurations:
   indicated indexes act on
 
 Tests
------
 
 - Run the driver's method that returns a list of index names, and:
 
@@ -45,4 +44,150 @@ Tests
   - verify the "unique" flags show up for the unique index
   - verify there are no duplicates in the returned list
   - if the result consists of statically defined index models that include an ``ns`` field, verify
-    that its value is accurate
+    that its value is accurate
+
+Search Index Management Helpers
+-------------------------------
+
+These tests are intended to smoke test the search management helpers end-to-end against a live Atlas cluster.
+
+The search index management commands are asynchronous and mongod/mongos returns before the changes to a clusters' search indexes have completed.  When
+these prose tests specify "waiting for the changes", drivers should repeatedly poll the cluster with ``listSearchIndexes``
+until the changes are visible.  Each test specifies the condition that is considered "ready".  For example, when creating a 
+new search index, waiting until the inserted index has a status ``queryable: true`` indicates that the index was successfully
+created.
+
+The commands tested in these prose tests take a while to successfully complete.  Drivers should raise the timeout for each test to avoid timeout errors if 
+the test timeout is too low.  5 minutes is a sufficiently large timeout that any timeout that occurs indicates a real failure, but this value is not required and can be tweaked per-driver.
+
+There is a server-side limitation that prevents multiple search indexes from being created with the same name, definition and 
+collection name.  This limitation does not take into account collection uuid.  Because these commands are asynchronous, any cleanup
+code that may run after a test (cleaning a database or dropping search indexes) may not have completed by the next iteration of the 
+test (or the next test run, if running locally).  To address this issue, each test uses a randomly generated collection name.  Drivers
+may generate this collection name however they like, but a suggested implementation is a hex representation of an
+ObjectId (``new ObjectId().toHexString()`` in Node).
+
+Setup
+~~~~~
+
+These tests must run against an Atlas cluster with a 7.0+ server.  `Scripts are available <https://github.com/mongodb-labs/drivers-evergreen-tools/tree/master/.evergreen/atlas>`_ in drivers-evergreen-tools which can setup and teardown
+Atlas clusters.  To ensure that the Atlas cluster is cleaned up after each CI run, drivers should configure evergreen to run these tests 
+as a part of a task group.  Be sure that the cluster gets torn down! 
+
+When working locally on these tests, the same Atlas setup and teardown scripts can be used locally to provision a cluster for development.
+
+Case 1: Driver can successfully create and list search indexes
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+#. Create a collection with the "create" command using a randomly generated name (referred to as ``coll0``).
+#. Create a new search index on ``coll0`` with the ``createSearchIndex`` helper.  Use the following definition:
+
+  .. code:: typescript
+
+    {
+      name: 'test-search-index',
+      definition: {
+        mappings: { dynamic: false }
+      }
+    }
+
+#. Assert that the command returns the name of the index: ``"test-search-index"``.
+#. Run ``coll0.listSearchIndexes()`` repeatedly every 5 seconds until the following condition is satisfied and store the value in a variable ``index``:
+   1. An index with the ``name`` of ``test-search-index`` is present and the index has a field ``queryable`` with a value of ``true``.
+
+#. Assert that ``index`` has a property ``latestDefinition`` whose value is ``{ 'mappings': { 'dynamic': false } }``
+
+Case 2: Driver can successfully create multiple indexes in batch
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+#. Create a collection with the "create" command using a randomly generated name (referred to as ``coll0``).
+#. Create two new search indexes on ``coll0`` with the ``createSearchIndexes`` helper.  Use the following
+   definitions when creating the indexes.  These definitions are referred to as ``indexDefinitions``.
+
+  .. code:: typescript
+
+    {
+      name: 'test-search-index-1',
+      definition: {
+        mappings: { dynamic: false }
+      }
+    }
+    {
+      name: 'test-search-index-2',
+      definition: {
+        mappings: { dynamic: false }
+      }
+    }
+
+#. Assert that the command returns an array containing the new indexes' names: ``["test-search-index-1", "test-search-index-2"]``.
+#. Run ``coll0.listSearchIndexes()`` repeatedly every 5 seconds until the following condition is satisfied.
+   1. An index with the ``name`` of ``test-search-index-1`` is present and index has a field ``queryable`` with the value of ``true``. Store result in ``index1``.
+   2. An index with the ``name`` of ``test-search-index-2`` is present and index has a field ``queryable`` with the value of ``true``. Store result in ``index2``.
+#. Assert that ``index1`` and ``index2`` have the property ``latestDefinition`` whose value is ``{ "mappings" : { "dynamic" : false } }``
+Case 3: Driver can successfully drop search indexes
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+#. Create a collection with the "create" command using a randomly generated name (referred to as ``coll0``).
+#. Create a new search index on ``coll0`` with the following definition:
+
+  .. code:: typescript
+
+    {
+      name: 'test-search-index',
+      definition: {
+        mappings: { dynamic: false }
+      }
+    }
+
+#. Assert that the command returns the name of the index: ``"test-search-index"``.
+#. Run ``coll0.listSearchIndexes()`` repeatedly every 5 seconds until the following condition is satisfied:
+   1. An index with the ``name`` of ``test-search-index`` is present and index has a field ``queryable`` with the value of ``true``.
+
+#. Run a ``dropSearchIndex`` on ``coll0``, using ``test-search-index`` for the name.
+#. Run ``coll0.listSearchIndexes()`` repeatedly every 5 seconds until ``listSearchIndexes`` returns an empty array.
+
+This test fails if it times out waiting for the deletion to succeed.
+
+Case 4: Driver can update a search index
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+#. Create a collection with the "create" command using a randomly generated name (referred to as ``coll0``).
+#. Create a new search index on ``coll0`` with the following definition:
+
+  .. code:: typescript
+
+    {
+      name: 'test-search-index',
+      definition: {
+        mappings: { dynamic: false }
+      }
+    }
+
+#. Assert that the command returns the name of the index: ``"test-search-index"``.
+#. Run ``coll0.listSearchIndexes()`` repeatedly every 5 seconds until the following condition is satisfied:
+   1. An index with the ``name`` of ``test-search-index`` is present and index has a field ``queryable`` with the value of ``true``.
+
+#. Run a ``updateSearchIndex`` on ``coll0``, using the following definition.
+
+  .. code:: typescript
+
+    {
+      name: 'test-search-index',
+      definition: {
+        mappings: { dynamic: true }
+      }
+    }
+
+#. Assert that the command does not error and the server responds with a success.
+#. Run ``coll0.listSearchIndexes()`` repeatedly every 5 seconds until the following condition is satisfied:
+   1. An index with the ``name`` of ``test-search-index`` is present.  This index is referred to as ``index``.
+   2. The index has a field ``queryable`` with a value of ``true`` and has a field ``status`` with the value of ``READY``.
+
+#. Assert that an index is present with the name ``test-search-index`` and the definition has a
+  property ``latestDefinition`` whose value is ``{ 'mappings': { 'dynamic': true } }``.
+
+Case 5: ``dropSearchIndex`` suppresses namespace not found errors
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+#. Create a driver-side collection object for a randomly generated collection name.  Do not create this collection on the server.
+#. Run a ``dropSearchIndex`` command and assert that no error is thrown.

source/index-management/tests/createSearchIndex.json

@@ -54,7 +54,8 @@
             }
           },
           "expectError": {
-            "isError": true
+            "isError": true,
+            "errorContains": "Search index commands are only supported with Atlas"
           }
         }
       ],
@@ -100,7 +101,8 @@
             }
           },
           "expectError": {
-            "isError": true
+            "isError": true,
+            "errorContains": "Search index commands are only supported with Atlas"
           }
         }
       ],

source/index-management/tests/createSearchIndex.yml

@@ -28,9 +28,10 @@ tests:
         arguments:
           model: { definition: &definition { mappings: { dynamic: true } } }
         expectError:
-          # Search indexes are only available on 7.0+ atlas clusters.  DRIVERS-2630 will add e2e testing 
-          # against an Atlas cluster and the expectError will be removed.
+          # This test always errors in a non-Atlas environment.  The test functions as a unit test  by asserting
+          # that the driver constructs and sends the correct command.
           isError: true
+          errorContains: Search index commands are only supported with Atlas
     expectEvents:
       - client: *client0
         events:
@@ -47,9 +48,10 @@ tests:
         arguments: 
           model: { definition: &definition { mappings: { dynamic: true } } , name: 'test index' }
         expectError:
-          # Search indexes are only available on 7.0+ atlas clusters.  DRIVERS-2630 will add e2e testing 
-          # against an Atlas cluster and the expectError will be removed.
+          # This test always errors in a non-Atlas environment.  The test functions as a unit test  by asserting
+          # that the driver constructs and sends the correct command.
           isError: true
+          errorContains: Search index commands are only supported with Atlas
     expectEvents:
       - client: *client0
         events:

source/index-management/tests/createSearchIndexes.json

@@ -48,7 +48,8 @@
             "models": []
           },
           "expectError": {
-            "isError": true
+            "isError": true,
+            "errorContains": "Search index commands are only supported with Atlas"
           }
         }
       ],
@@ -87,7 +88,8 @@
             ]
           },
           "expectError": {
-            "isError": true
+            "isError": true,
+            "errorContains": "Search index commands are only supported with Atlas"
           }
         }
       ],
@@ -135,7 +137,8 @@
             ]
           },
           "expectError": {
-            "isError": true
+            "isError": true,
+            "errorContains": "Search index commands are only supported with Atlas"
           }
         }
       ],