PYCBC-1725: Defer decoding to *Result object

thejcfactor · thejcfactor · commit 8d6e7c062c75 · 2025-11-11T18:30:58.000Z
Motivation ========== In order to more closely align with RFCs, it is preferred to defer decoding results (for applicable operations) to happen within the specific result object (e.g. GetResult) when the application accesses the content from the result object. Modification ============ * Move transcoding logic from decorators to base Result object * Update remaining to *Result objects (where applicable) to handle transcoder accordingly * Update tests to confirm functionality Change-Id: Idb1322a69a354bad9bc9455e77f00b35b4452bca Reviewed-on: https://review.couchbase.org/c/couchbase-python-client/+/236000 Tested-by: Build Bot <build@couchbase.com> Reviewed-by: Dimitris Christodoulou <dimitris.christodoulou@couchbase.com>
diff --git a/acouchbase/logic/wrappers.py b/acouchbase/logic/wrappers.py
@@ -25,7 +25,7 @@
                                   ExceptionMap,
                                   MissingConnectionException,
                                   ServiceUnavailableException)
-from couchbase.logic import decode_replicas, decode_value
+from couchbase.logic import decode_replicas
 
 
 def call_async_fn(ft, self, fn, *args, **kwargs):
@@ -286,17 +286,12 @@ def on_ok(res):
                             )
                             return
 
-                        value = res.raw_result.get('value', None)
-                        flags = res.raw_result.get('flags', None)
-
-                        res.raw_result['value'] = decode_value(transcoder, value, flags, is_subdoc=is_subdoc)
-
                         if return_cls is None:
                             retval = None
                         elif return_cls is True:
                             retval = res
                         else:
-                            retval = return_cls(res)
+                            retval = return_cls(res, transcoder=transcoder, is_subdoc=is_subdoc)
                         self.loop.call_soon_threadsafe(ft.set_result, retval)
                     except CouchbaseException as e:
                         self.loop.call_soon_threadsafe(ft.set_exception, e)
diff --git a/acouchbase/tests/collection_t.py b/acouchbase/tests/collection_t.py
@@ -30,7 +30,6 @@
                                   DocumentNotLockedException,
                                   DocumentUnretrievableException,
                                   InvalidArgumentException,
-                                  PathNotFoundException,
                                   TemporaryFailException)
 from couchbase.options import (GetAllReplicasOptions,
                                GetAnyReplicaOptions,
@@ -245,10 +244,13 @@ async def cas_matches(cb, new_cas):
 
     @pytest.mark.asyncio
     async def test_project_bad_path(self, cb_env, default_kvp):
-        cb = cb_env.collection
         key = default_kvp.key
-        with pytest.raises(PathNotFoundException):
-            await cb.get(key, GetOptions(project=["some", "qzx"]))
+        # CXXCBC-295 - b8bb98c31d377100934dd4b33998f0a118df41e8, bad path no longer raises PathNotFoundException
+        result = await cb_env.collection.get(key, GetOptions(project=['qzx']))
+        assert result.cas is not None
+        res_dict = result.content_as[dict]
+        assert res_dict == {}
+        assert 'qzx' not in res_dict
 
     @pytest.mark.asyncio
     async def test_project_project_not_list(self, cb_env, default_kvp):
diff --git a/couchbase/collection.py b/couchbase/collection.py
@@ -37,9 +37,7 @@
                                   QueueEmpty)
 from couchbase.exceptions import exception as CouchbaseBaseException
 from couchbase.kv_range_scan import RangeScanRequest
-from couchbase.logic import (BlockingWrapper,
-                             decode_replicas,
-                             decode_value)
+from couchbase.logic import BlockingWrapper, decode_replicas
 from couchbase.logic.collection import CollectionLogic
 from couchbase.logic.supportability import Supportability
 from couchbase.management.queries import CollectionQueryIndexManager
@@ -2010,17 +2008,8 @@ def get_multi(self,
             op_type=op_type,
             op_args=op_args
         )
-        for k, v in res.raw_result.items():
-            if k == 'all_okay':
-                continue
-            if isinstance(v, CouchbaseBaseException):
-                continue
-            value = v.raw_result.get('value', None)
-            flags = v.raw_result.get('flags', None)
-            tc = transcoders[k]
-            v.raw_result['value'] = decode_value(tc, value, flags)
 
-        return MultiGetResult(res, return_exceptions)
+        return MultiGetResult(res, return_exceptions, transcoders)
 
     def get_any_replica_multi(self,
                               keys,  # type: List[str]
@@ -2104,17 +2093,8 @@ def get_any_replica_multi(self,
             op_type=op_type,
             op_args=op_args
         )
-        for k, v in res.raw_result.items():
-            if k == 'all_okay':
-                continue
-            if isinstance(v, CouchbaseBaseException):
-                continue
-            value = v.raw_result.get('value', None)
-            flags = v.raw_result.get('flags', None)
-            tc = transcoders[k]
-            v.raw_result['value'] = decode_value(tc, value, flags)
 
-        return MultiGetReplicaResult(res, return_exceptions)
+        return MultiGetReplicaResult(res, return_exceptions, transcoders)
 
     def get_all_replicas_multi(self,
                                keys,  # type: List[str]
@@ -2309,17 +2289,8 @@ def get_and_lock_multi(self,
             op_type=op_type,
             op_args=op_args
         )
-        for k, v in res.raw_result.items():
-            if k == 'all_okay':
-                continue
-            if isinstance(v, CouchbaseBaseException):
-                continue
-            value = v.raw_result.get('value', None)
-            flags = v.raw_result.get('flags', None)
-            tc = transcoders[k]
-            v.raw_result['value'] = decode_value(tc, value, flags)
 
-        return MultiGetResult(res, return_exceptions)
+        return MultiGetResult(res, return_exceptions, transcoders)
 
     def exists_multi(self,
                      keys,  # type: List[str]
diff --git a/couchbase/logic/__init__.py b/couchbase/logic/__init__.py
@@ -15,4 +15,3 @@
 
 from .wrappers import BlockingWrapper  # noqa: F401
 from .wrappers import decode_replicas  # noqa: F401
-from .wrappers import decode_value  # noqa: F401
diff --git a/couchbase/logic/kv_range_scan.py b/couchbase/logic/kv_range_scan.py
@@ -21,7 +21,6 @@
 
 from couchbase.exceptions import ErrorMapper, InvalidArgumentException
 from couchbase.exceptions import exception as CouchbaseBaseException
-from couchbase.logic.wrappers import decode_value
 from couchbase.pycbc_core import kv_range_scan_operation
 from couchbase.result import ScanResult
 
@@ -159,9 +158,4 @@ def _get_next_row(self):
         if isinstance(resp, CouchbaseBaseException):
             raise ErrorMapper.build_exception(resp)
 
-        value = resp.raw_result.get('value', None)
-        flags = resp.raw_result.get('flags', None)
-        if value:
-            resp.raw_result['value'] = decode_value(self.transcoder, value, flags)
-
-        return ScanResult(resp, self._ids_only)
+        return ScanResult(resp, self._ids_only, self.transcoder)
diff --git a/couchbase/logic/wrappers.py b/couchbase/logic/wrappers.py
@@ -15,10 +15,8 @@
 
 from __future__ import annotations
 
-from copy import copy
 from functools import wraps
 
-from couchbase.constants import FMT_JSON
 from couchbase.exceptions import (PYCBC_ERROR_MAP,
                                   CouchbaseException,
                                   DocumentExistsException,
@@ -33,25 +31,6 @@
 from couchbase.exceptions import exception as CouchbaseBaseException
 
 
-def decode_value(transcoder, value, flags, is_subdoc=False):
-    if is_subdoc is False:
-        return transcoder.decode_value(value, flags)
-
-    final_value = []
-    for f in value:
-        if 'value' in f:
-            tmp = copy(f)
-            old = tmp.pop('value', None)
-            if old:
-                # no custom transcoder for subdoc ops, use JSON
-                tmp['value'] = transcoder.decode_value(old, FMT_JSON)
-            final_value.append(tmp)
-        else:
-            final_value.append(f)
-
-    return final_value
-
-
 def decode_replicas(transcoder, result, return_cls, is_subdoc=False):
     while True:
         try:
@@ -66,10 +45,7 @@ def decode_replicas(transcoder, result, return_cls, is_subdoc=False):
             if res is None:
                 return
 
-            value = res.raw_result.get('value', None)
-            flags = res.raw_result.get('flags', None)
-            res.raw_result['value'] = decode_value(transcoder, value, flags, is_subdoc=is_subdoc)
-            yield return_cls(res)
+            yield return_cls(res, transcoder=transcoder, is_subdoc=is_subdoc)
 
 
 class BlockingWrapper:
@@ -123,16 +99,12 @@ def wrapped_fn(self, *args, **kwargs):
                     if fn.__name__ in ['_get_all_replicas_internal', '_lookup_in_all_replicas_internal']:
                         return decode_replicas(transcoder, ret, return_cls, is_subdoc=is_subdoc)
 
-                    value = ret.raw_result.get('value', None)
-                    flags = ret.raw_result.get('flags', None)
-
-                    ret.raw_result['value'] = decode_value(transcoder, value, flags, is_subdoc=is_subdoc)
                     if return_cls is None:
                         return None
                     elif return_cls is True:
                         retval = ret
                     else:
-                        retval = return_cls(ret)
+                        retval = return_cls(ret, transcoder=transcoder, is_subdoc=is_subdoc)
                     return retval
                 except CouchbaseException as e:
                     raise e
diff --git a/couchbase/result.py b/couchbase/result.py
@@ -16,6 +16,7 @@
 from __future__ import annotations
 
 import json
+from copy import copy
 from datetime import datetime
 from typing import (Any,
                     Dict,
@@ -27,6 +28,7 @@
 from acouchbase.n1ql import AsyncN1QLRequest
 from acouchbase.search import AsyncFullTextSearchRequest
 from acouchbase.views import AsyncViewRequest
+from couchbase.constants import FMT_JSON
 from couchbase.diagnostics import (ClusterState,
                                    EndpointDiagnosticsReport,
                                    EndpointPingReport,
@@ -36,22 +38,30 @@
 from couchbase.exceptions import exception as CouchbaseBaseException
 from couchbase.pycbc_core import result
 from couchbase.subdocument import parse_subdocument_content_as, parse_subdocument_exists
+from couchbase.transcoder import Transcoder
 
 
 class Result:
     def __init__(
         self,
-        orig,  # type: result
+        orig,              # type: result
+        transcoder=None,  # type: Optional[Transcoder]
+        is_subdoc=None,  # type: Optional[bool]
     ):
-
         self._orig = orig
+        self._transcoder = transcoder
+        self._decoded_value: Optional[Any] = None
+        self._is_subdoc = is_subdoc if is_subdoc is not None else False
 
     @property
     def value(self) -> Optional[Any]:
         """
             Optional[Any]: The content of the document, if it exists.
         """
-        return self._orig.raw_result.get("value", None)
+        if self._decoded_value is not None:
+            return self._decoded_value
+        self._decode_value()
+        return self._decoded_value
 
     @property
     def cas(self) -> Optional[int]:
@@ -81,6 +91,27 @@ def success(self) -> bool:
         """
         return self.cas != 0
 
+    def _decode_value(self) -> None:
+        if self._transcoder is None:
+            return
+
+        value = self._orig.raw_result.get('value', None)
+        if self._is_subdoc is False:
+            flags = self._orig.raw_result.get('flags', None)
+            self._decoded_value = self._transcoder.decode_value(value, flags)
+        else:
+            self._decoded_value = []
+            for f in value:
+                if 'value' in f:
+                    tmp = copy(f)
+                    old = tmp.pop('value', None)
+                    if old:
+                        # no custom transcoder for subdoc ops, use JSON
+                        tmp['value'] = self._transcoder.decode_value(old, FMT_JSON)
+                    self._decoded_value.append(tmp)
+                else:
+                    self._decoded_value.append(f)
+
 
 class ContentProxy:
     """
@@ -353,7 +384,8 @@ class MultiResult:
     def __init__(self,
                  orig,  # type: result
                  result_type,  # type: Union[GetReplicaResult, GetResult]
-                 return_exceptions  # type: bool
+                 return_exceptions,  # type: bool
+                 transcoders=None  # type: Optional[Dict[str, Transcoder]]
                  ):
         self._orig = orig
         self._all_ok = self._orig.raw_result.pop('all_okay', False)
@@ -369,7 +401,9 @@ def __init__(self,
                 if isinstance(v, list):
                     self._results[k] = v
                 else:
-                    self._results[k] = result_type(v)
+                    if transcoders is None:
+                        raise InvalidArgumentException("Transcoders dictionary must be provided")
+                    self._results[k] = result_type(v, transcoder=transcoders[k])
 
     @property
     def all_ok(self) -> bool:
@@ -394,9 +428,10 @@ def exceptions(self) -> Dict[str, CouchbaseBaseException]:
 class MultiGetReplicaResult(MultiResult):
     def __init__(self,
                  orig,  # type: result
-                 return_exceptions  # type: bool
+                 return_exceptions,  # type: bool
+                 transcoders=None  # type: Optional[Dict[str, Transcoder]]
                  ):
-        super().__init__(orig, GetReplicaResult, return_exceptions)
+        super().__init__(orig, GetReplicaResult, return_exceptions, transcoders)
 
     @property
     def results(self) -> Dict[str, GetReplicaResult]:
@@ -423,9 +458,10 @@ def __repr__(self):
 class MultiGetResult(MultiResult):
     def __init__(self,
                  orig,  # type: result
-                 return_exceptions  # type: bool
+                 return_exceptions,  # type: bool
+                 transcoders  # type: Dict[str, Transcoder]
                  ):
-        super().__init__(orig, GetResult, return_exceptions)
+        super().__init__(orig, GetResult, return_exceptions, transcoders)
 
     @property
     def results(self) -> Dict[str, GetResult]:
@@ -948,8 +984,8 @@ def __init__(
 
 class ScanResult(Result):
 
-    def __init__(self, orig, ids_only):
-        super().__init__(orig)
+    def __init__(self, orig, ids_only, transcoder):
+        super().__init__(orig, transcoder=transcoder)
         self._ids_only = ids_only
 
     @property
diff --git a/couchbase/tests/transcoder_t.py b/couchbase/tests/transcoder_t.py
diff --git a/txcouchbase/logic/wrappers.py b/txcouchbase/logic/wrappers.py

Original file line number	Diff line number	Diff line change
`@@ -15,4 +15,3 @@`
`15`	`15`
`16`	`16`	`from .wrappers import BlockingWrapper # noqa: F401`
`17`	`17`	`from .wrappers import decode_replicas # noqa: F401`
`18`		`-from .wrappers import decode_value # noqa: F401`