Skip to content

Commit

Permalink
Allow nested arrays to be constructed client-side, relying on server-…
Browse files Browse the repository at this point in the history
…side validation

While nested arrays aren't allowed in documents, they are allowed for Where-In queries, where the query needs to express "the value of the field should be one of these arrays".
In an ideal world we'd only permit that for the query rather than document construction, but that's relatively hard to weave through the code... whereas just allowing it client-side and then preventing it server-side is simple.
Note that clients can already construct nested arrays by creating their own Value objects, so the server-side validation is required there anyway.

Fixes #5007.
  • Loading branch information
jskeet committed Jun 2, 2020
1 parent b1e6e8b commit 48cef0e
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 9 deletions.
Expand Up @@ -12,7 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

using Castle.DynamicProxy.Generators.Emitters.SimpleAST;
using Grpc.Core;
using System.Collections.Generic;
using System.Threading.Tasks;
using Xunit;

Expand Down Expand Up @@ -48,5 +50,22 @@ public async Task Create_AlreadyExists()
// See also https://github.com/googleapis/toolkit/issues/1357
await Assert.ThrowsAsync<RpcException>(() => reference.CreateAsync(new { Name = "Other" }));
}

[Fact]
public async Task Create_WithNestedArray()
{
var collection = _fixture.NonQueryCollection;
var objectWithNestedArray = new
{
Name = "Test nested array",
Array = new[] {
new object[] { 1, 2 },
new object[] { "a", "b" }
}
};
// Even though we now permit nested arrays locally for convenience, the server still
// prohibits it.
await Assert.ThrowsAsync<RpcException>(() => collection.AddAsync(objectWithNestedArray));
}
}
}
Expand Up @@ -162,8 +162,18 @@ public void BadDateTimeKind(DateTimeKind kind)
[Fact]
public void ArrayInArray()
{
var badArray = new[] { new int[10] };
Assert.Throws<ArgumentException>(() => ValueSerializer.Serialize(SerializationContext.Default, badArray));
// We allow this to be created, even though the server will reject *documents* created like this.
// It can be useful in filters, e.g. in a WhereIn query.
var nestedArray = new[] { new int[] { 10, 20 } };
var expected = new Value
{
ArrayValue = new ArrayValue
{
Values = { new Value { ArrayValue = new ArrayValue { Values = { new Value { IntegerValue = 10 }, new Value { IntegerValue = 20 } } } } }
}
};
var actual = ValueSerializer.Serialize(SerializationContext.Default, nestedArray);
Assert.Equal(expected, actual);
}

[Fact]
Expand Down
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

using Google.Cloud.Firestore.V1;
using System;
using System.Collections;
using BclType = System.Type;

Expand All @@ -36,12 +35,7 @@ public override Value Serialize(SerializationContext context, object value)
var repeatedField = proto.ArrayValue.Values;
foreach (object element in (IEnumerable) value)
{
var serializedElement = ValueSerializer.Serialize(context, element);
if (serializedElement.ArrayValue != null)
{
throw new ArgumentException("Array values cannot directly contain other array values");
}
repeatedField.Add(serializedElement);
repeatedField.Add(ValueSerializer.Serialize(context, element));
}
return proto;
}
Expand Down

0 comments on commit 48cef0e

Please sign in to comment.