Skip to content

feat: Allow creating of unique indexes #8671

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

Open
wants to merge 5 commits into
base: alpha
Choose a base branch
from
Open

feat: Allow creating of unique indexes #8671

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ad62422
feat: allow creating of unique indexes
dblythy Jun 27, 2023
29c9878
Update MongoStorageAdapter.js
dblythy Jun 27, 2023
a561edd
Update MongoStorageAdapter.js
dblythy Jun 28, 2023
35591c4
Merge branch 'alpha' into create-indexes
mtrezza Feb 29, 2024
4c328cb
Merge branch 'alpha' into create-indexes
mtrezza Sep 13, 2025
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
100 changes: 100 additions & 0 deletions spec/schemas.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3051,6 +3051,42 @@ describe('schemas', () => {
});
});

it('allows add unique index when you create a class', async () => {
await reconfigureServer({ silent: false });
const response = await request({
url: 'http://localhost:8378/1/schemas',
method: 'POST',
headers: masterKeyHeaders,
json: true,
body: {
className: 'NewClass',
fields: {
aString: { type: 'String' },
},
indexes: {
name1: { aString: 1, _options: { unique: true } },
},
},
});
expect(response.data).toEqual({
className: 'NewClass',
fields: {
ACL: { type: 'ACL' },
createdAt: { type: 'Date' },
updatedAt: { type: 'Date' },
objectId: { type: 'String' },
aString: { type: 'String' },
},
classLevelPermissions: defaultClassLevelPermissions,
indexes: {
name1: { aString: 1, _options: { unique: true } },
},
});
const indexes = await config.database.adapter.getIndexes('NewClass');
expect(indexes.length).toBe(2);
expect(indexes.filter(row => row.unique).length).toEqual(1);
});

Comment on lines +3130 to +3165
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Scope this unique-index test to Mongo and refresh Config after reconfigure.

Postgres adapter doesn’t handle _options yet; this test will likely fail on PG runs. Also, reconfigureServer() replaces the server; refresh config to avoid stale adapter refs.

Apply:

