Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for managing Atlas search indexes. #1158

Merged
merged 34 commits into from
Jul 31, 2023
Merged

Add support for managing Atlas search indexes. #1158

merged 34 commits into from
Jul 31, 2023

Conversation

vbabanin
Copy link
Member

@vbabanin vbabanin commented Jul 5, 2023 •
edited
Loading

  • Added new collection helpers that allow users to define their search indexes directly within their code.
  • Added unified tests to ensure the proper command structure and format.

The new search index commands are not supported in transactions yet. As a result, the ClientSession parameter has not been provided.

JAVA-4983

vbabanin added 3 commits July 5, 2023 11:54
@vbabanin
Add support for managing Atlas search indexes.
bb610dd 
- Add new collection helpers that allow users to define their search indexes directly within their code.
- Implement unified tests to ensure the proper command structure and format.

JAVA-4983
@vbabanin
Fix formatting, spotbugs issues.
123c8cd 
JAVA-4983
@vbabanin
Fix formatting.
1dd433d 
JAVA-4983
vbabanin added 3 commits July 5, 2023 15:32
@vbabanin
Add javadoc.
8ce344a 
JAVA-4983
@vbabanin
Fix tests.
e89553c 
JAVA-4983
@vbabanin
Change javadoc.
f977401 
JAVA-4983
@vbabanin vbabanin requested review from rozza and stIncMale July 7, 2023 15:43
@vbabanin vbabanin marked this pull request as ready for review July 7, 2023 15:43
@vbabanin
Copy link
Member Author

vbabanin commented Jul 7, 2023 •
edited
Loading

One of the spec tests is currently failing due to unexpected server behavior, and I'm working on resolving it.

UPD: There is an associated server issue has been created SERVER-78831. This issue is the reason behind the test failure.

rozza
rozza requested changes Jul 10, 2023
View reviewed changes
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor nits but looking good so far.

Main question I have is; should the API also include clientSession support?

driver-core/src/main/com/mongodb/internal/operation/ListSearchIndexesOperation.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/operation/SearchIndexRequest.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/operation/UpdateSearchIndexesOperation.java Outdated Show resolved Hide resolved
driver-sync/src/main/com/mongodb/client/ListSearchIndexesIterable.java Outdated Show resolved Hide resolved
driver-sync/src/main/com/mongodb/client/ListSearchIndexesIterable.java Outdated Show resolved Hide resolved
driver-sync/src/main/com/mongodb/client/ListSearchIndexesIterable.java Outdated Show resolved Hide resolved
driver-sync/src/main/com/mongodb/client/MongoCollection.java Show resolved Hide resolved
driver-sync/src/main/com/mongodb/client/internal/ListSearchIndexesIterableImpl.java Show resolved Hide resolved
...rc/integration/kotlin/com/mongodb/kotlin/client/coroutine/syncadapter/SyncMongoCollection.kt Outdated Show resolved Hide resolved
vbabanin added 4 commits July 10, 2023 18:32
@vbabanin
Remove wildcard imports.
795d351 
Use append for BsonDocument construction.
Change the order of exception handling to have logicaly grouped sections.

JAVA-4983
@vbabanin
Change documentation.
891d386 
JAVA-4983
@vbabanin
Change javadoc.
ab7d67d 
JAVA-4983
@vbabanin
Remove maxAwaitTime option for ListSearchIndexes iterable/publisher i…
95b6810 
…nterfaces since cursor is non-tailable.

JAVA-4983
@vbabanin vbabanin requested a review from rozza July 12, 2023 01:17
rozza
rozza approved these changes Jul 12, 2023
View reviewed changes
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

