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

Add filtering on deleteDocuments for Meilisearch v1.2 #1492

Merged
merged 5 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ module.exports = {
es2020: true,
'jest/globals': true,
node: true,
jasmine: true,
},
extends: [
'eslint:recommended',
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ client.index('myIndex').deleteDocument(documentId: string | number): Promise<Enq
#### [Delete multiple documents](https://docs.meilisearch.com/reference/api/documents.html#delete-documents)

```ts
client.index('myIndex').deleteDocuments(documentsIds: string[] | number[]): Promise<EnqueuedTask>
client.index('myIndex').deleteDocuments(params: DocumentsDeletionQuery | DocumentsIds): Promise<EnqueuedTask>
```

#### [Delete all documents](https://docs.meilisearch.com/reference/api/documents.html#delete-all-documents)
Expand Down
1 change: 1 addition & 0 deletions src/errors/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ export * from './meilisearch-api-error'
export * from './meilisearch-communication-error'
export * from './meilisearch-error'
export * from './meilisearch-timeout-error'
export * from './version-hint-message'
3 changes: 3 additions & 0 deletions src/errors/version-hint-message.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function versionErrorHintMessage(message: string, method: string) {
return `${message}\nHint: It might not be working because maybe you're not up to date with the Meilisearch version that ${method} call requires.`
}
97 changes: 73 additions & 24 deletions src/indexes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@

'use strict'

import { MeiliSearchError } from './errors'

import {
MeiliSearchError,
MeiliSearchCommunicationError,
versionErrorHintMessage,
MeiliSearchApiError,
} from './errors'
import {
Config,
SearchResponse,
Expand Down Expand Up @@ -39,6 +43,8 @@ import {
ResourceResults,
RawDocumentAdditionOptions,
ContentType,
DocumentsIds,
DocumentsDeletionQuery,
} from './types'
import { removeUndefinedFromObject } from './utils'
import { HttpRequests } from './http-requests'
Expand Down Expand Up @@ -302,30 +308,49 @@ class Index<T extends Record<string, any> = Record<string, any>> {
///

/**
* Get documents of an index
* Get documents of an index.
*
* @param parameters - Parameters to browse the documents
* @returns Promise containing Document responses
* @param parameters - Parameters to browse the documents. Parameters can
* contain the `filter` field only available in Meilisearch v1.2 and newer
* @returns Promise containing the returned documents
*/
async getDocuments<D extends Record<string, any> = T>(
parameters: DocumentsQuery<D> = {}
): Promise<ResourceResults<D[]>> {
const url = `indexes/${this.uid}/documents`

const fields = (() => {
if (Array.isArray(parameters?.fields)) {
return parameters?.fields?.join(',')
parameters = removeUndefinedFromObject(parameters)

// In case `filter` is provided, use `POST /documents/fetch`
if (parameters.filter !== undefined) {
try {
const url = `indexes/${this.uid}/documents/fetch`

return await this.httpRequest.post<
DocumentsQuery,
Promise<ResourceResults<D[]>>
>(url, parameters)
} catch (e) {
if (e instanceof MeiliSearchCommunicationError) {
e.message = versionErrorHintMessage(e.message, 'getDocuments')
} else if (e instanceof MeiliSearchApiError) {
e.message = versionErrorHintMessage(e.message, 'getDocuments')
}

throw e
}
return undefined
})()
// Else use `GET /documents` method
} else {
const url = `indexes/${this.uid}/documents`

return await this.httpRequest.get<Promise<ResourceResults<D[]>>>(
url,
removeUndefinedFromObject({
// Transform fields to query parameter string format
const fields = Array.isArray(parameters?.fields)
? { fields: parameters?.fields?.join(',') }
: {}

return await this.httpRequest.get<Promise<ResourceResults<D[]>>>(url, {
...parameters,
fields,
...fields,
})
)
}
}

/**
Expand Down Expand Up @@ -503,19 +528,43 @@ class Index<T extends Record<string, any> = Record<string, any>> {
}

/**
* Delete multiples documents of an index
* Delete multiples documents of an index.
*
* @param params - Params value can be:
*
* - DocumentsDeletionQuery: An object containing the parameters to customize
* your document deletion. Only available in Meilisearch v1.2 and newer
* - DocumentsIds: An array of document ids to delete
*
* @param documentsIds - Array of Document Ids to delete
* @returns Promise containing an EnqueuedTask
*/
async deleteDocuments(
documentsIds: string[] | number[]
params: DocumentsDeletionQuery | DocumentsIds
): Promise<EnqueuedTask> {
const url = `indexes/${this.uid}/documents/delete-batch`

const task = await this.httpRequest.post(url, documentsIds)
// If params is of type DocumentsDeletionQuery
const isDocumentsDeletionQuery =
!Array.isArray(params) && typeof params === 'object'
const endpoint = isDocumentsDeletionQuery
? 'documents/delete'
: 'documents/delete-batch'
const url = `indexes/${this.uid}/${endpoint}`

try {
const task = await this.httpRequest.post(url, params)
Copy link
Member

Choose a reason for hiding this comment

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

Does the response of this post method vary?

Copy link
Member

Choose a reason for hiding this comment

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

I thought it could be only a task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is always a task, but if the request fails it goes to the catch

Copy link
Member

Choose a reason for hiding this comment

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

So, why do you need to create a new instance of the task? Since it is already returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One is a plain object one is an instance of the Task Class


return new EnqueuedTask(task)
} catch (e) {
if (
e instanceof MeiliSearchCommunicationError &&
isDocumentsDeletionQuery
) {
e.message = versionErrorHintMessage(e.message, 'deleteDocuments')
} else if (e instanceof MeiliSearchApiError) {
e.message = versionErrorHintMessage(e.message, 'deleteDocuments')
}

return new EnqueuedTask(task)
throw e
bidoubiwa marked this conversation as resolved.
Show resolved Hide resolved
}
}

/**
Expand Down
13 changes: 13 additions & 0 deletions src/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,19 @@ export type RawDocumentAdditionOptions = DocumentOptions & {

export type DocumentsQuery<T = Record<string, any>> = ResourceQuery & {
fields?: Fields<T>
filter?: Filter
}

export type DocumentQuery<T = Record<string, any>> = {
fields?: Fields<T>
}

export type DocumentsDeletionQuery = {
filter: Filter
}

export type DocumentsIds = string[] | number[]

/*
** Settings
*/
Expand Down Expand Up @@ -549,6 +556,12 @@ export const enum ErrorStatusCode {
/** @see https://docs.meilisearch.com/errors/#invalid_document_offset */
INVALID_DOCUMENT_OFFSET = 'invalid_document_offset',

/** @see https://docs.meilisearch.com/errors/#invalid_document_offset */
INVALID_DOCUMENT_FILTER = 'invalid_document_filter',

/** @see https://docs.meilisearch.com/errors/#invalid_document_offset */
MISSING_DOCUMENT_FILTER = 'missing_document_filter',

/** @see https://docs.meilisearch.com/errors/#payload_too_large */
PAYLOAD_TOO_LARGE = 'payload_too_large',

Expand Down
130 changes: 130 additions & 0 deletions tests/documents.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
getClient,
dataset,
Book,
getKey,
HOST,
} from './utils/meilisearch-test-utils'

const indexNoPk = {
Expand Down Expand Up @@ -140,6 +142,57 @@ describe('Documents tests', () => {
expect(documents.offset).toEqual(2)
})

test(`${permission} key: Get documents with filters`, async () => {
const client = await getClient(permission)
await client.index(indexPk.uid).updateFilterableAttributes(['id'])
const { taskUid } = await client
.index(indexPk.uid)
.addDocuments(dataset)
await client.waitForTask(taskUid)

const documents = await client.index(indexPk.uid).getDocuments<Book>({
filter: [['id = 1', 'id = 2']],
})

expect(documents.results.length).toEqual(2)
})

test(`${permission} key: Get documents should trigger error with a MeilisearchCommunicationError`, async () => {
const apiKey = await getKey(permission)
const client = new MeiliSearch({ host: `${HOST}/indexes`, apiKey })

try {
await client.index(indexPk.uid).getDocuments({ filter: '' })

fail(
'getDocuments should have raised an error when the route does not exist'
)
} catch (e: any) {
expect(e.message).toEqual(
"Not Found\nHint: It might not be working because maybe you're not up to date with the Meilisearch version that getDocuments call requires."
)
}
})

test(`${permission} key: Get documents should trigger error with a hint on a MeilisearchApiError`, async () => {
const apiKey = await getKey(permission)
const client = new MeiliSearch({ host: `${HOST}`, apiKey })

try {
await client.index(indexPk.uid).getDocuments({ filter: 'id = 1' })

fail(
'getDocuments should have raised an error when the route does not exist'
)
} catch (e: any) {
expect(e.message).toEqual(
`Attribute \`id\` is not filterable. This index does not have configured filterable attributes.
1:3 id = 1
Hint: It might not be working because maybe you're not up to date with the Meilisearch version that getDocuments call requires.`
)
}
})

test(`${permission} key: Get documents from index that has NO primary key`, async () => {
const client = await getClient(permission)
const { taskUid } = await client
Expand Down Expand Up @@ -417,6 +470,48 @@ describe('Documents tests', () => {
expect(response.results.length).toEqual(dataset.length)
})

test(`${permission} key: Delete some documents with string filters`, async () => {
const client = await getClient(permission)
await client.index(indexPk.uid).updateFilterableAttributes(['id'])
const { taskUid: addDocTask } = await client
.index(indexPk.uid)
.addDocuments(dataset)
await client.index(indexPk.uid).waitForTask(addDocTask)

const task = await client
.index(indexPk.uid)
.deleteDocuments({ filter: 'id IN [1, 2]' })

const resolvedTask = await client
.index(indexPk.uid)
.waitForTask(task.taskUid)
const documents = await client.index(indexPk.uid).getDocuments<Book>()

expect(resolvedTask.details.deletedDocuments).toEqual(2)
expect(documents.results.length).toEqual(dataset.length - 2)
})

test(`${permission} key: Delete some documents with array filters`, async () => {
const client = await getClient(permission)
await client.index(indexPk.uid).updateFilterableAttributes(['id'])
const { taskUid: addDocTask } = await client
.index(indexPk.uid)
.addDocuments(dataset)
await client.index(indexPk.uid).waitForTask(addDocTask)

const task = await client
.index(indexPk.uid)
.deleteDocuments({ filter: [['id = 1', 'id = 2']] })

const resolvedTask = await client
.index(indexPk.uid)
.waitForTask(task.taskUid)
const documents = await client.index(indexPk.uid).getDocuments<Book>()

expect(resolvedTask.details.deletedDocuments).toEqual(2)
expect(documents.results.length).toEqual(dataset.length - 2)
})

test(`${permission} key: Delete some documents from index that has NO primary key`, async () => {
const client = await getClient(permission)
const { taskUid: addDocTask } = await client
Expand Down Expand Up @@ -458,6 +553,41 @@ describe('Documents tests', () => {
expect(returnedIds).not.toContain(ids[1])
})

test(`${permission} key: Delete some documents should trigger error with a hint on a MeilisearchApiError`, async () => {
const client = await getClient(permission)
const task = await client.createIndex(indexPk.uid)
await client.waitForTask(task.taskUid)

try {
await client.index(indexPk.uid).deleteDocuments({ filter: '' })

fail(
'deleteDocuments should have raised an error when the parameters are wrong'
)
} catch (e: any) {
expect(e.message).toEqual(
"Sending an empty filter is forbidden.\nHint: It might not be working because maybe you're not up to date with the Meilisearch version that deleteDocuments call requires."
)
}
})

test(`${permission} key: Delete some documents should trigger error with a hint on a MeilisearchCommunicationError`, async () => {
const apiKey = await getKey(permission)
const client = new MeiliSearch({ host: `${HOST}/indexes`, apiKey })

try {
await client.index(indexPk.uid).deleteDocuments({ filter: 'id = 1' })

fail(
'deleteDocuments should have raised an error when the route does not exist'
)
} catch (e: any) {
expect(e.message).toEqual(
"Not Found\nHint: It might not be working because maybe you're not up to date with the Meilisearch version that deleteDocuments call requires."
)
}
})

test(`${permission} key: Delete all document from index that has NO primary key`, async () => {
const client = await getClient(permission)
const task = await client.index(indexNoPk.uid).deleteAllDocuments()
Expand Down