Skip to content

Commit 7159799

Browse files
marshallswainmarshallswain
committed
fix: correct create handling and find limit calculation
- Fix create() to preserve query param while ignoring other query params - Fix find() to respect Elasticsearch max_result_window constraint (from + size <= 10000) - Remove delay hook from tests (no longer needed with proper refresh handling) - All 216 adapter tests now passing
1 parent c65781d commit 7159799

File tree

3 files changed

+107
-109
lines changed

3 files changed

+107
-109
lines changed

‎src/methods/create.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,18 @@ export function create(
5454
const createParams = getCreateParams(service, docDescriptor, params)
5555
const getParams = prepareGetParams(params, 'upsert')
5656

57-
// If we have routing (parent document), pass it in the query for the get operation
57+
// Create should ignore query parameters except $select (which controls returned fields)
58+
const originalSelect = getParams.query?.$select
59+
delete getParams.query
60+
61+
// Restore $select if it was present
62+
if (originalSelect !== undefined) {
63+
getParams.query = { $select: originalSelect }
64+
}
65+
66+
// If we have routing (parent document), add it to the query
5867
if (routing !== undefined) {
59-
getParams.query = Object.assign({}, getParams.query, { [service.parent as string]: routing })
68+
getParams.query = { ...getParams.query, [service.parent as string]: routing }
6069
}
6170
// Elasticsearch `create` expects _id, whereas index does not.
6271
// Our `create` supports both forms.

‎src/methods/find.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,12 @@ export function find(service: ElasticAdapterInterface, params: ElasticsearchServ
4444
// Without this, Elasticsearch defaults to only 10 results
4545
// Note: For >10k results, users must either:
4646
// 1. Set explicit query.$limit, 2. Configure higher index.max_result_window, or 3. Use scroll API
47+
// Important: from + size must not exceed max_result_window (10000)
48+
const skip = (filters.$skip as number) || 0
49+
const maxWindow = 10000
4750
const limit = filters.$limit !== undefined
4851
? (filters.$limit as number)
49-
: (paginate === false ? 10000 : undefined)
52+
: (paginate === false ? Math.max(0, maxWindow - skip) : undefined)
5053

5154
const findParams: SearchRequest = {
5255
index: (filters.$index as string) ?? service.index,

‎test/index.ts‎

Lines changed: 92 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,6 @@ describe('Elasticsearch Service', () => {
4545
},
4646
})
4747
)
48-
49-
// Add global hook to delay after write operations for Elasticsearch consistency
50-
app.hooks({
51-
after: {
52-
all: async (context) => {
53-
if (['create', 'update', 'patch', 'remove'].includes(context.method)) {
54-
await new Promise<void>((resolve) => {
55-
setTimeout(resolve, 20)
56-
})
57-
}
58-
return context
59-
}
60-
}
61-
})
6248
})
6349

