PYCBC-1753: Client is not passing scope_name or bucket_name when using scope search indexes

Changes
-------
* Update scope implementations to add scope_name and bucket_name to
  request (these params are then passed into the C++ core).
* Fix tests to properly load a scope-level search index and test
  scope-level search queries.
* Update search tests to test both search_query (deprecated) and
  search().
* Update tracing/metrics streaming tests to verify bucket and scope are
  present when expected.

Change-Id: I5988f8f7f528259c642a9f4d96a6c0b084f7fd0f
Reviewed-on: https://review.couchbase.org/c/couchbase-python-client/+/243397
Reviewed-by: Dimitris Christodoulou <dimitris.christodoulou@couchbase.com>
Tested-by: Build Bot <build@couchbase.com>

Author: thejcfactor

acouchbase/logic/scope_impl.py

@@ -155,6 +155,8 @@ def search(self, req: SearchQueryRequest) -> SearchResult:
                                                                                default_serializer=self.default_serializer,  # noqa: E501
                                                                                streaming_timeout=streaming_timeout,
                                                                                obs_handler=req.obs_handler,
+                                                                               bucket_name=req.bucket_name,
+                                                                               scope_name=req.scope_name,
                                                                                num_workers=req.num_workers))
 
     async def wait_until_bucket_connected(self) -> None:

acouchbase/scope.py

