Support `authorizedCollections` option for `listCollections` helpers #1270

stIncMale · 2023-12-05T19:24:55Z

This is the 3rd incarnation of the PR. The previous are (from younger to older): #1263, #855 (the original one).

API changes

`MongoDatabase.listCollectionNames`

Changed the return type of a method. The change preserves source-code compatibility (existing source code that uses the changed methods does not have to be changed), breaks binary compatibility (existing source code that uses the changed methods has to be recompiled against the updated driver dependency).

package	class/interface	method(s)	old return type	new return type
`com.mongodb.client`	`MongoDatabase`	`listCollectionNames`	`MongoIterable`	`ListCollectionNamesIterable`
`com.mongodb.reactivestreams.client`	`MongoDatabase`	`listCollectionNames`	`Publisher`	`ListCollectionNamesPublisher`
`org.mongodb.scala`	`MongoDatabase`	`listCollectionNames`	`Observable`	`ListCollectionNamesObservable`
`com.mongodb.kotlin.client`	`MongoDatabase`	`listCollectionNames`	`MongoIterable`	`ListCollectionNamesIterable`
`com.mongodb.kotlin.client.coroutine`	`MongoDatabase`	`listCollectionNames`	`Flow`	`ListCollectionNamesFlow`

`authorizedCollections`

Added classes/interfaces with a new method authorizedCollections.

package	existing class/interface (was not modified)	new class/interface
`com.mongodb.client`	`ListCollectionsIterable`	`ListCollectionNamesIterable`
`com.mongodb.reactivestreams.client`	`ListCollectionsPublisher`	`ListCollectionNamesPublisher`
`org.mongodb.scala`	`ListCollectionsObservable`	`ListCollectionNamesObservable`
`com.mongodb.kotlin.client`	`ListCollectionsIterable`	`ListCollectionNamesIterable`
`com.mongodb.kotlin.client.coroutine`	`ListCollectionsFlow`	`ListCollectionNamesFlow`

JAVA-4353

stIncMale · 2023-12-05T19:56:20Z

...coroutine/src/test/kotlin/com/mongodb/kotlin/client/coroutine/ListCollectionNamesFlowTest.kt