-    it('allows add unique index when you create a class', async () => {
+    it_exclude_dbs(['postgres'])('allows add unique index when you create a class', async () => {
       await reconfigureServer({ silent: false });
+      config = Config.get('test');
@@
-          indexes: {
-            name1: { aString: 1, _options: { unique: true } },
-          },
+          indexes: {
+            // recommend sparse to allow multiple docs without aString
+            name1: { aString: 1, _options: { unique: true, sparse: true } },
+          },

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In spec/schemas.spec.js around lines 3130-3165, the test must be limited to the
Mongo adapter and must refresh the shared config after reconfigureServer to
avoid a stale adapter reference; after calling await reconfigureServer(...)
update the local config reference (e.g. config = await reconfigureServer(...) or
re-require the module that exports config) and add a guard that skips the test
unless config.database.adapter identifies as Mongo (e.g. return or pending if
adapter.name/type !== 'mongo'); then proceed to call
config.database.adapter.getIndexes using the refreshed config.

it('empty index returns nothing', done => {
request({
url: 'http://localhost:8378/1/schemas',
Expand Down Expand Up @@ -3148,6 +3184,70 @@ describe('schemas', () => {
});
});

it('lets you add unique indexes', async () => {
await request({
url: 'http://localhost:8378/1/schemas/NewClass',
method: 'POST',
headers: masterKeyHeaders,
json: true,
body: {},
});
let response = await request({
url: 'http://localhost:8378/1/schemas/NewClass',
method: 'PUT',
headers: masterKeyHeaders,
json: true,
body: {
fields: {
aString: { type: 'String' },
},
indexes: {
name1: { aString: 1, _options: { unique: true } },
},
},
});
expect(
dd(response.data, {
className: 'NewClass',
fields: {
ACL: { type: 'ACL' },
createdAt: { type: 'Date' },
updatedAt: { type: 'Date' },
objectId: { type: 'String' },
aString: { type: 'String' },
},
classLevelPermissions: defaultClassLevelPermissions,
indexes: {
_id_: { _id: 1 },
name1: { aString: 1, _options: { unique: true } },
},
})
).toEqual(undefined);
response = await request({
url: 'http://localhost:8378/1/schemas/NewClass',
headers: masterKeyHeaders,
json: true,
});
expect(response.data).toEqual({
className: 'NewClass',
fields: {
ACL: { type: 'ACL' },
createdAt: { type: 'Date' },
updatedAt: { type: 'Date' },
objectId: { type: 'String' },
aString: { type: 'String' },
},
classLevelPermissions: defaultClassLevelPermissions,
indexes: {
_id_: { _id: 1 },
name1: { aString: 1, _options: { unique: true } },
},
});
const indexes = await config.database.adapter.getIndexes('NewClass');
expect(indexes.length).toEqual(2);
expect(indexes.filter(row => row.unique).length).toEqual(1);
});

Comment on lines +3263 to +3326
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Gate the “add unique indexes” test for Mongo; consider asserting sparse semantics.

Mirror DB scoping to keep CI green on PG. Optionally include sparse: true to match intended nullable-unique behavior.

Apply:

-    it('lets you add unique indexes', async () => {
+    it_exclude_dbs(['postgres'])('lets you add unique indexes', async () => {
@@
-          indexes: {
-            name1: { aString: 1, _options: { unique: true } },
-          },
+          indexes: {
+            name1: { aString: 1, _options: { unique: true, sparse: true } },
+          },

Optionally, follow up by saving two objects without aString to verify no duplicate errors are thrown.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('lets you add unique indexes', async () => {
await request({
url: 'http://localhost:8378/1/schemas/NewClass',
method: 'POST',
headers: masterKeyHeaders,
json: true,
body: {},
});
let response = await request({
url: 'http://localhost:8378/1/schemas/NewClass',
method: 'PUT',
headers: masterKeyHeaders,
json: true,
body: {
fields: {
aString: { type: 'String' },
},
indexes: {
name1: { aString: 1, _options: { unique: true } },
},
},
});
expect(
dd(response.data, {
className: 'NewClass',
fields: {
ACL: { type: 'ACL' },
createdAt: { type: 'Date' },
updatedAt: { type: 'Date' },
objectId: { type: 'String' },
aString: { type: 'String' },
},
classLevelPermissions: defaultClassLevelPermissions,
indexes: {
_id_: { _id: 1 },
name1: { aString: 1, _options: { unique: true } },
},
})
).toEqual(undefined);
response = await request({
url: 'http://localhost:8378/1/schemas/NewClass',
headers: masterKeyHeaders,
json: true,
});
expect(response.data).toEqual({
className: 'NewClass',
fields: {
ACL: { type: 'ACL' },
createdAt: { type: 'Date' },
updatedAt: { type: 'Date' },
objectId: { type: 'String' },
aString: { type: 'String' },
},
classLevelPermissions: defaultClassLevelPermissions,
indexes: {
_id_: { _id: 1 },
name1: { aString: 1, _options: { unique: true } },
},
});
const indexes = await config.database.adapter.getIndexes('NewClass');
expect(indexes.length).toEqual(2);
expect(indexes.filter(row => row.unique).length).toEqual(1);
});
it_exclude_dbs(['postgres'])('lets you add unique indexes', async () => {
await request({
url: 'http://localhost:8378/1/schemas/NewClass',
method: 'POST',
headers: masterKeyHeaders,
json: true,
body: {},
});
let response = await request({
url: 'http://localhost:8378/1/schemas/NewClass',
method: 'PUT',
headers: masterKeyHeaders,
json: true,
body: {
fields: {
aString: { type: 'String' },
},
indexes: {
name1: { aString: 1, _options: { unique: true, sparse: true } },
},
},
});
expect(
dd(response.data, {
className: 'NewClass',
fields: {
ACL: { type: 'ACL' },
createdAt: { type: 'Date' },
updatedAt: { type: 'Date' },
objectId: { type: 'String' },
aString: { type: 'String' },
},
classLevelPermissions: defaultClassLevelPermissions,
indexes: {
_id_: { _id: 1 },
name1: { aString: 1, _options: { unique: true, sparse: true } },
},
})
).toEqual(undefined);
response = await request({
url: 'http://localhost:8378/1/schemas/NewClass',
headers: masterKeyHeaders,
json: true,
});
expect(response.data).toEqual({
className: 'NewClass',
fields: {
ACL: { type: 'ACL' },
createdAt: { type: 'Date' },
updatedAt: { type: 'Date' },
objectId: { type: 'String' },
aString: { type: 'String' },
},
classLevelPermissions: defaultClassLevelPermissions,
indexes: {
_id_: { _id: 1 },
name1: { aString: 1, _options: { unique: true, sparse: true } },
},
});
const indexes = await config.database.adapter.getIndexes('NewClass');
expect(indexes.length).toEqual(2);
expect(indexes.filter(row => row.unique).length).toEqual(1);
});
🤖 Prompt for AI Agents 
In spec/schemas.spec.js around lines 3263-3326 the "lets you add unique indexes"
test should be gated to run only for Mongo (to avoid PG CI failures) and should
assert the sparse behavior for nullable-unique fields: wrap the test or its body
in a runtime check for the database adapter being Mongo (or process.env DB ===
'mongodb'), change the expected index metadata to include sparse: true (i.e.
expect a unique sparse index), and add an optional follow-up step saving two
objects without aString to ensure no duplicate-key error is thrown; this keeps
Postgres CI green while verifying intended nullable-unique semantics for Mongo.

it_only_db('mongo')('lets you add index with with pointer like structure', done => {
request({
url: 'http://localhost:8378/1/schemas/NewClass',
Expand Down
15 changes: 13 additions & 2 deletions src/Adapters/Storage/Mongo/MongoStorageAdapter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import MongoCollection from './MongoCollection';
import MongoSchemaCollection from './MongoSchemaCollection';
import { StorageAdapter } from '../StorageAdapter';
import deepcopy from 'deepcopy';
import type { SchemaType, QueryType, StorageClass, QueryOptions } from '../StorageAdapter';
import { parse as parseUrl, format as formatUrl } from '../../../vendor/mongodbUrl';
import {
Expand Down Expand Up @@ -274,7 +275,12 @@ export class MongoStorageAdapter implements StorageAdapter {
const deletePromises = [];
const insertedIndexes = [];
Object.keys(submittedIndexes).forEach(name => {
const field = submittedIndexes[name];
const field = deepcopy(submittedIndexes[name]);
let indexOptions = {};
if (field._options) {
indexOptions = field._options;
delete field._options;
}
Comment on lines +276 to +281
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: small opti, actually you only need to deep copy if __options is used

if (existingIndexes[name] && field.__op !== 'Delete') {
throw new Parse.Error(Parse.Error.INVALID_QUERY, `Index ${name} exists, cannot update.`);
}
Expand Down Expand Up @@ -302,11 +308,16 @@ export class MongoStorageAdapter implements StorageAdapter {
);
}
});
existingIndexes[name] = field;
insertedIndexes.push({
key: field,
name,
...indexOptions,
});
Comment on lines 309 to 313
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Don’t let _options.name override the declared index name.

Spreading indexOptions after name allows callers to override the index name, desynchronizing _SCHEMA._metadata.indexes from the actual DB index and breaking future drop/update operations.

Apply this diff to sanitize or de-prioritize name from _options:

-        insertedIndexes.push({
-          key: field,
-          name,
-          ...indexOptions,
-        });
+        const { name: _ignoredIndexName, ...sanitizedOptions } = indexOptions;
+        insertedIndexes.push({
+          key: field,
+          name,
+          ...sanitizedOptions,
+        });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
insertedIndexes.push({
key: field,
name,
...indexOptions,
});
const { name: _ignoredIndexName, ...sanitizedOptions } = indexOptions;
insertedIndexes.push({
key: field,
name,
...sanitizedOptions,
});
🤖 Prompt for AI Agents 
In src/Adapters/Storage/Mongo/MongoStorageAdapter.js around lines 309 to 313,
the current object spread places ...indexOptions after name which allows callers
to override the declared index name; remove or neutralize any name property
coming from indexOptions before merging (either delete indexOptions.name or
spread indexOptions first and then set name explicitly) so the declared name
cannot be overridden and _SCHEMA._metadata.indexes stays in sync with the DB.

const fieldCopy = deepcopy(field);
if (Object.keys(indexOptions).length) {
fieldCopy._options = indexOptions;
}
existingIndexes[name] = fieldCopy;
}
});
let insertPromise = Promise.resolve();
Expand Down