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

fix: partial index are updated during startup #437

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: partial index are updated during startup #437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4abe093
fix: partial index are updated during startup
flower-of-the-bridges Dec 30, 2024
e5f65c0
chore: add mongodb version to index tests
flower-of-the-bridges Dec 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions lib/createIndexes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ const {
GEO_INDEX,
TEXT_INDEX,
} = require('./consts')
const { isEqual } = require('lodash')

const addKeyToSpec = (specObject, { name, order }) => ({ ...specObject, [name]: order })
const addTextKeyToSpec = (specObject, { name }) => ({ ...specObject, [name]: TEXT_FIELD })
Expand Down Expand Up @@ -115,24 +116,31 @@ function getIndexOptions(index) {
options = { ...options, language_override: index.languageOverride }
}
if (index.usePartialFilter) {
let partialFilterExpression
try {
partialFilterExpression = index.partialFilterExpression ? JSON.parse(index.partialFilterExpression) : {}
} catch (error) {
throw new Error(`Impossible to parse the Partial Index expression of index ${index.name}`, { catch: error })
}

const partialFilterExpression = parsePartialIndex(index)
options = { ...options, partialFilterExpression }
}
return options
}

function parsePartialIndex(index) {
try {
return index.partialFilterExpression ? JSON.parse(index.partialFilterExpression) : {}
} catch (error) {
throw new Error(`Impossible to parse the Partial Index expression of index ${index.name}`, { catch: error })
}
}

function checkIndexEquality(foundIndex, index) {
const isOptionUniqueEqual = index.unique ? foundIndex.unique === true : foundIndex.unique === undefined
const isOptionUniqueEqual = Boolean(index.unique) === Boolean(foundIndex.unique)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it correct that isOptionUniqueEqual is set to true whenever both properties have a falsy value?

In theory, the outcome should be true in case both values are either true or undefined. However, when both are set to false, the expression would be evaluated to true, leading to an incorrect behavior 🤔

const isOptionExpireAfterSecondsEqual = foundIndex.expireAfterSeconds === index.expireAfterSeconds
const isOptionPartialFilterEqual = isEqual(
foundIndex.partialFilterExpression || {},
parsePartialIndex(index)
)
return (
isOptionUniqueEqual
&& isOptionExpireAfterSecondsEqual
&& isOptionPartialFilterEqual
&& checkIndexEqualityByType(foundIndex, index)
)
}
Expand Down
194 changes: 171 additions & 23 deletions tests/createIndexes.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1050,40 +1050,188 @@ tap.test('createIndexes', async t => {
],
expectedIndexCreatedCount: 1,
},
{
name: 'partial index with already present one (but not partial)',
alreadyPresentIndexes: [
{
spec: {
name: 1,
},
options: {
name: nameIndex1,
unique: true,
},
},
],
indexes: [
{
name: nameIndex1,
type: NORMAL_INDEX,
unique: true,
fields: [
{ name: 'name',
order: 1,
},
],
usePartialFilter: true,
partialFilterExpression: '{ "name": { "$eq": "test" } }',
},
],
expectedIndexes: [
{
v: 2,
key: {
name: 1,
},
name: nameIndex1,
background: true,
unique: true,
partialFilterExpression: { name: { $eq: 'test' } },
},
],
expectedIndexCreatedCount: 1,
},
{
name: 'complex partial index not replaced',
// nested operators in partial indexes are not supported on mongodb:5.0
minMongoDbVersion: '6.0',
alreadyPresentIndexes: [
{
spec: { b: 1, d: 1 },
options: {
name: nameIndex1,
unique: true,
partialFilterExpression: { $or: [{ b: 'c' }, { d: 'e' }] },
},
},
],
indexes: [
{
name: nameIndex1,
type: NORMAL_INDEX,
unique: true,
fields: [
{
name: 'b',
order: 1,
},
{
name: 'd',
order: 1,
},
],
usePartialFilter: true,
partialFilterExpression: '{"$or":[{"b":"c"},{"d":"e"}]}',
},
],
expectedIndexes: [
{
v: 2,
key: {
b: 1,
d: 1,
},
name: nameIndex1,
unique: true,
partialFilterExpression: { $or: [{ b: 'c' }, { d: 'e' }] },
},
],
expectedIndexCreatedCount: 0,
},
{
name: 'complex partial replaced',
// nested operators in partial indexes are not supported on mongodb:5.0
minMongoDbVersion: '6.0',
alreadyPresentIndexes: [
{
spec: { b: 1, d: 1 },
options: {
name: nameIndex1,
unique: true,
partialFilterExpression: { $or: [{ b: 'c' }, { d: 'e' }] },
},
},
],
indexes: [
{
name: nameIndex1,
type: NORMAL_INDEX,
unique: true,
fields: [
{
name: 'b',
order: 1,
},
{
name: 'd',
order: 1,
},
],
usePartialFilter: true,
partialFilterExpression: '{"$or":[{"b":"c"},{"d":"g"}]}',
},
],
expectedIndexes: [
{
v: 2,
key: {
b: 1,
d: 1,
},
background: true,
name: nameIndex1,
unique: true,
partialFilterExpression: { $or: [{ b: 'c' }, { d: 'g' }] },
},
],
expectedIndexCreatedCount: 1,
},
]

