NCBC-1453: Use JSON transcoder when reading updated bucket config

MOTIVATION ---------- It possible to configure a custom document content transcoder where the transcoder is responsible for encoding and decoding document bodies. However, if a NotMyVBucket response is received from the server, this transcoder is used to try and decode the updated bucket config. This is dangerous because the custom transcoder may not work with JSON and the response will always be JSON. MODIFICATIONS ------------- - Add an overload to IOperation.GetConfig that takes an ITypeTranscoder to be used for decoding the response body - Overload the new method on OperationBase to use the transcoder parameter instead of the content transcoder - Add ServerConfigTranscoder to IClusterController & ClusterController that internall resolves to a DefaultTranscoder - Update all internal usage of GetConfig to use the transcoder in IClusterController - Add unit tests to verify behaviour for both method variants RESULT ------ When a NotMyVBucket response is received, the SDK will use a JSON transcoder to decode the response instead of the custom transcoder. Change-Id: Ic2d0aa63653947070a7b8e18c378ce2b35386d81 Reviewed-on: http://review.couchbase.org/80524 Reviewed-by: Jeffry Morris <jeffrymorris@gmail.com> Tested-by: Build Bot <build@couchbase.com>
couchbase · Jul 20, 2017 · b349839 · b349839
1 parent 8e67747
commit b349839
Show file tree

Hide file tree

Showing 8 changed files with 101 additions and 9 deletions.
diff --git a/Src/Couchbase.UnitTests/IO/Operations/OperationBaseTests.cs b/Src/Couchbase.UnitTests/IO/Operations/OperationBaseTests.cs
@@ -0,0 +1,76 @@
+using Couchbase.Configuration.Server.Serialization;
+using Couchbase.Core.Transcoders;
+using Couchbase.IO;
+using Couchbase.IO.Operations;
+using Moq;
+using NUnit.Framework;
+
+namespace Couchbase.UnitTests.IO.Operations
+{
+    [TestFixture]
+    public class OperationBaseTests
+    {
+        [Test]
+        public void GetConfig_Without_Transcoder_Parameter_Uses_Content_Transcoder()
+        {
+            var config = new BucketConfig();
+            var contentTranscoder = new Mock<ITypeTranscoder>();
+            contentTranscoder
+                .Setup(transcoder => transcoder.Decode<BucketConfig>(It.IsAny<byte[]>(), It.IsAny<int>(),
+                    It.IsAny<int>(), It.IsAny<Flags>(), It.IsAny<OperationCode>()))
+                .Returns(config);
+            var operation = new FakeOperation(contentTranscoder.Object);
+
+            var result = operation.GetConfig();
+            Assert.AreSame(config, result);
+
+            // make sure the transcoder for decoding document content is not used
+            contentTranscoder.Verify(
+                t => t.Decode<BucketConfig>(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Flags>(),
+                    It.IsAny<OperationCode>()), Times.Once);
+        }
+
+        [Test]
+        public void Getconfig_Uses_Paramater_Transcoder()
+        {
+            var contentTranscoder = new Mock<ITypeTranscoder>();
+            var operation = new FakeOperation(contentTranscoder.Object);
+
+            var config = new BucketConfig();
+            var configTranscoder = new Mock<ITypeTranscoder>();
+            configTranscoder
+                .Setup(transcoder => transcoder.Decode<BucketConfig>(It.IsAny<byte[]>(), It.IsAny<int>(),
+                    It.IsAny<int>(), It.IsAny<Flags>(), It.IsAny<OperationCode>()))
+                .Returns(config);
+
+            var result = operation.GetConfig(configTranscoder.Object);
+            Assert.AreSame(config, result);
+
+            // make sure the transcoder for decoding document content is not used
+            configTranscoder.Verify(
+                t => t.Decode<BucketConfig>(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Flags>(),
+                    It.IsAny<OperationCode>()), Times.Once);
+            contentTranscoder.Verify(
+                t => t.Decode<BucketConfig>(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Flags>(),
+                    It.IsAny<OperationCode>()), Times.Never);
+        }
+
+        private class FakeOperation : OperationBase
+        {
+            public FakeOperation(ITypeTranscoder transcoder)
+                : base("hello", null, transcoder, 0)
+            {
+            }
+
+            public override OperationCode OperationCode
+            {
+                get { return OperationCode.Get; }
+            }
+
+            public override ResponseStatus GetResponseStatus()
+            {
+                return ResponseStatus.VBucketBelongsToAnotherServer;
+            }
+        }
+    }
+}
diff --git a/Src/Couchbase/Core/Buckets/CallbackFactory.cs b/Src/Couchbase/Core/Buckets/CallbackFactory.cs
@@ -2,6 +2,7 @@
 using System.Collections.Concurrent;
 using System.Threading;
 using System.Threading.Tasks;