@@ -435,7 +435,8 @@ def search_indexes(self) -> ScopeSearchIndexManager:
 
         """  # noqa: E501
         return ScopeSearchIndexManager(self._impl._client_adapter,
-                                       self.bucket_name, self.name,
+                                       self.bucket_name,
+                                       self.name,
                                        self._impl.observability_instruments)
 
     def eventing_functions(self) -> ScopeEventingFunctionManager:

acouchbase/tests/metrics_tests/streaming_t.py

@@ -182,9 +182,10 @@ async def test_http_search_query_op(self, acb_env: AsyncMetricsEnvironment) -> N
                                                         acb_env.mock_server_type):
             return  # skip rest of test if collections not supported
 
-        # TODO(PYCBC-1753): The bucket and scope name are not passed when doing scope-level search queries.
-        #                   This means we cannot validate the span attributes have db.namespace and couchbase.scope.name
-        validator.reset(op_name=OpName.SearchQuery, validate_error=True)
+        validator.reset(op_name=OpName.SearchQuery,
+                        bucket_name=acb_env.bucket.name,
+                        scope_name=acb_env.scope.name,
+                        validate_error=True)
         try:
             [r async for r in acb_env.scope.search('not-an-index', TermQuery('auto')).rows()]
         except Exception:

acouchbase/tests/search_t.py

@@ -38,52 +38,42 @@
 
 class SearchCollectionTestSuite:
     TEST_MANIFEST = [
-        'test_cluster_query_collections',
         'test_scope_query_collections',
         'test_scope_search_fields',
         'test_scope_search_highlight',
     ]
 
+    @pytest.mark.parametrize('query_op', ['search_query', 'search'])
     @pytest.mark.asyncio
-    async def test_cluster_query_collections(self, cb_env):
+    async def test_scope_query_collections(self, cb_env, query_op):
         q = search.TermQuery('auto')
-        res = cb_env.cluster.search_query(cb_env.TEST_COLLECTION_INDEX_NAME,
-                                          q,
-                                          SearchOptions(limit=10,
-                                                        scope_name=cb_env.scope.name,
-                                                        collections=[cb_env.collection.name]))
+        operation = getattr(cb_env.scope, query_op)
+        res = operation(cb_env.TEST_COLLECTION_INDEX_NAME,
+                        q,
+                        SearchOptions(limit=10, collections=[cb_env.collection.name]))
         rows = await cb_env.assert_rows(res, 2, return_rows=True)
 
         collections = list(map(lambda r: r.fields['_$c'], rows))
         assert all([c for c in collections if c == cb_env.collection.name]) is True
 
-    @pytest.mark.asyncio
-    async def test_scope_query_collections(self, cb_env):
-        q = search.TermQuery('auto')
-        res = cb_env.scope.search_query(cb_env.TEST_COLLECTION_INDEX_NAME,
-                                        q,
-                                        SearchOptions(limit=10, collections=[cb_env.collection.name]))
-        rows = await cb_env.assert_rows(res, 2, return_rows=True)
-
-        collections = list(map(lambda r: r.fields['_$c'], rows))
-        assert all([c for c in collections if c == cb_env.collection.name]) is True
-
-        res = cb_env.scope.search_query(cb_env.TEST_COLLECTION_INDEX_NAME, q, SearchOptions(limit=10))
+        res = operation(cb_env.TEST_COLLECTION_INDEX_NAME, q, SearchOptions(limit=10))
         rows = await cb_env.assert_rows(res, 2, return_rows=True)
 
         collections = list(map(lambda r: r.fields['_$c'], rows))
         assert all([c for c in collections if c in [cb_env.collection.name, cb_env.OTHER_COLLECTION]]) is True
 
+    @pytest.mark.parametrize('query_op', ['search_query', 'search'])
     @pytest.mark.asyncio
-    async def test_scope_search_fields(self, cb_env):
+    async def test_scope_search_fields(self, cb_env, query_op):
         test_fields = ['make', 'model']
+        operation = getattr(cb_env.scope, query_op)
         q = search.TermQuery('auto')
         # verify fields works w/in kwargs
-        res = cb_env.scope.search_query(cb_env.TEST_COLLECTION_INDEX_NAME,
-                                        q,
-                                        SearchOptions(limit=10),
-                                        fields=test_fields,
-                                        collections=[cb_env.collection.name])
+        res = operation(cb_env.TEST_COLLECTION_INDEX_NAME,
+                        q,
+                        SearchOptions(limit=10),
+                        fields=test_fields,
+                        collections=[cb_env.collection.name])
 
         fields_with_col = copy(test_fields)
         fields_with_col.append('_$c')
@@ -97,11 +87,11 @@ async def test_scope_search_fields(self, cb_env):
         assert all([c for c in collections if c == cb_env.collection.name]) is True
 
         # verify fields works w/in options
-        res = cb_env.scope.search_query(cb_env.TEST_COLLECTION_INDEX_NAME,
-                                        q,
-                                        SearchOptions(limit=10,
-                                                      fields=test_fields,
-                                                      collections=[cb_env.collection.name]))
+        res = operation(cb_env.TEST_COLLECTION_INDEX_NAME,
+                        q,
+                        SearchOptions(limit=10,
+                                      fields=test_fields,
+                                      collections=[cb_env.collection.name]))
 
         rows = await cb_env.assert_rows(res, 1, return_rows=True)
         first_entry = rows[0]
@@ -112,14 +102,15 @@ async def test_scope_search_fields(self, cb_env):
         collections = list(map(lambda r: r.fields['_$c'], rows))
         assert all([c for c in collections if c == cb_env.collection.name]) is True
 
+    @pytest.mark.parametrize('query_op', ['search_query', 'search'])
     @pytest.mark.asyncio
-    async def test_scope_search_highlight(self, cb_env):
-
+    async def test_scope_search_highlight(self, cb_env, query_op):
         q = search.TermQuery('auto')
+        operation = getattr(cb_env.scope, query_op)
         # check w/in options
-        res = cb_env.scope.search_query(cb_env.TEST_COLLECTION_INDEX_NAME,
-                                        q,
-                                        SearchOptions(limit=10, highlight_style=HighlightStyle.Html))
+        res = operation(cb_env.TEST_COLLECTION_INDEX_NAME,
+                        q,
+                        SearchOptions(limit=10, highlight_style=HighlightStyle.Html))
         rows = await cb_env.assert_rows(res, 1, return_rows=True)
         locations = rows[0].locations
         fragments = rows[0].fragments
@@ -130,8 +121,11 @@ async def test_scope_search_highlight(self, cb_env):
         assert all([c for c in collections if c == cb_env.collection.name]) is True
 
         # check w/in kwargs
-        res = cb_env.scope.search_query(cb_env.TEST_COLLECTION_INDEX_NAME, q, SearchOptions(
-            limit=10), highlight_style=HighlightStyle.Html, collections=[cb_env.collection.name])
+        res = operation(cb_env.TEST_COLLECTION_INDEX_NAME,
+                        q,
+                        SearchOptions(limit=10),
+                        highlight_style=HighlightStyle.Html,
+                        collections=[cb_env.collection.name])
         rows = await cb_env.assert_rows(res, 1, return_rows=True)
         locations = rows[0].locations
         fragments = rows[0].fragments
@@ -753,7 +747,7 @@ async def couchbase_test_environment(self, acb_base_env, test_manifest_validated
             pytest.fail(f'Test manifest not validated.  Missing tests: {test_manifest_validated}.')
 
         acb_env = AsyncSearchTestEnvironment.from_environment(acb_base_env)
-        acb_env.enable_search_mgmt()
+        # acb_env.enable_search_mgmt()
         await acb_env.setup(request.param)
         yield acb_env
         await acb_env.teardown(request.param)
@@ -775,7 +769,7 @@ async def couchbase_test_environment(self, acb_base_env, test_manifest_validated
             pytest.fail(f'Test manifest not validated.  Missing tests: {test_manifest_validated}.')
 
         acb_env = AsyncSearchTestEnvironment.from_environment(acb_base_env)
-        acb_env.enable_search_mgmt()
+        # acb_env.enable_search_mgmt()
         await acb_env.setup(request.param)
         yield acb_env
         await acb_env.teardown(request.param)

acouchbase/tests/tracing_tests/streaming_t.py

@@ -252,9 +252,11 @@ async def test_http_search_query_op(self, acb_env: AsyncBaseTracingEnvironment)
                                                         acb_env.mock_server_type):
             return  # skip rest of test if collections not supported
 
-        # TODO(PYCBC-1753): The bucket and scope name are not passed when doing scope-level search queries.
-        #                   This means we cannot validate the span attributes have db.namespace and couchbase.scope.name
-        validator.reset(op_name=OpName.SearchQuery, clear_statement=True, validate_error=True)
+        validator.reset(op_name=OpName.SearchQuery,
+                        clear_statement=True,
+                        bucket_name=acb_env.bucket.name,
+                        scope_name=acb_env.scope.name,
+                        validate_error=True)
         try:
             [r async for r in acb_env.scope.search('not-an-index', TermQuery('auto')).rows()]
         except Exception:
@@ -263,12 +265,12 @@ async def test_http_search_query_op(self, acb_env: AsyncBaseTracingEnvironment)
 
         acb_env.tracer.clear_spans()
         parent_span = acb_env.tracer.request_span(f'parent_{OpName.SearchQuery.value}_span')
-        # TODO(PYCBC-1753): The bucket and scope name are not passed when doing scope-level search queries.
-        #                   This means we cannot validate the span attributes have db.namespace and couchbase.scope.name
         validator.reset(do_not_clear_spans=True,
                         op_name=OpName.SearchQuery,
                         parent_span=parent_span,
                         clear_statement=True,
+                        bucket_name=acb_env.bucket.name,
+                        scope_name=acb_env.scope.name,
                         validate_error=True)
         try:
             [r async for r in acb_env.scope.search('not-an-index',

couchbase/logic/scope_impl.py

@@ -137,4 +137,6 @@ def search(self, req: SearchQueryRequest) -> SearchResult:
                                                                           req.query_builder.as_encodable(),
                                                                           default_serializer=self.default_serializer,
                                                                           streaming_timeout=streaming_timeout,
-                                                                          obs_handler=req.obs_handler))
+                                                                          obs_handler=req.obs_handler,
+                                                                          bucket_name=req.bucket_name,
+                                                                          scope_name=req.scope_name))

couchbase/logic/scope_req_builder.py

@@ -23,9 +23,7 @@
                                          QueryRequest,
                                          SearchQueryRequest)
 from couchbase.n1ql import N1QLQuery
-from couchbase.options import (AnalyticsOptions,
-                               QueryOptions,
-                               SearchOptions)
+from couchbase.options import AnalyticsOptions, QueryOptions
 from couchbase.search import (SearchQuery,
                               SearchQueryBuilder,
                               SearchRequest)
@@ -90,22 +88,14 @@ def build_search_request(self,
                              *options: object,
                              **kwargs: object) -> SearchQueryRequest:
         num_workers = kwargs.pop('num_workers', None)
+
         if isinstance(query, SearchQuery):
-            opt = SearchOptions()
-            opts = list(options)
-            for o in opts:
-                if isinstance(o, SearchOptions):
-                    opt = o
-                    opts.remove(o)
-
-            # set the scope_name as this scope if not provided
-            if not ('scope_name' in opt or 'scope_name' in kwargs):
-                kwargs['scope_name'] = f'{self._scope_name}'
-            req = SearchQueryRequest(
-                SearchQueryBuilder.create_search_query_object(index, query, *options, **kwargs), obs_handler)
+            query_builder = SearchQueryBuilder.create_search_query_object(index, query, *options, **kwargs)
         else:
-            req = SearchQueryRequest(
-                SearchQueryBuilder.create_search_query_from_request(index, query, *options, **kwargs), obs_handler)
+            query_builder = SearchQueryBuilder.create_search_query_from_request(index, query, *options, **kwargs)
+
+        scope_name = query_builder.scope_name if query_builder.scope_name else self._scope_name
+        req = SearchQueryRequest(query_builder, obs_handler, self._bucket_name, scope_name)
 
         # since query is lazy executed, we wait until we submit the query to create the span
         if num_workers:

couchbase/logic/scope_types.py

@@ -53,6 +53,8 @@ def op_name(self) -> str:
 class SearchQueryRequest:
     query_builder: SearchQueryBuilder
     obs_handler: ObservableRequestHandler
+    bucket_name: str
+    scope_name: str
     num_workers: Optional[int] = None
 
     @property

couchbase/tests/metrics_tests/streaming_t.py

@@ -178,9 +178,10 @@ def test_http_search_query_op(self, cb_env: MetricsEnvironment) -> None:
                                                         cb_env.mock_server_type):
             return  # skip rest of test if collections not supported
 
-        # TODO(PYCBC-1753): The bucket and scope name are not passed when doing scope-level search queries.
-        #                   This means we cannot validate the span attributes have db.namespace and couchbase.scope.name
-        validator.reset(op_name=OpName.SearchQuery, validate_error=True)
+        validator.reset(op_name=OpName.SearchQuery,
+                        bucket_name=cb_env.bucket.name,
+                        scope_name=cb_env.scope.name,
+                        validate_error=True)
         try:
             [r for r in cb_env.scope.search('not-an-index', TermQuery('auto')).rows()]
         except Exception: