PYCBC-1538: Get should not fail with InvalidArgumentException when projecting on more than 16 fields

Motivation
==========
The RFC states that once the 16 field limit is reached the SDK should fall back to a full document fetch. Currently the Python SDK is raising an InvalidArgumentException. The C++ core already has the correct logic, so only the check for the number of fields in the projection needs to be removed

Modifications
=============
Remove the check for the number of fields in the projection
Update test_project_too_many_projections for all APIs

Results
=======
All tests pass

Change-Id: Ie9ece154d7269908f0b47520fc4d0fda34b76693
Reviewed-on: https://review.couchbase.org/c/couchbase-python-client/+/200237
Tested-by: Build Bot <[email protected]>
Reviewed-by: Jared Casey <[email protected]>

Author: DemetrisChr

acouchbase/tests/collection_t.py

@@ -39,6 +39,7 @@
                               GetReplicaResult,
                               GetResult,
                               MutationResult)
+from tests.mock_server import MockServerType
 
 from ._test_utils import (CollectionType,
                           KVPair,
@@ -133,6 +134,11 @@ def check_multi_node(self, num_nodes):
         if num_nodes == 1:
             pytest.skip("Test only for clusters with more than a single node.")
 
+    @pytest.fixture(scope="class")
+    def skip_if_go_caves(self, cb_env):
+        if cb_env.is_mock_server and cb_env.mock_server_type == MockServerType.GoCAVES:
+            pytest.skip("GoCAVES does not like this operation.")
+
     @pytest.mark.asyncio
     async def test_exists(self, cb_env, default_kvp):
         cb = cb_env.collection
@@ -240,16 +246,29 @@ async def test_project_project_not_list(self, cb_env, default_kvp):
         with pytest.raises(InvalidArgumentException):
             await cb.get(key, GetOptions(project="thiswontwork"))
 
+    @pytest.mark.usefixtures('skip_if_go_caves')
     @pytest.mark.asyncio
     async def test_project_too_many_projections(self, cb_env, default_kvp):
         cb = cb_env.collection
         key = default_kvp.key
-        project = []
-        for _ in range(17):
-            project.append("something")
+        value = {f'f{i}': i for i in range(1, 21)}
+        result = await cb.upsert(key, value, UpsertOptions(expiry=timedelta(seconds=2)))
 
-        with pytest.raises(InvalidArgumentException):
-            await cb.get(key, GetOptions(project=project))
+        async def cas_matches(cb, new_cas):
+            r = await cb.get(key)
+            if new_cas != r.cas:
+                raise Exception(f"{new_cas} != {r.cas}")
+
+        await cb_env.try_n_times(10, 3, cas_matches, cb, result.cas)
+
+        project = [f'f{i}' for i in range(1, 19)]
+        result = await cb.get(key, GetOptions(project=project))
+        assert result.cas is not None
+        res_dict = result.content_as[dict]
+        assert res_dict != {}
+        assert res_dict.get('f20') is None
+        for field in project:
+            assert res_dict.get(field) is not None
 
     @pytest.mark.asyncio
     async def test_upsert(self, cb_env, default_kvp):

couchbase/logic/collection.py

@@ -135,15 +135,8 @@ class :class:`~couchbase.logic.CollectionLogic`.
                 overrride provided :class:`~.options.GetOptions`
 
         Raises:
-            :class:`~.exceptions.InvalidArgumentException`: When attempting a get projection
-                operation with more than 16 projections.
             :class:`~.exceptions.DocumentNotFoundException`: If the provided document key does not exist.
         """
-        projections = kwargs.get("project")
-        if isinstance(projections, list) and len(projections) > 16:
-            raise InvalidArgumentException(
-                f"Maximum of 16 projects allowed. Provided {len(projections)}"
-            )
         op_type = operations.GET.value
         return kv_operation(**self._get_connection_args(),
                             key=key,

couchbase/tests/collection_t.py

@@ -124,6 +124,11 @@ def check_multi_node(self, num_nodes):
         if num_nodes == 1:
             pytest.skip("Test only for clusters with more than a single node.")
 
+    @pytest.fixture(scope="class")
+    def skip_if_go_caves(self, cb_env):
+        if cb_env.is_mock_server and cb_env.mock_server_type == MockServerType.GoCAVES:
+            pytest.skip("GoCAVES does not like this operation.")
+
     @pytest.mark.usefixtures('check_xattr_supported')
     @pytest.mark.parametrize("expiry", [FIFTY_YEARS + 1,
                                         FIFTY_YEARS,
@@ -389,14 +394,21 @@ def test_project_project_not_list(self, cb_env):
         with pytest.raises(InvalidArgumentException):
             cb_env.collection.get(key, GetOptions(project='thiswontwork'))
 
+    @pytest.mark.usefixtures('skip_if_go_caves')
     def test_project_too_many_projections(self, cb_env):
-        key = cb_env.get_existing_doc(key_only=True)
-        project = []
-        for _ in range(17):
-            project.append('something')
+        key = cb_env.get_new_doc(key_only=True)
+        value = {f'f{i}': i for i in range(1, 21)}
+        cb_env.collection.upsert(key, value)
 
-        with pytest.raises(InvalidArgumentException):
-            cb_env.collection.get(key, GetOptions(project=project))
+        project = [f'f{i}' for i in range(1, 19)]
+        result = cb_env.collection.get(key, GetOptions(project=project))
+
+        assert result.cas is not None
+        res_dict = result.content_as[dict]
+        assert res_dict != {}
+        assert res_dict.get('f20') is None
+        for field in project:
+            assert res_dict.get(field) is not None
 
     def test_remove(self, cb_env):
         key = cb_env.get_existing_doc(key_only=True)

txcouchbase/tests/collection_t.py

@@ -41,6 +41,7 @@
                               GetReplicaResult,
                               GetResult,
                               MutationResult)
+from tests.mock_server import MockServerType
 
 from ._test_utils import (CollectionType,
                           KVPair,
@@ -113,6 +114,11 @@ def check_replicas(self, cb_env):
         if kv_endpoints is None or len(kv_endpoints) < (num_replicas + 1):
             pytest.skip("Not all replicas are online")
 
+    @pytest.fixture(scope="class")
+    def skip_if_go_caves(self, cb_env):
+        if cb_env.is_mock_server and cb_env.mock_server_type == MockServerType.GoCAVES:
+            pytest.skip("GoCAVES does not like this operation.")
+
     @pytest.fixture(scope="class")
     def num_replicas(self, cb_env):
         bucket_settings = cb_env.try_n_times(10, 1, cb_env.bm.get_bucket, cb_env.bucket.name)
@@ -231,17 +237,33 @@ def test_project_project_not_list(self, cb_env, default_kvp):
                                   key,
                                   GetOptions(project="thiswontwork"))
 
+    @pytest.mark.usefixtures('skip_if_go_caves')
     def test_project_too_many_projections(self, cb_env, default_kvp):
         cb = cb_env.collection
         key = default_kvp.key
-        project = []
-        for _ in range(17):
-            project.append("something")
+        value = {f'f{i}': i for i in range(1, 21)}
+        result = run_in_reactor_thread(cb.upsert,
+                                       key,
+                                       value,
+                                       UpsertOptions(expiry=timedelta(seconds=2)))
 
-        with pytest.raises(InvalidArgumentException):
-            run_in_reactor_thread(cb.get,
-                                  key,
-                                  GetOptions(project=project))
+        def cas_matches(cb, new_cas):
+            r = run_in_reactor_thread(cb.get, key)
+            if new_cas != r.cas:
+                raise Exception(f"{new_cas} != {r.cas}")
+
+        print('waiting for cas matches')
+        cb_env.try_n_times(10, 3, cas_matches, cb, result.cas, is_deferred=False)
+
+        project = [f'f{i}' for i in range(1, 19)]
+        result = run_in_reactor_thread(cb.get, key, GetOptions(project=project))
+
+        assert result.cas is not None
+        res_dict = result.content_as[dict]
+        assert res_dict != {}
+        assert res_dict.get('f20') is None
+        for field in project:
+            assert res_dict.get(field) is not None
 
     def test_upsert(self, cb_env, default_kvp):
         cb = cb_env.collection