+using Couchbase.Core.Transcoders;
 using Couchbase.Logging;
 using Couchbase.IO;
 using Couchbase.IO.Operations;
@@ -46,7 +47,7 @@ public static Func<SocketAsyncState, Task> CompletedFuncWithRetryForMemcached<T>
                         {
                             if (result.IsNmv())
                             {
-                                var config = op.GetConfig();
+                                var config = op.GetConfig(controller.ServerConfigTranscoder);
                                 if (config != null)
                                 {
                                     controller.NotifyConfigPublished(config);
@@ -141,7 +142,7 @@ public static Func<SocketAsyncState, Task> CompletedFuncWithRetryForMemcached(IR
                         {
                             if (result.IsNmv())
                             {
-                                var config = op.GetConfig();
+                                var config = op.GetConfig(controller.ServerConfigTranscoder);
                                 if (config != null)
                                 {
                                     controller.NotifyConfigPublished(config);
@@ -243,7 +244,7 @@ public static Func<SocketAsyncState, Task> CompletedFuncWithRetryForCouchbase<T>
                         {
                             if (result.IsNmv())
                             {
-                                var config = op.GetConfig();
+                                var config = op.GetConfig(controller.ServerConfigTranscoder);
                                 if (config != null)
                                 {
                                     controller.NotifyConfigPublished(config);
@@ -343,7 +344,7 @@ public static Func<SocketAsyncState, Task> CompletedFuncWithRetryForCouchbase(IR
                         {
                             if (result.IsNmv())
                             {
-                                var config = op.GetConfig();
+                                var config = op.GetConfig(controller.ServerConfigTranscoder);
                                 if (config != null)
                                 {
                                     controller.NotifyConfigPublished(config);
@@ -439,7 +440,7 @@ public static Func<SocketAsyncState, Task> CompletedFuncForRetry<T>(
                         var result = actual.GetResultWithValue();
                         if (result.IsNmv())
                         {
-                            var config = actual.GetConfig();
+                            var config = actual.GetConfig(controller.ServerConfigTranscoder);
                             if (config != null)
                             {
                                 controller.NotifyConfigPublished(config);
@@ -501,7 +502,7 @@ public static Func<SocketAsyncState, Task> CompletedFuncForRetry(
                         var result = op.GetResult();
                         if (result.IsNmv())
                         {
-                            var config = op.GetConfig();
+                            var config = op.GetConfig(controller.ServerConfigTranscoder);
                             if (config != null)
                             {
                                 controller.NotifyConfigPublished(config);

diff --git a/Src/Couchbase/Core/Buckets/CouchbaseRequestExecuter.cs b/Src/Couchbase/Core/Buckets/CouchbaseRequestExecuter.cs
@@ -9,6 +9,7 @@
 using Couchbase.Configuration.Server.Serialization;
 using Couchbase.Core.Diagnostics;
 using Couchbase.Core.Services;
+using Couchbase.Core.Transcoders;
 using Couchbase.IO;
 using Couchbase.IO.Operations;
 using Couchbase.N1QL;
@@ -62,7 +63,7 @@ public bool CheckForConfigUpdates(IOperation operation)
             var requiresRetry = false;
             try
             {
-                var bucketConfig = operation.GetConfig();
+                var bucketConfig = operation.GetConfig(ClusterController.ServerConfigTranscoder);
                 if (bucketConfig != null)
                 {
                     Log.Info("New config found {0}|{1}", bucketConfig.Rev, ConfigInfo.BucketConfig.Rev);

diff --git a/Src/Couchbase/Core/ClusterController.cs b/Src/Couchbase/Core/ClusterController.cs
@@ -68,6 +68,7 @@ public ClusterController(ClientConfiguration clientConfig,
             _saslFactory = saslFactory;
             Converter = converter;
             Transcoder = transcoder;
+            ServerConfigTranscoder = new DefaultTranscoder();
             Initialize();
 
             if (clientConfig.EnableDeadServiceUriPing)
@@ -124,6 +125,8 @@ public void EnqueueConfigForProcessing(IBucketConfig config)
 
         public ITypeTranscoder Transcoder { get; private set; }
 
+        public ITypeTranscoder ServerConfigTranscoder { get; private set; }
+
         public List<IConfigProvider> ConfigProviders { get { return _configProviders; } }
 
         private void Initialize()

diff --git a/Src/Couchbase/Core/IClusterController.cs b/Src/Couchbase/Core/IClusterController.cs
@@ -25,6 +25,8 @@ internal interface IClusterController : IConfigPublisher, IDisposable
 
         ITypeTranscoder Transcoder { get; }
 
+        ITypeTranscoder ServerConfigTranscoder { get; }
+
         IConfigProvider GetProvider(string name);
 
         IBucket CreateBucket(string bucketName, IAuthenticator authenticator = null);

diff --git a/Src/Couchbase/CouchbaseBucket.cs b/Src/Couchbase/CouchbaseBucket.cs
@@ -3732,7 +3732,7 @@ bool CheckForConfigUpdates<T>(IOperationResult<T> operationResult, IOperation op
             {
                 try
                 {
-                    var bucketConfig = operation.GetConfig();
+                    var bucketConfig = operation.GetConfig(_clusterController.ServerConfigTranscoder);
                     if (bucketConfig != null)
                     {
                         Log.Info("New config found {0}|{1}: {2}", bucketConfig.Rev, _configInfo.BucketConfig.Rev, JsonConvert.SerializeObject(bucketConfig));

diff --git a/Src/Couchbase/IO/Operations/IOperation.cs b/Src/Couchbase/IO/Operations/IOperation.cs
@@ -57,8 +57,11 @@ public interface IOperation
 
         void HandleClientError(string message, ResponseStatus responseStatus);
 
+        [Obsolete("Please use Getconfig(ITypeTranscoder) instead.")]
         IBucketConfig GetConfig();
 
+        IBucketConfig GetConfig(ITypeTranscoder transcoder);
+
         uint Opaque { get; }
 
         uint Timeout { get; set; }

diff --git a/Src/Couchbase/IO/Operations/OperationBase.cs b/Src/Couchbase/IO/Operations/OperationBase.cs
@@ -400,7 +400,13 @@ public string GetMessage()
             return message;
         }
 
+        [Obsolete("Please use Getconfig(ITypeTranscoder) instead.")]
         public virtual IBucketConfig GetConfig()
+        {
+            return GetConfig(Transcoder);
+        }
+
+        public virtual IBucketConfig GetConfig(ITypeTranscoder transcoder)
         {
             IBucketConfig config = null;
             if (GetResponseStatus() == ResponseStatus.VBucketBelongsToAnotherServer && Data != null)
@@ -409,7 +415,7 @@ public virtual IBucketConfig GetConfig()
                 var length = Header.BodyLength - Header.ExtrasLength;
 
                 //Override any flags settings since the body of the response has changed to a config
-                config = Transcoder.Decode<BucketConfig>(Data.ToArray(), offset, length, new Flags
+                config = transcoder.Decode<BucketConfig>(Data.ToArray(), offset, length, new Flags
                 {
                     Compression = Compression.None,
                     DataFormat = DataFormat.Json,