Copilot commented on code in PR #7596:
URL: https://github.com/apache/ignite-3/pull/7596#discussion_r2821298760
##########
modules/platforms/dotnet/Apache.Ignite.Tests/Table/KeyValueViewPocoPrimitiveTests.cs:
##########
@@ -90,6 +90,46 @@ public void TestPutNullKeyThrowsArgumentException()
Assert.AreEqual("Value cannot be null. (Parameter 'key')",
keyEx!.Message);
}
+ [Test]
+ public async Task TestContainsAllKeysWhenAllKeysExistReturnsTrue()
+ {
+ await KvView.PutAsync(null, GetKeyPoco(1L), "val1");
+ await KvView.PutAsync(null, GetKeyPoco(2L), "val2");
+ await KvView.PutAsync(null, GetKeyPoco(3L), "val3");
+
+ var result = await KvView.ContainsAllKeysAsync(null, [GetKeyPoco(1L),
GetKeyPoco(2L), GetKeyPoco(3L)]);
+
+ Assert.IsTrue(result);
+ }
+
+ [Test]
+ public async Task TestContainsAllKeysWithAllNonExistingKeysReturnsFalse()
+ {
+ var result = await KvView.ContainsAllKeysAsync(null, [GetKeyPoco(1),
GetKeyPoco(2)]);
+
+ Assert.IsFalse(result);
+ }
+
+ [Test]
+ public async Task TestContainsAllKeysWithNonExistingKeysReturnsFalse()
+ {
+ await KvView.PutAsync(null, GetKeyPoco(1L), "val1");
+ await KvView.PutAsync(null, GetKeyPoco(2L), "val2");
+
+ var result = await KvView.ContainsAllKeysAsync(null, [GetKeyPoco(1L),
GetKeyPoco(2L), GetKeyPoco(3L)]);
+
+ Assert.IsFalse(result);
+ }
+
+ [Test]
+ public void
TestContainsAllKeysThrowsArgumentExceptionOnNullCollectionElement()
+ {
+ var ex = Assert.ThrowsAsync<ArgumentNullException>(
+ async () => await KvView.ContainsAllKeysAsync(null,
[GetKeyPoco(1), null!]));
+
Review Comment:
Test name says it throws ArgumentException, but the assertion expects
ArgumentNullException. Rename the test to reflect the actual expected exception
type (or change the expected exception if that was unintended).
##########
modules/platforms/dotnet/Apache.Ignite.Tests/Table/KeyValueViewBinaryTests.cs:
##########
@@ -93,6 +93,46 @@ public void TestPutNullThrowsArgumentException()
Assert.AreEqual("Value cannot be null. (Parameter 'val')",
valEx!.Message);
}
+ [Test]
+ public async Task TestContainsAllKeysWhenAllKeysExistReturnsTrue()
+ {
+ await KvView.PutAsync(null, GetTuple(1L), GetTuple("val1"));
+ await KvView.PutAsync(null, GetTuple(2L), GetTuple("val2"));
+ await KvView.PutAsync(null, GetTuple(3L), GetTuple("val3"));
+
+ var result = await KvView.ContainsAllKeysAsync(null, [GetTuple(1L),
GetTuple(2L), GetTuple(3L)]);
+
+ Assert.IsTrue(result);
+ }
+
+ [Test]
+ public async Task TestContainsAllKeysWithAllNonExistingKeysReturnsFalse()
+ {
+ var result = await KvView.ContainsAllKeysAsync(null, [GetTuple(1),
GetTuple(2)]);
+
+ Assert.IsFalse(result);
+ }
+
+ [Test]
+ public async Task TestContainsAllKeysWithNonExistingKeysReturnsFalse()
+ {
+ await KvView.PutAsync(null, GetTuple(1L), GetTuple("val1"));
+ await KvView.PutAsync(null, GetTuple(2L), GetTuple("val2"));
+
+ var result = await KvView.ContainsAllKeysAsync(null, [GetTuple(1L),
GetTuple(2L), GetTuple(3L)]);
+
+ Assert.IsFalse(result);
+ }
+
+ [Test]
+ public void
TestContainsAllKeysThrowsArgumentExceptionOnNullCollectionElement()
+ {
+ var ex = Assert.ThrowsAsync<ArgumentNullException>(
+ async () => await KvView.ContainsAllKeysAsync(null, [GetTuple(1),
null!]));
+
Review Comment:
Test name says it throws ArgumentException, but the assertion expects
ArgumentNullException. Rename the test to reflect the actual expected exception
type (or change the expected exception if that was unintended).
##########
modules/platforms/dotnet/Apache.Ignite/Internal/Table/RecordView.cs:
##########
@@ -122,6 +122,21 @@ public async Task<bool> ContainsKeyAsync(ITransaction?
transaction, T key)
return ReadSchemaAndBoolean(resBuf);
}
+ /// <inheritdoc/>
+ public async Task<bool> ContainsAllKeysAsync(ITransaction?
transaction, IEnumerable<T> keys)
+ {
+ IgniteArgumentCheck.NotNull(keys);
+
+ using var resBuf = await
DoMultiRecordOutOpAsync(ClientOp.TupleContainsAllKeys, transaction, keys, true)
+ .ConfigureAwait(false);
+ if (resBuf == null)
+ {
+ return false;
+ }
+
+ return ReadSchemaAndBoolean(resBuf);
Review Comment:
ContainsAllKeysAsync returns false when the keys collection is empty because
DoMultiRecordOutOpAsync returns null and this method maps that to false. For
“contains all”, an empty key set should evaluate to true (vacuous truth) and
matches behavior in other Ignite clients (e.g., Java containsAll on an empty
collection returns true). Consider short-circuiting to true when the collection
is empty (prefer a non-enumerating count check) and add a test for the
empty-collection case.
##########
modules/platforms/dotnet/Apache.Ignite.Tests/Table/KeyValueViewPocoTests.cs:
##########
@@ -97,6 +97,46 @@ public void TestPutNullThrowsArgumentException()
Assert.AreEqual("Value cannot be null. (Parameter 'val')",
valEx!.Message);
}
+ [Test]
+ public async Task TestContainsAllKeysWhenAllKeysExistReturnsTrue()
+ {
+ await KvView.PutAsync(null, GetKeyPoco(1L), GetValPoco("val1"));
+ await KvView.PutAsync(null, GetKeyPoco(2L), GetValPoco("val2"));
+ await KvView.PutAsync(null, GetKeyPoco(3L), GetValPoco("val3"));
+
+ var result = await KvView.ContainsAllKeysAsync(null, [GetKeyPoco(1L),
GetKeyPoco(2L), GetKeyPoco(3L)]);
+
+ Assert.IsTrue(result);
+ }
+
+ [Test]
+ public async Task TestContainsAllKeysWithAllNonExistingKeysReturnsFalse()
+ {
+ var result = await KvView.ContainsAllKeysAsync(null, [GetKeyPoco(1),
GetKeyPoco(2)]);
+
+ Assert.IsFalse(result);
+ }
+
+ [Test]
+ public async Task TestContainsAllKeysWithNonExistingKeysReturnsFalse()
+ {
+ await KvView.PutAsync(null, GetKeyPoco(1L), GetValPoco("val1"));
+ await KvView.PutAsync(null, GetKeyPoco(2L), GetValPoco("val2"));
+
+ var result = await KvView.ContainsAllKeysAsync(null, [GetKeyPoco(1L),
GetKeyPoco(2L), GetKeyPoco(3L)]);
+
+ Assert.IsFalse(result);
+ }
+
+ [Test]
+ public void
TestContainsAllKeysThrowsArgumentExceptionOnNullCollectionElement()
+ {
+ var ex = Assert.ThrowsAsync<ArgumentNullException>(
+ async () => await KvView.ContainsAllKeysAsync(null,
[GetKeyPoco(1), null!]));
+
Review Comment:
Test name says it throws ArgumentException, but the assertion expects
ArgumentNullException. Rename the test to reflect the actual expected exception
type (or change the expected exception if that was unintended).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