t.plan(testConfigs.length)
const MIN_MONGODB_VERSION = '5.0'
const MONGODB_VERSION = process.env.MONGO_VERSION || MIN_MONGODB_VERSION
// RUN TESTS ACCORDING TO MONGO_VERSION env var, since
// nested operators in partial indexes are not supported on mongodb:5.0
t.plan(
testConfigs
.filter(({ minMongoDbVersion = MIN_MONGODB_VERSION }) => MONGODB_VERSION >= minMongoDbVersion)
.length
)
testConfigs.forEach(({
name,
minMongoDbVersion = MIN_MONGODB_VERSION,
alreadyPresentIndexes,
indexes,
expectedIndexes,
expectedIndexCreatedCount,
}) => {
t.test(name, async t => {
t.plan(2 + expectedIndexes.length)
await collection.drop()
await database.createCollection(BOOKS_COLLECTION_NAME)
await Promise.all(alreadyPresentIndexes
.map(({ spec, options }) => collection.createIndex(spec, options)))
const createdIndexNames = await createIndexes(collection, indexes, 'preserve_')
const retIndexes = await collection.indexes()
if (MONGODB_VERSION >= minMongoDbVersion) {
t.test(name, async t => {
t.plan(2 + expectedIndexes.length)
await collection.drop()
await database.createCollection(BOOKS_COLLECTION_NAME)

await Promise.all(alreadyPresentIndexes
.map(({ spec, options }) => collection.createIndex(spec, options)))
const createdIndexNames = await createIndexes(collection, indexes, 'preserve_')
const retIndexes = await collection.indexes()

/* Starting in MongoDB 4.4 the `ns` field is no more included in index objects.
* Since we are not using `ns` field in this service we remove it in the below line.
* In this way we guarantee compatibility with MongoDB 4.4 and the previous MongoDB versions.
* https://docs.mongodb.com/manual/reference/method/db.collection.getIndexes/
*/
retIndexes.forEach(index => delete index.ns)
// sort by name because the return order is not deterministic with background indexes
// skip the first returned index as it is always the _id one
t.strictSame(retIndexes.slice(1).sort(by('name')), expectedIndexes.sort(by('name')))
retIndexes.slice(1).forEach(retIndex => {
const index = expectedIndexes.find(expectedIndex => expectedIndex.name === retIndex.name)
t.ok(JSON.stringify(index.key) === JSON.stringify(retIndex.key))
/* Starting in MongoDB 4.4 the `ns` field is no more included in index objects.
* Since we are not using `ns` field in this service we remove it in the below line.
* In this way we guarantee compatibility with MongoDB 4.4 and the previous MongoDB versions.
* https://docs.mongodb.com/manual/reference/method/db.collection.getIndexes/
*/
retIndexes.forEach(index => delete index.ns)
// sort by name because the return order is not deterministic with background indexes
// skip the first returned index as it is always the _id one
t.strictSame(retIndexes.slice(1).sort(by('name')), expectedIndexes.sort(by('name')))
retIndexes.slice(1).forEach(retIndex => {
const index = expectedIndexes.find(expectedIndex => expectedIndex.name === retIndex.name)
t.ok(JSON.stringify(index.key) === JSON.stringify(retIndex.key))
})
t.equal(createdIndexNames.length, expectedIndexCreatedCount)
})
t.equal(createdIndexNames.length, expectedIndexCreatedCount)
})
}
})
})

Expand Down
Loading