+import org.mockito.kotlin.verify
+import org.mockito.kotlin.verifyNoMoreInteractions
+
+class ListCollectionNamesFlowTest {


Is based on com.mongodb.kotlin.client.coroutine.ListCollectionsFlowTest.

stIncMale · 2023-12-05T19:57:34Z

...on/kotlin/com/mongodb/kotlin/client/coroutine/syncadapter/SyncListCollectionNamesIterable.kt

+import org.bson.BsonValue
+import org.bson.conversions.Bson
+
+data class SyncListCollectionNamesIterable(val wrapped: ListCollectionNamesFlow) :


Is based on com.mongodb.kotlin.client.coroutine.syncadapter.SyncListCollectionsIterable.

stIncMale · 2023-12-05T19:58:30Z

...lin-coroutine/src/main/kotlin/com/mongodb/kotlin/client/coroutine/ListCollectionNamesFlow.kt

+ *
+ * @see [List collections](https://www.mongodb.com/docs/manual/reference/command/listCollections/)
+ */
+public class ListCollectionNamesFlow(private val wrapped: ListCollectionNamesPublisher) :


Is based on com.mongodb.kotlin.client.coroutine.ListCollectionsFlow.

stIncMale · 2023-12-05T20:00:59Z

.../integration/kotlin/com/mongodb/kotlin/client/syncadapter/SyncListCollectionNamesIterable.kt

+import org.bson.BsonValue
+import org.bson.conversions.Bson
+
+internal class SyncListCollectionNamesIterable(val wrapped: ListCollectionNamesIterable) :


Is based on com.mongodb.kotlin.client.syncadapter.SyncListCollectionsIterable.

stIncMale · 2023-12-05T20:14:59Z

driver-kotlin-sync/src/main/kotlin/com/mongodb/kotlin/client/ListCollectionNamesIterable.kt

+ *
+ * @see [List collections](https://www.mongodb.com/docs/manual/reference/command/listCollections/)
+ */
+public class ListCollectionNamesIterable(private val wrapped: JListCollectionNamesIterable) :


Is based on com.mongodb.kotlin.client.ListCollectionsIterable.

stIncMale · 2023-12-05T22:49:05Z

driver-kotlin-sync/src/test/kotlin/com/mongodb/kotlin/client/ListCollectionNamesIterableTest.kt

+import org.mockito.kotlin.verify
+import org.mockito.kotlin.verifyNoMoreInteractions
+
+class ListCollectionNamesIterableTest {


Is based on com.mongodb.kotlin.client.ListCollectionsIterableTest.

stIncMale · 2023-12-05T23:39:38Z

...nctional/com/mongodb/reactivestreams/client/syncadapter/SyncListCollectionNamesIterable.java

+
+import java.util.concurrent.TimeUnit;
+
+final class SyncListCollectionNamesIterable extends SyncMongoIterable<String> implements ListCollectionNamesIterable {


Is based on com.mongodb.reactivestreams.client.syncadapter.SyncListCollectionsIterable.

stIncMale · 2023-12-05T23:42:09Z

...rc/test/tck/com/mongodb/reactivestreams/client/ListCollectionNamesPublisherVerification.java

+import static com.mongodb.reactivestreams.client.MongoFixture.PUBLISHER_REFERENCE_CLEANUP_TIMEOUT_MILLIS;
+import static com.mongodb.reactivestreams.client.MongoFixture.run;
+
+public class ListCollectionNamesPublisherVerification extends PublisherVerification<String> {


Is based on com.mongodb.reactivestreams.client.ListCollectionsPublisherVerification.

stIncMale · 2023-12-05T23:49:15Z

driver-scala/src/main/scala/org/mongodb/scala/ListCollectionNamesObservable.scala

+ * @param wrapped the underlying java ListCollectionNamesPublisher
+ * @since 5.0
+ */
+case class ListCollectionNamesObservable(wrapped: ListCollectionNamesPublisher) extends Observable[String] {


Is based on org.mongodb.scala.ListCollectionsObservable.

stIncMale · 2023-12-05T23:50:42Z

driver-scala/src/test/scala/org/mongodb/scala/ListCollectionNamesObservableSpec.scala

+import java.util.concurrent.TimeUnit
+import scala.concurrent.duration.Duration
+
+class ListCollectionNamesObservableSpec extends BaseSpec with MockitoSugar {


Is based on org.mongodb.scala.ListCollectionsObservableSpec.

stIncMale · 2023-12-06T00:16:55Z

...t/unit/com/mongodb/reactivestreams/client/internal/ListCollectionNamesPublisherImplTest.java

+import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+final class ListCollectionNamesPublisherImplTest extends TestHelper {


Is based on com.mongodb.reactivestreams.client.internal.ListCollectionsPublisherImplTest.

JAVA-4353

jyemin

Started with a review of the API, will proceed with the implementation after initial comments are resolved.

driver-sync/src/main/com/mongodb/client/CommonListCollectionsIterable.java

driver-sync/src/main/com/mongodb/client/ListCollectionsIterable.java

JAVA-4353

jyemin

Waiting on the unit test we discussed. Otherwise LGTM.

jyemin · 2023-12-06T22:31:08Z

driver-core/src/main/com/mongodb/internal/operation/ListCollectionsOperation.java

@@ -206,6 +226,7 @@ private BsonDocument getCommand() {
        if (nameOnly) {
            command.append("nameOnly", BsonBoolean.TRUE);
        }
+        putIfTrue(command, "authorizedCollections", authorizedCollections);


As discussed, add a unit test for this.

Done.

The added ListCollectionsOperationTest tests the public method ListCollectionsOperation.execute instead of testing the (currently private) method getCommand. This way it covers more of ListCollectionsOperation. Since mocking is needed for this, I introduced MongoMockito.mock (see its documentation for the detailed explanation and links to demo tests). I demonstrated it to @vbabanin and @rozza, and my understanding was that they liked this improvement over Mockito.mock.

JAVA-4353

stIncMale · 2023-12-07T23:52:41Z

...er-core/src/test/unit/com/mongodb/internal/mockito/InsufficientStubbingDetectorDemoTest.java

+    @Test
+    void stubbingWithInsufficientStubbingDetector() {
+        MongoMockito.mock(ReadBinding.class, bindingMock ->
+                when(bindingMock.getOperationContext()).thenReturn(new OperationContext())
+        );
+    }


For some reason, unused Mockito stubbing is not reported when we run tests (ideally, stubbingWithInsufficientStubbingDetector must fail with UnnecessaryStubbingException because the test never calls getOperationContext). This has nothing to do with MongoMockito, as I tried the same with Mockito, and nothing was reported there either. Even calling Mockito.validateMockitoUsage didn't help. If someone has any ideas, please comment.

Add the juniper extension and 💥

@ExtendWith(MockitoExtension.class) final class InsufficientStubbingDetectorDemoTest {

Indeed, thank you! After looking at MockitoExtension's code, I discovered that the following manually written code also enables validation:

private MockitoSession session; @BeforeEach void beforeEach() { session = Mockito.mockitoSession().startMocking(); } @AfterEach void afterEach() { session.finishMocking(); }

finishMocking calls Mockito.validateMockitoUsage, but only after notifying Mockito.framework() that a test has finished via UniversalTestListener.testFinished.

Oh, actually, it's the UniversalTestListener.testFinished that checks unused stubbing, not Mockito.validateMockitoUsage.

rozza

lgtm

driver-core/src/test/unit/com/mongodb/internal/operation/ListCollectionsOperationTest.java

driver-core/src/test/unit/com/mongodb/internal/mockito/MongoMockito.java

driver-core/src/test/unit/com/mongodb/internal/mockito/InsufficientStubbingDetector.java

JAVA-4353

jyemin

LGTM.

Remove a redundant constructor from ListCollectionsIterableImpl

d91efbb

JAVA-4353

stIncMale self-assigned this Dec 5, 2023

stIncMale commented Dec 5, 2023

View reviewed changes

stIncMale marked this pull request as draft December 5, 2023 20:48

stIncMale commented Dec 5, 2023

View reviewed changes

stIncMale force-pushed the JAVA-4353 branch 2 times, most recently from 72a7033 to 619a974 Compare December 6, 2023 00:13

stIncMale commented Dec 6, 2023

View reviewed changes

stIncMale force-pushed the JAVA-4353 branch from 619a974 to 9eeadbc Compare December 6, 2023 00:36

Support authorizedCollections option for listCollections helpers

c646b12

JAVA-4353

stIncMale force-pushed the JAVA-4353 branch from 9eeadbc to c646b12 Compare December 6, 2023 01:04

stIncMale requested review from jyemin and rozza December 6, 2023 01:06

stIncMale mentioned this pull request Dec 6, 2023

Support authorizedCollections option for listCollections helpers #1263

Closed

stIncMale marked this pull request as ready for review December 6, 2023 01:07

jyemin reviewed Dec 6, 2023

View reviewed changes

driver-sync/src/main/com/mongodb/client/CommonListCollectionsIterable.java Outdated Show resolved Hide resolved

driver-sync/src/main/com/mongodb/client/ListCollectionsIterable.java Outdated Show resolved Hide resolved

Remove CommonListCollectionsIterable, CommonListCollectionsPublisher

537e9ab

JAVA-4353

stIncMale requested a review from jyemin December 6, 2023 19:50

jyemin requested changes Dec 6, 2023

View reviewed changes

Add ListCollectionsOperationTest and MongoMockito

9a7dbe7

JAVA-4353

stIncMale commented Dec 7, 2023

View reviewed changes

stIncMale requested a review from jyemin December 8, 2023 00:04

rozza approved these changes Dec 8, 2023

View reviewed changes

jyemin requested changes Dec 8, 2023

View reviewed changes

driver-core/src/test/unit/com/mongodb/internal/operation/ListCollectionsOperationTest.java Show resolved Hide resolved

jyemin requested changes Dec 8, 2023

View reviewed changes

driver-core/src/test/unit/com/mongodb/internal/mockito/MongoMockito.java Outdated Show resolved Hide resolved

driver-core/src/test/unit/com/mongodb/internal/mockito/InsufficientStubbingDetector.java Show resolved Hide resolved

stIncMale added 2 commits December 8, 2023 10:20

Fix typos

b1d8b57

JAVA-4353

Add more tests

0ac0567

JAVA-4353

stIncMale requested a review from jyemin December 8, 2023 17:47

jyemin approved these changes Dec 8, 2023

View reviewed changes

stIncMale merged commit a03649c into mongodb:master Dec 8, 2023
56 checks passed

stIncMale deleted the JAVA-4353 branch December 8, 2023 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support `authorizedCollections` option for `listCollections` helpers #1270

Support `authorizedCollections` option for `listCollections` helpers #1270

stIncMale commented Dec 5, 2023 •

edited

stIncMale Dec 5, 2023 •

edited

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 5, 2023

stIncMale Dec 6, 2023

jyemin left a comment

jyemin left a comment

jyemin Dec 6, 2023

stIncMale Dec 7, 2023

stIncMale Dec 7, 2023 •

edited

rozza Dec 8, 2023 •

edited

stIncMale Dec 8, 2023 •

edited

stIncMale Dec 8, 2023

rozza left a comment

jyemin left a comment


		import java.util.concurrent.TimeUnit;

		final class SyncListCollectionNamesIterable extends SyncMongoIterable<String> implements ListCollectionNamesIterable {

Support authorizedCollections option for listCollections helpers #1270

Support authorizedCollections option for listCollections helpers #1270

Conversation

stIncMale commented Dec 5, 2023 • edited

API changes

MongoDatabase.listCollectionNames

authorizedCollections

stIncMale Dec 5, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jyemin left a comment

Choose a reason for hiding this comment

jyemin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stIncMale Dec 7, 2023 • edited

Choose a reason for hiding this comment

rozza Dec 8, 2023 • edited

Choose a reason for hiding this comment

stIncMale Dec 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rozza left a comment

Choose a reason for hiding this comment

jyemin left a comment

Choose a reason for hiding this comment

Support `authorizedCollections` option for `listCollections` helpers #1270

Support `authorizedCollections` option for `listCollections` helpers #1270

stIncMale commented Dec 5, 2023 •

edited

`MongoDatabase.listCollectionNames`

`authorizedCollections`

stIncMale Dec 5, 2023 •

edited

stIncMale Dec 7, 2023 •

edited

rozza Dec 8, 2023 •

edited

stIncMale Dec 8, 2023 •

edited