Skip to content

Commit 69f4501

Browse files
fix: handle long text values performantly (#1874)
* test: add test for long text performance This test fails in the current state, as jsondiffpatch uses the diff-match-patch library for long text diffing. This text diff is very slow, but produces a nice delta that can be used to see the difference between two text strings. However, we don't actually care about identifying the specific differences between two strings - we just care if they changed. The regular text diffing strategy is MUCH faster, and performance is what we want to prioritize. - https://github.com/benjamine/jsondiffpatch/blob/master/docs/deltas.md#text-diffs - https://github.com/google/diff-match-patch * fix: disable diff-match-patch for long string diff jsondiffpatch uses diff-match-patch for long text diffing. This text diff is very slow, but produces a nice delta that can be used to see the difference between two text strings. However, we don't actually care about identifying the specific differences between two strings - we just care if they changed. The regular text diffing strategy is MUCH faster, and performance is what we want to prioritize. - https://github.com/benjamine/jsondiffpatch/blob/master/docs/deltas.md#text-diffs - https://github.com/google/diff-match-patch * docs: add changeset --------- Co-authored-by: Markus Azer <markusgihady@gmail.com>
1 parent ea17aa2 commit 69f4501

3 files changed

Lines changed: 85 additions & 5 deletions

File tree

.changeset/violet-timers-act.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@commercetools/sync-actions': minor
3+
---
4+
5+
Handle long text values performantly

packages/sync-actions/src/utils/diffpatcher.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@ const diffpatcher = new DiffPatcher({
2020
includeValueOnMove: false,
2121
},
2222
textDiff: {
23-
// If the value to diff has a bigger length,
24-
// a text diffing algorithm is used
25-
// See https://github.com/benjamine/jsondiffpatch/
26-
// blob/master/docs/deltas.md#text-diffs
27-
minLength: 300,
23+
/**
24+
* jsondiffpatch uses a very fine-grained diffing algorithm for long strings to easily identify
25+
* what changed between strings. However, we don't actually care about what changed, just
26+
* if the string changed at all. So we set the minimum length to diff to a very large number to avoid
27+
* using the very slow algorithm.
28+
* See https://github.com/benjamine/jsondiffpatch/blob/master/docs/deltas.md#text-diffs.
29+
*/
30+
minLength: Number.MAX_SAFE_INTEGER,
2831
},
2932
})
3033

packages/sync-actions/test/product-sync-variants.spec.js

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,78 @@ describe('Actions', () => {
116116
])
117117
})
118118

119+
test('should handle long text values performantly', () => {
120+
const longText = 'a'.repeat(10_000)
121+
const longText2 = 'b'.repeat(10_000)
122+
const before = {
123+
id: '123',
124+
masterVariant: {
125+
id: 1,
126+
attributes: [
127+
{ name: 'color', value: longText },
128+
{ name: 'size', value: longText },
129+
{ name: 'weight', value: longText },
130+
],
131+
},
132+
variants: [
133+
{
134+
id: 2,
135+
attributes: [
136+
{ name: 'color', value: longText },
137+
{ name: 'size', value: longText },
138+
{ name: 'weight', value: longText },
139+
],
140+
},
141+
],
142+
}
143+
144+
const now = {
145+
id: '123',
146+
masterVariant: {
147+
id: 1,
148+
attributes: [
149+
{ name: 'color', value: longText2 },
150+
{ name: 'size', value: longText2 },
151+
{ name: 'weight', value: longText2 },
152+
],
153+
},
154+
variants: [
155+
{
156+
id: 2,
157+
attributes: [
158+
{ name: 'color', value: longText2 },
159+
{ name: 'size', value: longText2 },
160+
{ name: 'weight', value: longText2 },
161+
],
162+
},
163+
],
164+
}
165+
166+
const startTime = Date.now()
167+
const actions = productsSync.buildActions(now, before)
168+
169+
// Should take less than 100ms.
170+
expect(Date.now() - startTime).toBeLessThan(100)
171+
expect(actions).toEqual([
172+
{ action: 'setAttribute', variantId: 1, name: 'color', value: longText2 },
173+
{ action: 'setAttribute', variantId: 1, name: 'size', value: longText2 },
174+
{
175+
action: 'setAttribute',
176+
variantId: 1,
177+
name: 'weight',
178+
value: longText2,
179+
},
180+
{ action: 'setAttribute', variantId: 2, name: 'color', value: longText2 },
181+
{ action: 'setAttribute', variantId: 2, name: 'size', value: longText2 },
182+
{
183+
action: 'setAttribute',
184+
variantId: 2,
185+
name: 'weight',
186+
value: longText2,
187+
},
188+
])
189+
})
190+
119191
test('should build SameForAll attribute actions', () => {
120192
const before = {
121193
id: '123',

0 commit comments

Comments
 (0)