Skip to content

Commit

Permalink
NCBC-1447: Correct behavior of QueryRequest.Pretty(bool) method
Browse files Browse the repository at this point in the history
Motivation
----------
Implementation of QueryRequest.Pretty(bool) is only sending pretty=true
to the query engine, never pretty=false.  This makes it impossible to
improve performance by disabling pretty print on Couchbase Server < 5.0.

Modifications
-------------
Always send pretty=x to query engine if QueryRequest.Pretty(bool) is
called.  If not called, it will continue to use the server-side default.

Added unit and integration tests to test the behavior.

Results
-------
Calling Pretty(false) now works as expected against Couchbase Server 4.5
and 4.6.

Change-Id: I42cfa655e50a78df018fbd156f29f2a8de2b53cf
Reviewed-on: http://review.couchbase.org/79529
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Matt Ingenthron <matt@couchbase.com>
Reviewed-by: Mike Goldsmith <goldsmith.mike@gmail.com>
  • Loading branch information
brantburnett authored and jeffrymorris committed Jun 20, 2017
1 parent 34e2b5b commit 67cc36b
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 3 deletions.
36 changes: 36 additions & 0 deletions Src/Couchbase.IntegrationTests/CouchbaseBucket_N1QlQuery_Tests.cs
@@ -1,12 +1,15 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using Couchbase.Configuration.Client;
using Couchbase.Core;
using Couchbase.IntegrationTests.Utils;
using Couchbase.N1QL;
using Couchbase.Views;
using Moq;
using NUnit.Framework;

Expand Down Expand Up @@ -281,6 +284,39 @@ public void Test_DefaultValueHandlingIgnoreAndPoplate_BadQuery_NoException()
}
}

[Test(Description = "https://issues.couchbase.com/browse/NCBC-1447")]
[TestCase(true)]
[TestCase(false)]
public async Task PrettyPrint_AffectsWhitespace(bool pretty)
{
string content = null;

var dataMapper = new Mock<IDataMapper>();
dataMapper
.Setup(p => p.Map<QueryResultData<dynamic>>(It.IsAny<Stream>()))
.Callback((Stream stream) =>
{
using (var reader = new StreamReader(stream, System.Text.Encoding.UTF8))
{
content = reader.ReadToEnd();
}
});

var request = new QueryRequest("SELECT META().id FROM `default` WHERE name = $1 LIMIT 2;")
{
DataMapper = dataMapper.Object
}
.Pretty(pretty);

await _bucket.QueryAsync<dynamic>(request);

Assert.IsNotNull(content);

// Test to see if content contains whitespace at the beginning of lines
// Even with pretty=false it will still have line feeds and some spaces
Assert.AreEqual(pretty, Regex.IsMatch(content, @"^ ", RegexOptions.Multiline));
}

[OneTimeTearDown]
public void OneTimeTearDown()
{
Expand Down
1 change: 1 addition & 0 deletions Src/Couchbase.UnitTests/Couchbase.UnitTests.csproj
Expand Up @@ -125,6 +125,7 @@
<Compile Include="Core\ExpressionVisitors\SubDocumentPathExpressionVisitorTests.cs" />
<Compile Include="CouchbaseBucketTests.cs" />
<Compile Include="IO\ConnectionTests.cs" />
<Compile Include="N1Ql\QueryRequestTests.cs" />
<Compile Include="N1Ql\StreamingQueryRequestTests.cs" />
<Compile Include="N1Ql\N1qlRyowTests.cs" />
<Compile Include="N1Ql\QueryClientTests.cs" />
Expand Down
54 changes: 54 additions & 0 deletions Src/Couchbase.UnitTests/N1Ql/QueryRequestTests.cs
@@ -0,0 +1,54 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Couchbase.N1QL;
using NUnit.Framework;

namespace Couchbase.UnitTests.N1Ql
{
[TestFixture]
public class QueryRequestTests
{
#region GetFormValues

[Test]
public void GetFormValues_NoPrettyCall_NoPrettyParam()
{
// Arrange

var request = new QueryRequest("SELECT * FROM default");

// Act

var fields = request.GetFormValues();

// Assert

Assert.False(fields.Keys.Contains("pretty"));
}

[Test]
[TestCase(true)]
[TestCase(false)]
public void GetFormValues_PrettyCall_IncludesParam(bool pretty)
{
// Arrange

var request = new QueryRequest("SELECT * FROM default")
.Pretty(pretty);

// Act

var fields = request.GetFormValues();

// Assert

Assert.True(fields.Keys.Contains("pretty"));
Assert.AreEqual(pretty, fields["pretty"]);
}

#endregion
}
}
6 changes: 3 additions & 3 deletions Src/Couchbase/N1QL/QueryRequest.cs
Expand Up @@ -29,7 +29,7 @@ public class QueryRequest : IQueryRequestWithDataMapper
private ScanConsistency? _scanConsistency;
private bool? _includeSignature;
private TimeSpan? _scanWait;
private bool _pretty;
private bool? _pretty;
private readonly Dictionary<string, string> _credentials = new Dictionary<string, string>();
private string _clientContextId;
private Uri _baseUri;
Expand Down Expand Up @@ -721,9 +721,9 @@ public QueryPlan GetPreparedPayload()
{
formValues.Add(QueryParameters.ScanWait, string.Format("{0}ms", (uint) _scanWait.Value.TotalMilliseconds));
}
if (_pretty)
if (_pretty != null)
{
formValues.Add(QueryParameters.Pretty, _pretty);
formValues.Add(QueryParameters.Pretty, _pretty.Value);
}
if (_credentials.Count > 0)
{
Expand Down

0 comments on commit 67cc36b

Please sign in to comment.