Skip to content

Commit

Permalink
extract the ID field using extractField
Browse files Browse the repository at this point in the history
This makes the ID field extraction consistent with other fields, making
it possible to customize the extraction logic.

_Note_: a test case that was asserting a special behavior for the ID
field extraction when it matches a default object property name (like
"constructor") was removed. It's better to let the user specify their
own extractField function instead of second guessing them. This is not
considered to be a breaking change, as the existing behavior was not
publicly documented, and would only take place in corner cases (when the
idField is set to a default object property name like "constructor",
which should be discouraged anyway).
  • Loading branch information
lucaong committed Aug 13, 2020
1 parent 7622a16 commit 2e70b76
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 17 deletions.
12 changes: 7 additions & 5 deletions src/MiniSearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,11 @@ class MiniSearch {
*/
add (document) {
const { extractField, tokenize, processTerm, fields, idField } = this._options
if (getOwnProperty(document, idField) == null) {
const id = extractField(document, idField)
if (id == null) {
throw new Error(`MiniSearch: document does not have ID field "${idField}"`)
}
const shortDocumentId = addDocumentId(this, document[idField])
const shortDocumentId = addDocumentId(this, id)
saveStoredFields(this, shortDocumentId, document)

fields.forEach(field => {
Expand Down Expand Up @@ -253,16 +254,17 @@ class MiniSearch {
*/
remove (document) {
const { tokenize, processTerm, extractField, fields, idField } = this._options
const id = extractField(document, idField)

if (getOwnProperty(document, idField) == null) {
if (id == null) {
throw new Error(`MiniSearch: document does not have ID field "${idField}"`)
}

const [shortDocumentId] = Object.entries(this._documentIds)
.find(([_, longId]) => document[idField] === longId) || []
.find(([_, longId]) => id === longId) || []

if (shortDocumentId == null) {
throw new Error(`MiniSearch: cannot remove document with ID ${document[idField]}: it is not in the index`)
throw new Error(`MiniSearch: cannot remove document with ID ${id}: it is not in the index`)
}

fields.filter(field => getOwnProperty(document, field) != null).forEach(field => {
Expand Down
23 changes: 11 additions & 12 deletions src/MiniSearch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,17 @@ describe('MiniSearch', () => {
}).toThrowError('MiniSearch: document does not have ID field "foo"')
})

it('throws error if the document does not have the ID field, even when named like a default object property', () => {
const ms = new MiniSearch({ idField: 'constructor', fields: ['title', 'text'] })
expect(() => {
ms.add({ text: 'I do not have an ID' })
}).toThrowError('MiniSearch: document does not have ID field "constructor"')
it('extracts the ID field using extractField', () => {
const extractField = (document, fieldName) => {
if (fieldName === 'id') { return document['id']['value'] }
return MiniSearch.getDefault('extractField')(document, fieldName)
}
const ms = new MiniSearch({ fields: ['text'], extractField })

ms.add({ id: { value: 123 }, text: 'Nel mezzo del cammin di nostra vita' })

const results = ms.search('vita')
expect(results[0].id).toEqual(123)
})

it('rejects falsy terms', () => {
Expand Down Expand Up @@ -181,13 +187,6 @@ describe('MiniSearch', () => {
}).toThrowError('MiniSearch: document does not have ID field "foo"')
})

it('throws error if the document does not have the ID field, even if named like a default property of object', () => {
const ms = new MiniSearch({ idField: 'constructor', fields: ['title', 'text'] })
expect(() => {
ms.remove({ text: 'I do not have an ID' })
}).toThrowError('MiniSearch: document does not have ID field "constructor"')
})

it('does not crash when the document has field named like default properties of object', () => {
const ms = new MiniSearch({ fields: ['constructor'] })
const document = { id: 1 }
Expand Down

0 comments on commit 2e70b76

Please sign in to comment.