Skip to content

Commit 93b374f

Browse files
kriszypkriszyp
committed
Further fixes/updates for parsing in RequestTarget
1 parent 825753f commit 93b374f

File tree

4 files changed

+75
-87
lines changed

4 files changed

+75
-87
lines changed

‎resources/RequestTarget.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,12 @@ export class RequestTarget extends URLSearchParams {
6767
super(search);
6868
this.search = search;
6969
parseQuery(search, this);
70+
if (!path) {
71+
// if there is a query string, but no path, treat as a collection anyway
72+
this.isCollection = true;
73+
this.id = null;
74+
return;
75+
}
7076
} else {
7177
super();
7278
path = target;
@@ -76,19 +82,12 @@ export class RequestTarget extends URLSearchParams {
7682
if (path) {
7783
// parse for properties and set the id
7884
if (path.startsWith('/')) path = path.substring(1);
79-
const dotIndex = path.indexOf('.');
80-
if (dotIndex > -1) {
81-
// handle paths of the form /path/id.property
82-
this.property = decodeURIComponent(path.slice(dotIndex + 1));
83-
path = path.substring(0, dotIndex);
84-
}
8585
} else {
8686
return; // leave this.id undefined
8787
}
8888
if (path) {
8989
if (path.endsWith('/')) {
9090
this.isCollection = true;
91-
path = path.substring(0, -1);
9291
}
9392
this.id = decodeURIComponent(path);
9493
} else {

‎resources/Resource.ts‎

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -334,18 +334,13 @@ export class Resource<Record extends object = any> implements ResourceInterface<
334334
else {
335335
return {
336336
query: { property },
337-
id: pathToId(path, this),
337+
id: path,
338338
isCollection: idWasCollection,
339339
};
340340
}
341341
}
342342
}
343-
// convert paths to arrays like /nested/path/4 -> ['nested', 'path', 4] if splitSegments is enabled
344-
const id = pathToId(path, this);
345-
if (idWasCollection) {
346-
return { id, isCollection: true };
347-
}
348-
return id;
343+
return path;
349344
}
350345
/**
351346
* Gets an instance of a resource by id
@@ -594,39 +589,31 @@ function transactional(action, options) {
594589
if (typeof idOrQuery === 'object' && idOrQuery) {
595590
// it is a query
596591
query = idOrQuery;
597-
id = idOrQuery instanceof URLSearchParams ? idOrQuery.toString() : idOrQuery.url; // get the request target (check .url for back-compat), and try to parse
598-
if (idOrQuery.conditions) {
599-
// it is already parsed, nothing more to do other than coerce and assign the id
592+
if (idOrQuery instanceof URLSearchParams) {
593+
// already RequestTarget (or URLSearchParams), consider it already parsed,
594+
// nothing more to do other than coerce and assign the id
600595
id = idOrQuery.id;
601596
if (typeof id === 'string') {
602-
id = idOrQuery.id = this.coerceId(id);
603-
}
604-
} else if (typeof id === 'string') {
605-
if (this.directURLMapping) {
606-
id = id.slice(1); // remove the leading slash
607-
query.id = id;
608-
} else {
609-
// handle queries in local URLs like /path/?name=value
610-
const searchIndex = id.indexOf('?');
611-
if (searchIndex > -1) {
612-
if (!(idOrQuery instanceof URLSearchParams))
613-
query = this.parseQuery(id.slice(searchIndex + 1), idOrQuery);
614-
id = id.slice(0, searchIndex);
615-
if (id === '') isCollection = true;
616-
}
617-
// handle paths of the form /path/id.property
618-
const parsedId = this.parsePath(id, context, query);
619-
if (parsedId?.id !== undefined) {
620-
if (parsedId.query) {
621-
if (query) query = Object.assign(parsedId.query, query);
622-
else query = parsedId.query;
623-
}
624-
isCollection = parsedId.isCollection;
625-
id = parsedId.id;
597+
if (this.directURLMapping) {
598+
id = idOrQuery.toString().slice(1); // remove the leading slash
599+
query.id = id;
626600
} else {
627-
id = parsedId;
601+
// handle paths of the form /path/id.property
602+
const parsedId = this.parsePath(id, context, query);
603+
if (parsedId?.id !== undefined) {
604+
if (parsedId.query) {
605+
if (query) query = Object.assign(parsedId.query, query);
606+
else query = parsedId.query;
607+
}
608+
isCollection = parsedId.isCollection;
609+
id = parsedId.id;
610+
} else {
611+
id = parsedId;
612+
}
613+
if (id) {
614+
query.id = id = this.coerceId(id);
615+
}
628616
}
629-
if (id) query.id = id;
630617
}
631618
} else if (idOrQuery[Symbol.iterator]) {
632619
// get the id part from an iterable query
@@ -672,7 +659,7 @@ function transactional(action, options) {
672659
query = new RequestTarget();
673660
query.id = id;
674661
}
675-
if (isCollection) query.isCollection = true;
662+
isCollection = query.isCollection;
676663
let resourceOptions;
677664
if (!context) {
678665
// try to get the context from the async context if possible

‎server/DurableSubscriptionsSession.ts‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ class SubscriptionsSession {
149149
this.user = user;
150150
}
151151
async addSubscription(subscriptionRequest, needsAck, filter?) {
152-
const { topic, rh: retainHandling, startTime: startTime } = subscriptionRequest;
152+
const { topic, rh: retainHandling, startTime } = subscriptionRequest;
153153
const searchIndex = topic.indexOf('?');
154154
let search, path;
155155
if (searchIndex > -1) {
@@ -168,7 +168,7 @@ class SubscriptionsSession {
168168
} else {
169169
omitCurrent = retainHandling === 2;
170170
}
171-
const request = {
171+
let request = {
172172
search,
173173
async: true,
174174
user: this.user,
@@ -186,20 +186,22 @@ class SubscriptionsSession {
186186
notFoundError.statusCode = 404;
187187
throw notFoundError;
188188
}
189-
request.url = entry.relativeURL;
189+
let url = entry.relativeURL;
190+
let isCollection;
191+
let onlyChildren;
190192
let hashIndex: number;
191-
if (request.url.indexOf('+') > -1 || (hashIndex = request.url.indexOf('#')) > -1) {
192-
const path = request.url.slice(1); // remove leading slash
193+
if (url.indexOf('+') > -1 || (hashIndex = url.indexOf('#')) > -1) {
194+
const path = url.slice(1); // remove leading slash
193195
hashIndex--; // adjust accordingly
194196
if (hashIndex > -1 && hashIndex !== path.length - 1)
195197
throw new Error('Multi-level wildcards can only be used at the end of a topic');
196198
// treat as a collection to get all children, but we will need to filter out any that are not direct children or matching the pattern
197-
request.isCollection = true; // used by Resource to determine if the resource should be treated as a collection
199+
isCollection = true; // used by Resource to determine if the resource should be treated as a collection
198200
if (path.indexOf('+') === path.length - 1) {
199201
// if it is only a trailing single-level wildcard, we can treat it as a shallow wildcard
200202
// and use the optimized onlyChildren option, which will be faster, and does not require any filtering
201-
request.onlyChildren = true;
202-
request.url = '/' + path.slice(0, path.length - 1);
203+
onlyChildren = true;
204+
url = '/' + path.slice(0, path.length - 1);
203205
} else {
204206
// otherwise we have a potentially complex wildcard, so we will need to filter out any that are not direct children or matching the pattern
205207
const matchingPath = path.split('/');
@@ -237,11 +239,17 @@ class SubscriptionsSession {
237239
};
238240
}
239241
const firstWildcard = matchingPath.indexOf('+');
240-
request.url =
241-
'/' + (firstWildcard > -1 ? matchingPath.slice(0, firstWildcard) : matchingPath).concat('').join('/');
242+
url = '/' + (firstWildcard > -1 ? matchingPath.slice(0, firstWildcard) : matchingPath).concat('').join('/');
242243
}
243-
} else request.isCollection = false; // must explicitly turn this off so topics that end in a slash are not treated as collections
244-
244+
} else isCollection = false; // must explicitly turn this off so topics that end in a slash are not treated as collections
245+
request = new RequestTarget(url);
246+
Object.assign(request, {
247+
isCollection,
248+
onlyChildren,
249+
startTime,
250+
omitCurrent,
251+
checkPermission: this.user?.role?.permission ?? {},
252+
});
245253
const resourcePath = entry.path;
246254
const resource = entry.Resource;
247255
const subscription = await transaction(request, async () => {
@@ -479,7 +487,7 @@ export class DurableSubscriptionsSession extends SubscriptionsSession {
479487

480488
async addSubscription(subscription, needsAck) {
481489
await this.resumeSubscription(subscription, needsAck);
482-
const { qos, startTime: startTime } = subscription;
490+
const { qos, startTime } = subscription;
483491
if (qos > 0 && !startTime) this.saveSubscriptions();
484492
return subscription.qos;
485493
}

‎unitTests/resources/query.test.js‎

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const { getMockLMDBPath } = require('../test_utils');
44
const { parseQuery } = require('#src/resources/search');
55
const { table } = require('#src/resources/databases');
66
const { transaction } = require('#src/resources/transaction');
7+
const { RequestTarget } = require('#src/resources/RequestTarget');
78
const { setMainIsWorker } = require('#js/server/threads/manageThreads');
89
let x = 532532;
910
function random(max) {
@@ -1368,7 +1369,7 @@ describe('Querying through Resource API', () => {
13681369
});
13691370
it('Parsed query data in a table and select with special properties and star', async function () {
13701371
let results = [];
1371-
for await (let record of QueryTable.search({ url: '?id=ge=id-90&select($id,$updatedtime,*)' })) {
1372+
for await (let record of QueryTable.search(new RequestTarget('?id=ge=id-90&select($id,$updatedtime,*)'))) {
13721373
results.push(record);
13731374
}
13741375

@@ -1383,7 +1384,9 @@ describe('Querying through Resource API', () => {
13831384

13841385
it('Parsed nested query data in a table', async function () {
13851386
let results = [];
1386-
for await (let record of QueryTable.search({ url: '?(id=ge=id-90&id=le=id-93)|(name=name-95&id=ge=id-94)' })) {
1387+
for await (let record of QueryTable.search(
1388+
new RequestTarget('?(id=ge=id-90&id=le=id-93)|(name=name-95&id=ge=id-94)')
1389+
)) {
13871390
results.push(record);
13881391
}
13891392
assert.equal(results.length, 5);
@@ -1392,7 +1395,9 @@ describe('Querying through Resource API', () => {
13921395
});
13931396
it('Parsed nested query data in a table with brackets', async function () {
13941397
let results = [];
1395-
for await (let record of QueryTable.search({ url: '?[id=ge=id-90&id=le=id-93]|[name=name-95&id=ge=id-94]' })) {
1398+
for await (let record of QueryTable.search(
1399+
new RequestTarget('?[id=ge=id-90&id=le=id-93]|[name=name-95&id=ge=id-94]')
1400+
)) {
13961401
results.push(record);
13971402
}
13981403
assert.equal(results.length, 5);
@@ -1401,9 +1406,9 @@ describe('Querying through Resource API', () => {
14011406
});
14021407
it('Parsed nested query data in a table with joined sort', async function () {
14031408
let results = [];
1404-
for await (let record of QueryTable.search({
1405-
url: '?(id>id-80&id<=id-93)|(name=name-95&id>id-94)&sort(-related.name)',
1406-
})) {
1409+
for await (let record of QueryTable.search(
1410+
new RequestTarget('?(id>id-80&id<=id-93)|(name=name-95&id>id-94)&sort(-related.name)')
1411+
)) {
14071412
results.push(record);
14081413
}
14091414
assert.equal(results.length, 15);
@@ -1413,9 +1418,9 @@ describe('Querying through Resource API', () => {
14131418
});
14141419
it('Parsed query data in a table with one-to-many joined sort that is not primary', async function () {
14151420
let results = [];
1416-
for await (let record of RelatedTable.search({
1417-
url: '?name=related name 3&sort(-relatedToMany.name)&select(id,relatedToMany)',
1418-
})) {
1421+
for await (let record of RelatedTable.search(
1422+
new RequestTarget('?name=related name 3&sort(-relatedToMany.name)&select(id,relatedToMany)')
1423+
)) {
14191424
results.push(record);
14201425
}
14211426
assert.equal(results.length, 1);
@@ -1426,9 +1431,9 @@ describe('Querying through Resource API', () => {
14261431
});
14271432
it('Parsed query data in a table with many-to-many joined sort that is not primary', async function () {
14281433
let results = [];
1429-
for await (let record of QueryTable.search({
1430-
url: '?id=id-14&sort(-manyToMany.name)&select(id,manyToMany)',
1431-
})) {
1434+
for await (let record of QueryTable.search(
1435+
new RequestTarget('?id=id-14&sort(-manyToMany.name)&select(id,manyToMany)')
1436+
)) {
14321437
results.push(record);
14331438
}
14341439
assert.equal(results.length, 1);
@@ -1439,9 +1444,9 @@ describe('Querying through Resource API', () => {
14391444
});
14401445
it('Parsed query data in a table with many-to-many joined sort that has missing entries and multiple sorts', async function () {
14411446
let results = [];
1442-
for await (let record of QueryTable.search({
1443-
url: '?id=id-12|id=id-24&sort(-id,-manyToMany.name)&select(id,manyToMany,manyToManyIds)',
1444-
})) {
1447+
for await (let record of QueryTable.search(
1448+
new RequestTarget('?id=id-12|id=id-24&sort(-id,-manyToMany.name)&select(id,manyToMany,manyToManyIds)')
1449+
)) {
14451450
results.push(record);
14461451
}
14471452
assert.equal(results.length, 2);
@@ -1455,9 +1460,7 @@ describe('Querying through Resource API', () => {
14551460

14561461
it('Parsed query data in a table with not equal to null', async function () {
14571462
let results = [];
1458-
for await (let record of QueryTable.search({
1459-
url: '?sparse!=null',
1460-
})) {
1463+
for await (let record of QueryTable.search(new RequestTarget('?sparse!=null'))) {
14611464
results.push(record);
14621465
}
14631466
assert.equal(results.length, 17);
@@ -1467,9 +1470,7 @@ describe('Querying through Resource API', () => {
14671470
});
14681471
it('Parsed query with boolean equal to true', async function () {
14691472
let results = [];
1470-
for await (let record of RelatedTable.search({
1471-
url: '?aFlag==true',
1472-
})) {
1473+
for await (let record of RelatedTable.search(new RequestTarget('?aFlag==true'))) {
14731474
results.push(record);
14741475
}
14751476
assert.equal(results.length, 2);
@@ -1479,9 +1480,7 @@ describe('Querying through Resource API', () => {
14791480
});
14801481
it('Parsed query with boolean equal to false', async function () {
14811482
let results = [];
1482-
for await (let record of RelatedTable.search({
1483-
url: '?aFlag==false',
1484-
})) {
1483+
for await (let record of RelatedTable.search(new RequestTarget('?aFlag==false'))) {
14851484
results.push(record);
14861485
}
14871486
assert.equal(results.length, 3);
@@ -1494,12 +1493,7 @@ describe('Querying through Resource API', () => {
14941493
let context = {};
14951494
await RelatedTable.get(1, context); // This will set the lastModified on the context
14961495
assert(isFinite(context.lastModified));
1497-
for await (let record of RelatedTable.search(
1498-
{
1499-
url: '?aFlag==true',
1500-
},
1501-
context
1502-
)) {
1496+
for await (let record of RelatedTable.search(new RequestTarget('?aFlag==true'), context)) {
15031497
results.push(record);
15041498
}
15051499
assert(!isFinite(context.lastModified));

0 commit comments

Comments
 (0)