stIncMale
stIncMale requested changes Jul 12, 2023
View reviewed changes
...er-core/src/test/resources/unified-test-format/index-management/tests/createSearchIndex.json Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/internal/operation/AggregateOperation.java Show resolved Hide resolved
driver-sync/src/main/com/mongodb/client/ListSearchIndexesIterable.java Outdated Show resolved Hide resolved
driver-core/src/main/com/mongodb/client/model/SearchIndexModel.java Show resolved Hide resolved
driver-core/src/main/com/mongodb/client/model/SearchIndexModel.java Outdated Show resolved Hide resolved
...reactive-streams/src/main/com/mongodb/reactivestreams/client/ListSearchIndexesPublisher.java Show resolved Hide resolved
...ams/src/main/com/mongodb/reactivestreams/client/internal/ListSearchIndexesPublisherImpl.java Outdated Show resolved Hide resolved
...ams/src/main/com/mongodb/reactivestreams/client/internal/ListSearchIndexesPublisherImpl.java Show resolved Hide resolved
...ams/src/main/com/mongodb/reactivestreams/client/internal/ListSearchIndexesPublisherImpl.java Outdated Show resolved Hide resolved
...ams/src/main/com/mongodb/reactivestreams/client/internal/ListSearchIndexesPublisherImpl.java Outdated Show resolved Hide resolved
vbabanin and others added 10 commits July 14, 2023 20:18
@vbabanin @stIncMale
Update driver-core/src/main/com/mongodb/internal/operation/SearchInde…
78dcd04 
…xRequest.java

Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin @stIncMale
Update driver-core/src/main/com/mongodb/client/model/SearchIndexModel…
d88b6a8 
….java

Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin @stIncMale
Update driver-core/src/main/com/mongodb/internal/operation/AbstractWr…
36c6bc8 
…iteSearchIndexOperation.java

Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin @stIncMale
Update driver-core/src/main/com/mongodb/internal/operation/AbstractWr…
d7208ca 
…iteSearchIndexOperation.java

Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin @stIncMale
Update driver-core/src/main/com/mongodb/internal/operation/AbstractWr…
d04f161 
…iteSearchIndexOperation.java

Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin @stIncMale
Update driver-core/src/main/com/mongodb/internal/operation/CreateSear…
a7298ad 
…chIndexesOperation.java

Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin @stIncMale
Update driver-core/src/main/com/mongodb/internal/operation/CreateSear…
f8879d1 
…chIndexesOperation.java

Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin @stIncMale
Apply suggestions from code review
eb25379 
Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin
Add retryable functionality.
f83e870 
Add strictest modifiers.

JAVA-4983
@vbabanin
Change javadoc.
1554663 
JAVA-4983
@stIncMale
Copy link
Member

TODO for @stIncMale: We discussed that a write concern can't really be null. Check that eventually it is not, or discuss again.

@vbabanin vbabanin requested a review from stIncMale July 17, 2023 20:08
stIncMale
stIncMale requested changes Jul 24, 2023
View reviewed changes
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few things from the previous review round, and a few small new things.

driver-sync/src/main/com/mongodb/client/MongoCollection.java Outdated Show resolved Hide resolved
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoCollection.java Outdated Show resolved Hide resolved
driver-scala/src/main/scala/org/mongodb/scala/MongoCollection.scala Outdated Show resolved Hide resolved
driver-scala/src/main/scala/org/mongodb/scala/MongoCollection.scala Outdated Show resolved Hide resolved
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoCollection.java Outdated Show resolved Hide resolved
driver-reactive-streams/src/main/com/mongodb/reactivestreams/client/MongoCollection.java Outdated Show resolved Hide resolved
driver-scala/src/main/scala/org/mongodb/scala/MongoCollection.scala Outdated Show resolved Hide resolved
config/spotbugs/exclude.xml Outdated Show resolved Hide resolved
vbabanin and others added 3 commits July 24, 2023 14:16
@vbabanin @stIncMale
Update driver-sync/src/main/com/mongodb/client/MongoCollection.java
2235541 
Co-authored-by: Valentin Kovalenko <[email protected]>
@vbabanin
Fix spotless warnings.
e7b8192 
Make UpdateSearchIndexOperation use SearchIndexRequest.

JAVA-4983
@vbabanin
Make class internal.
f5f6230 
Remove redundunt method.

JAVA-4983
@vbabanin vbabanin requested a review from stIncMale July 25, 2023 21:24
stIncMale
stIncMale reviewed Jul 25, 2023
View reviewed changes
Copy link
Member

@stIncMale stIncMale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#1158 (comment) is still outstanding, otherwise 👍

@vbabanin
Add null-check.
f63ff7b 
JAVA-4983
@vbabanin vbabanin requested a review from stIncMale July 26, 2023 00:41
@vbabanin
Add null-check.
5cbee87 
JAVA-4983
@vbabanin vbabanin merged commit 46903fb into mongodb:master Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rozza rozza rozza approved these changes

@stIncMale stIncMale stIncMale approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vbabanin @stIncMale @rozza