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.

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

fix(NODE-6019): indexExists always returns false when full is set to true #4034

Merged

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Mar 12, 2024

Description

What is changing?

In preparation for NODE-5971, I took time to clean up our index management implementation.

  • index_management.test.js is converted to TS and async await.
  • unused EnsureIndexOperation is removed.
  • CreateIndexOperation has been removed in favor of CreateIndexesOperation.
  • the indexInformation helper has been removed and the classes that used it as well: Indexes, IndexExists and IndexInformation operations have had their logic shifted into the collection class. Additionally, tests were added to ensure the behavior was not broken for these functions.
Is there new documentation needed for these changes?

What is the motivation for this change?

Release Highlight

indexExists() no longer supports the full option

The Collection.indexExists() helper supported an option, full, that modified the internals of the method. When full was set to true, the driver would always return false, regardless of whether or not the index exists.

The full option is intended to modify the return type of index enumeration APIs (Collection.indexes() and Collection.indexInformation(), but since the return type of Collection.indexExists() this option does not make sense for the Collection.indexExists() helper.

We have removed support for this option.

indexExists(), indexes() and indexInformation() support cursor options in Typescript

These APIs have supported cursor options at runtime since the 4.x version of the driver, but our Typescript has incorrectly omitted cursor options from these APIs.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@baileympearson baileympearson force-pushed the NODE-6019-remove-dead-and-redundant-index-management-code branch from 329f658 to 4930d0e Compare March 15, 2024 17:22
@baileympearson baileympearson changed the title remove EnsureIndexOperation and CreateIndexOperation refactor(NODE-6019): clean up index management code Mar 15, 2024
@baileympearson baileympearson marked this pull request as ready for review March 15, 2024 18:52
@nbbeeken nbbeeken self-assigned this Mar 15, 2024
@nbbeeken nbbeeken added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Mar 15, 2024
src/collection.ts Outdated Show resolved Hide resolved
src/collection.ts Outdated Show resolved Hide resolved
src/collection.ts Outdated Show resolved Hide resolved
src/collection.ts Outdated Show resolved Hide resolved
src/collection.ts Outdated Show resolved Hide resolved
src/operations/indexes.ts Outdated Show resolved Hide resolved
src/operations/create_collection.ts Outdated Show resolved Hide resolved
src/operations/common_functions.ts Outdated Show resolved Hide resolved
src/db.ts Outdated Show resolved Hide resolved
src/db.ts Show resolved Hide resolved
src/collection.ts Outdated Show resolved Hide resolved
src/db.ts Outdated Show resolved Hide resolved
src/operations/create_collection.ts Outdated Show resolved Hide resolved
src/collection.ts Outdated Show resolved Hide resolved
@baileympearson baileympearson force-pushed the NODE-6019-remove-dead-and-redundant-index-management-code branch from cf3906b to 9316bb9 Compare March 18, 2024 17:13
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

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

From slack: this fixes a bug with full:true in indexExists, we should call attention to that with the usual fix and release note pairing 🍷🧀. Should only need PR title and body changes so no need for another CI run. I'll merge when you re-request me. TIA.

@baileympearson baileympearson changed the title refactor(NODE-6019): clean up index management code fix(NODE-6019): indexExists always returns false when full is set to true Mar 18, 2024
@baileympearson baileympearson force-pushed the NODE-6019-remove-dead-and-redundant-index-management-code branch from ded0c23 to 4b054f1 Compare March 18, 2024 21:36
@nbbeeken nbbeeken merged commit 0ebc1ac into main Mar 18, 2024
20 of 25 checks passed
@nbbeeken nbbeeken deleted the NODE-6019-remove-dead-and-redundant-index-management-code branch March 18, 2024 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
2 participants