6450
after(async function () {
@@ -91,7 +77,7 @@ describe('Elasticsearch Service', () => {
9177
peopleService.options.multi = true
9278
try {
9379
await peopleService.remove(null, { query: { $limit: 1000 }, refresh: 'wait_for' })
94-
} catch (_error) {
80+
} catch {
9581
// Ignore errors if no data exists
9682
}
9783
peopleService.options.multi = originalMulti
@@ -100,93 +86,93 @@ describe('Elasticsearch Service', () => {
10086
})
10187

10288
adapterTests([
103-
// '.id',
104-
// '.options',
105-
// '.events',
106-
// '._get',
107-
// '._find',
108-
// '._create',
109-
// '._update',
110-
// '._patch',
111-
// '._remove',
112-
// '.$get',
113-
// '.$find',
114-
// '.$create',
115-
// '.$update',
116-
// '.$patch',
117-
// '.$remove',
118-
// '.get',
119-
// '.get + $select',
120-
// '.get + id + query',
121-
// '.get + NotFound',
122-
// '.get + NotFound (integer)',
123-
// '.get + id + query id',
124-
// '.find',
125-
// '.remove',
126-
// '.remove + $select',
127-
// '.remove + id + query',
128-
// '.remove + multi',
129-
// '.remove + NotFound',
130-
// '.remove + NotFound (integer)',
131-
// '.remove + id + query id',
132-
// '.update',
133-
// '.update + $select',
134-
// '.update + id + query',
135-
// '.update + NotFound',
136-
// '.update + NotFound (integer)',
137-
// '.update + query + NotFound',
138-
// '.update + id + query id',
139-
// '.patch',
140-
// '.patch + $select',
141-
// '.patch + id + query',
142-
// '.patch multi query changed',
143-
// '.patch + NotFound',
144-
// '.patch + NotFound (integer)',
145-
// '.patch + query + NotFound',
146-
// '.patch + id + query id',
147-
// '.create',
148-
// '.create + $select',
149-
// 'internal .find',
150-
// 'internal .get',
151-
// 'internal .create',
152-
// 'internal .update',
153-
// 'internal .patch',
154-
// 'internal .remove',
155-
// '.find + equal',
156-
// '.find + equal multiple',
157-
// '.find + $limit',
158-
// '.find + $limit 0',
159-
// '.find + $select',
160-
// '.find + $or',
161-
// '.find + $in',
162-
// '.find + $gt + $lt + $sort',
163-
// '.find + $or nested + $sort',
164-
// '.find + $and',
165-
// '.find + $and + $or',
166-
// 'params.adapter + multi',
167-
// '.find + paginate + query',
168-
// 'params.adapter + paginate',
89+
'.id',
90+
'.options',
91+
'.events',
92+
'._get',
93+
'._find',
94+
'._create',
95+
'._update',
96+
'._patch',
97+
'._remove',
98+
'.$get',
99+
'.$find',
100+
'.$create',
101+
'.$update',
102+
'.$patch',
103+
'.$remove',
104+
'.get',
105+
'.get + $select',
106+
'.get + id + query',
107+
'.get + NotFound',
108+
'.get + NotFound (integer)',
109+
'.get + id + query id',
110+
'.find',
111+
'.remove',
112+
'.remove + $select',
113+
'.remove + id + query',
114+
'.remove + multi',
115+
'.remove + NotFound',
116+
'.remove + NotFound (integer)',
117+
'.remove + id + query id',
118+
'.update',
119+
'.update + $select',
120+
'.update + id + query',
121+
'.update + NotFound',
122+
'.update + NotFound (integer)',
123+
'.update + query + NotFound',
124+
'.update + id + query id',
125+
'.patch',
126+
'.patch + $select',
127+
'.patch + id + query',
128+
'.patch multi query changed',
129+
'.patch + NotFound',
130+
'.patch + NotFound (integer)',
131+
'.patch + query + NotFound',
132+
'.patch + id + query id',
133+
'.create',
134+
'.create + $select',
135+
'internal .find',
136+
'internal .get',
137+
'internal .create',
138+
'internal .update',
139+
'internal .patch',
140+
'internal .remove',
141+
'.find + equal',
142+
'.find + equal multiple',
143+
'.find + $limit',
144+
'.find + $limit 0',
145+
'.find + $select',
146+
'.find + $or',
147+
'.find + $in',
148+
'.find + $gt + $lt + $sort',
149+
'.find + $or nested + $sort',
150+
'.find + $and',
151+
'.find + $and + $or',
152+
'params.adapter + multi',
153+
'.find + paginate + query',
154+
'params.adapter + paginate',
169155
//
170156
// Failing tests - moved to bottom due to Elasticsearch eventual consistency issues
171157
'.remove + multi no pagination',
172-
// '.patch multiple',
173-
// '.patch multiple no pagination',
174-
// '.patch multi query same',
175-
// '.create ignores query',
176-
// '.create multi',
177-
// '.find + $sort',
178-
// '.find + $sort + string',
179-
// '.find + $skip',
180-
// '.find + $nin',
181-
// '.find + $lt',
182-
// '.find + $lte',
183-
// '.find + $gt',
184-
// '.find + $gte',
185-
// '.find + $ne',
186-
// '.find + paginate',
187-
// '.find + paginate + $limit + $skip',
188-
// '.find + paginate + $limit 0',
189-
// '.find + paginate + params'
158+
'.patch multiple',
159+
'.patch multiple no pagination',
160+
'.patch multi query same',
161+
'.create ignores query',
162+
'.create multi',
163+
'.find + $sort',
164+
'.find + $sort + string',
165+
'.find + $skip',
166+
'.find + $nin',
167+
'.find + $lt',
168+
'.find + $lte',
169+
'.find + $gt',
170+
'.find + $gte',
171+
'.find + $ne',
172+
'.find + paginate',
173+
'.find + paginate + $limit + $skip',
174+
'.find + paginate + $limit 0',
175+
'.find + paginate + params'
190176
])(app, errors, 'people', 'id')
191177
})
192178

@@ -242,12 +228,12 @@ describe('Elasticsearch Service', () => {
242228
await app.service(serviceName).remove(null, { query: { $limit: 1000 } })
243229
})
244230

245-
// coreTests.find(app, serviceName, esVersion)
246-
// coreTests.get(app, serviceName)
247-
// coreTests.create(app, serviceName)
248-
// coreTests.patch(app, serviceName, esVersion)
249-
// coreTests.remove(app, serviceName)
250-
// coreTests.update(app, serviceName)
251-
// coreTests.raw(app, serviceName, esVersion)
231+
coreTests.find(app, serviceName, esVersion)
232+
coreTests.get(app, serviceName)
233+
coreTests.create(app, serviceName)
234+
coreTests.patch(app, serviceName, esVersion)
235+
coreTests.remove(app, serviceName)
236+
coreTests.update(app, serviceName)
237+
coreTests.raw(app, serviceName, esVersion)
252238
})
253239
})

0 commit comments

Comments
 (0)