Skip to content

Commit

Permalink
fix: addCases performance (#1240)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kswenson
kswenson authored May 2, 2024
1 parent bf064b0 commit ba23fb5
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 68 deletions.
6 changes: 3 additions & 3 deletions v3/src/models/data/data-set-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ export interface IGetCasesOptions extends IGetCaseOptions {
count?: number;
}
export interface IAddCaseOptions {
// id(s) of case(s) before which to insert new cases
// id of case before/after which to insert new cases
// if not specified, new cases are appended
before?: string | string[];
after?: string | string[];
before?: string;
after?: string;
}

export interface IAddAttributeOptions {
Expand Down
44 changes: 22 additions & 22 deletions v3/src/models/data/data-set.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,28 +308,28 @@ test("DataSet basic functionality", () => {
expect((caseIndex >= 0) ? dataset.cases[caseIndex].__id__ : "").toBe(aCase.__id__)
})

// add multiple new cases after specified case
dataset.addCases(toCanonical(dataset, [{ str: "j", num: 1 }, { str: "k", num: 2 }]), { after: caseC3ID })
const caseJ1ID = dataset.cases[4].__id__,
caseK2ID = dataset.cases[3].__id__
expect(dataset.cases.length).toBe(7)
expect(dataset.attributes[0].value(4)).toBe("j")
expect(dataset.attributes[1].value(4)).toBe("1")
expect(dataset.attributes[0].value(3)).toBe("k")
expect(dataset.attributes[1].value(3)).toBe("2")
expect(dataset.getValue(caseJ1ID, "foo")).toBeUndefined()
expect(dataset.getValue("foo", "bar")).toBeUndefined()
expect(dataset.getCase(caseJ1ID, { canonical: false })).toEqual({ __id__: caseJ1ID, str: "j", num: 1 })
expect(dataset.getCase(caseK2ID, { canonical: false })).toEqual({ __id__: caseK2ID, str: "k", num: 2 })
expect(dataset.getCase(caseJ1ID, { canonical: true }))
.toEqual({ __id__: caseJ1ID, [strAttrID]: "j", [numAttrID]: 1 })
expect(dataset.getCase(caseK2ID, { canonical: true }))
.toEqual({ __id__: caseK2ID, [strAttrID]: "k", [numAttrID]: 2 })
expect(dataset.getCases([caseJ1ID, caseK2ID], { canonical: true }))
.toEqual([{ __id__: caseJ1ID, [strAttrID]: "j", [numAttrID]: 1 },
{ __id__: caseK2ID, [strAttrID]: "k", [numAttrID]: 2 }])
expect(dataset.getCasesAtIndex().length).toBe(7)
expect(dataset.getCasesAtIndex(4).length).toBe(3)
// add multiple new cases after specified case
dataset.addCases(toCanonical(dataset, [{ str: "j", num: 1 }, { str: "k", num: 2 }]), { after: caseC3ID })
const caseJ1ID = dataset.cases[3].__id__,
caseK2ID = dataset.cases[4].__id__
expect(dataset.cases.length).toBe(7)
expect(dataset.attributes[0].value(3)).toBe("j")
expect(dataset.attributes[1].value(3)).toBe("1")
expect(dataset.attributes[0].value(4)).toBe("k")
expect(dataset.attributes[1].value(4)).toBe("2")
expect(dataset.getValue(caseJ1ID, "foo")).toBeUndefined()
expect(dataset.getValue("foo", "bar")).toBeUndefined()
expect(dataset.getCase(caseJ1ID, { canonical: false })).toEqual({ __id__: caseJ1ID, str: "j", num: 1 })
expect(dataset.getCase(caseK2ID, { canonical: false })).toEqual({ __id__: caseK2ID, str: "k", num: 2 })
expect(dataset.getCase(caseJ1ID, { canonical: true }))
.toEqual({ __id__: caseJ1ID, [strAttrID]: "j", [numAttrID]: 1 })
expect(dataset.getCase(caseK2ID, { canonical: true }))
.toEqual({ __id__: caseK2ID, [strAttrID]: "k", [numAttrID]: 2 })
expect(dataset.getCases([caseJ1ID, caseK2ID], { canonical: true }))
.toEqual([{ __id__: caseJ1ID, [strAttrID]: "j", [numAttrID]: 1 },
{ __id__: caseK2ID, [strAttrID]: "k", [numAttrID]: 2 }])
expect(dataset.getCasesAtIndex().length).toBe(7)
expect(dataset.getCasesAtIndex(4).length).toBe(3)

// setCaseValues
dataset.setCaseValues(toCanonical(dataset, [{ __id__: caseA1ID, str: "A", num: 10 }]))
Expand Down
88 changes: 45 additions & 43 deletions v3/src/models/data/data-set.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -677,41 +677,6 @@ export const DataSet = V2Model.named("DataSet").props({
return id ? getCase(id, options) : undefined
}

function beforeIndexForInsert(index: number, beforeID?: string | string[]) {
if (!beforeID) { return self.cases.length }
return Array.isArray(beforeID)
? self.caseIDMap.get(beforeID[index])
: self.caseIDMap.get(beforeID)
}

function afterIndexForInsert(index: number, afterID?: string | string[]) {
if (!afterID) { return self.cases.length }
return Array.isArray(afterID)
? (self.caseIDMap.get(afterID[index]) || 0) + 1
: (self.caseIDMap.get(afterID) || 0) + 1
}

function insertCaseIDAtIndex(id: string, beforeIndex: number) {
const newCase = { __id__: id }
if ((beforeIndex != null) && (beforeIndex < self.cases.length)) {
self.cases.splice(beforeIndex, 0, newCase)
// increment indices of all subsequent cases
for (let i = beforeIndex + 1; i < self.cases.length; ++i) {
const aCase = self.cases[i]
const currentVal = self.caseIDMap.get(aCase.__id__)
if (currentVal != null) {
self.caseIDMap.set(aCase.__id__, currentVal + 1)
}
}
}
else {
self.cases.push(newCase)
beforeIndex = self.cases.length - 1
}
self.caseIDMap.set(self.cases[beforeIndex].__id__, beforeIndex)

}

function setCaseValues(caseValues: ICase) {
const index = self.caseIDMap.get(caseValues.__id__)
if (index == null) { return }
Expand Down Expand Up @@ -972,18 +937,55 @@ export const DataSet = V2Model.named("DataSet").props({

addCases(cases: ICaseCreation[], options?: IAddCaseOptions) {
const { before, after } = options || {}

const beforePosition = before ? self.caseIDMap.get(before) : undefined
const _afterPosition = after ? self.caseIDMap.get(after) : undefined
const afterPosition = _afterPosition != null ? _afterPosition + 1 : undefined
const insertPosition = beforePosition ?? afterPosition ?? self.cases.length

// insert/append cases and empty values
const ids: string[] = []
const _cases = cases.map(({ __id__ = uniqueCaseId() }) => {
ids.push(__id__)
return { __id__ }
})
const _values = new Array(cases.length)
if (insertPosition < self.cases.length) {
self.cases.splice(insertPosition, 0, ..._cases)
// update the indices of cases after the insert
self.caseIDMap.forEach((caseIndex, caseId) => {
if (caseIndex >= insertPosition) {
self.caseIDMap.set(caseId, caseIndex + cases.length)
}
})
// insert values for each attribute
self.attributesMap.forEach(attr => {
attr.addValues(_values, insertPosition)
})
}
else {
self.cases.push(..._cases)
// append values to each attribute
self.attributesMap.forEach(attr => {
attr.setLength(self.cases.length)
})
}
// update the indices for the appended cases
ids.forEach((caseId, index) => {
self.caseIDMap.set(caseId, insertPosition + index)
})

// copy any values provided
cases.forEach((aCase, index) => {
// shouldn't ever have to assign an id here since the middleware should do so
const { __id__ = uniqueCaseId() } = aCase
const insertPosition = after ? afterIndexForInsert(index, after) : beforeIndexForInsert(index, before)
self.attributes.forEach((attr: IAttribute) => {
const value = aCase[attr.id]
attr.addValue(value != null ? value : undefined, insertPosition)
Object.keys(aCase).forEach(key => {
const attr = self.getAttribute(key)
const value = aCase[key]
if (attr && value != null) {
attr.setValue(insertPosition + index, value)
}
})
insertCaseIDAtIndex(__id__, insertPosition ?? 0)
ids.push(__id__)
})

// invalidate collectionGroups (including childCases)
self.invalidateCollectionGroups()
return ids
Expand Down

0 comments on commit ba23fb5

Please sign in to comment.