Skip to content
This repository was archived by the owner on Apr 17, 2020. It is now read-only.

Commit 86954b6

Browse files
committed
fix: move reference-check error to preparation phase
previously this error would be thrown synchronously, as the join options are configured: ```js QueryError('Model: there are no references to `OtherModel`'); ```
1 parent ba4a0e0 commit 86954b6

File tree

2 files changed

+84
-89
lines changed

2 files changed

+84
-89
lines changed

lib/KnormRelations.js

Lines changed: 56 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ class KnormRelations {
117117
super(model);
118118
// TODO: only initialize parsedRows when needed
119119
this.parsedRows = new Map();
120+
// TODO: move this to base model default options
120121
this.options.ensureUniqueField = true;
121122
this.config.references = model.config.references;
122123
this.config.referenceFunctions = model.config.referenceFunctions;
@@ -127,56 +128,14 @@ class KnormRelations {
127128
joins = [joins];
128129
}
129130

131+
// TODO: use appendOption
130132
this.options.joins = this.options.joins || [];
131133

132134
joins.forEach(join => {
133135
if (join.prototype instanceof Model) {
134136
join = join.query;
135137
}
136138

137-
const joinReferences = Object.assign({}, join.config.references);
138-
const thisReferences = Object.assign({}, this.config.references);
139-
140-
Object.entries(join.config.referenceFunctions).forEach(
141-
([fieldName, referenceFunction]) => {
142-
const field = join.config.fields[fieldName];
143-
addReferenceByFunction(joinReferences, referenceFunction, field);
144-
}
145-
);
146-
147-
Object.entries(this.config.referenceFunctions).forEach(
148-
([fieldName, referenceFunction]) => {
149-
const field = this.config.fields[fieldName];
150-
addReferenceByFunction(thisReferences, referenceFunction, field);
151-
}
152-
);
153-
154-
if (
155-
!thisReferences[join.model.name] &&
156-
!joinReferences[this.model.name]
157-
) {
158-
throw new Query.QueryError(
159-
`${this.model.name}: there are no references to \`${
160-
join.model.name
161-
}\``
162-
);
163-
}
164-
165-
const isReverseJoin = !!thisReferences[join.model.name];
166-
const mergedReferences = Object.assign(
167-
{},
168-
thisReferences[join.model.name],
169-
joinReferences[this.model.name]
170-
);
171-
const mergedReferencesReversed = mapReferencesByReferencedField(
172-
mergedReferences,
173-
isReverseJoin ? join.model : this.model
174-
);
175-
176-
join.config.isReverseJoin = isReverseJoin;
177-
join.config.mergedReferences = mergedReferences;
178-
join.config.mergedReferencesReversed = mergedReferencesReversed;
179-
180139
join.options.joinType = type;
181140
join.setOptions(options);
182141

@@ -206,18 +165,57 @@ class KnormRelations {
206165
}
207166

208167
prepareOn() {
168+
const join = this;
169+
const parent = this.parent;
170+
const joinReferences = Object.assign({}, join.config.references);
171+
const parentReferences = Object.assign({}, parent.config.references);
172+
173+
Object.entries(join.config.referenceFunctions).forEach(
174+
([fieldName, referenceFunction]) => {
175+
const field = join.config.fields[fieldName];
176+
addReferenceByFunction(joinReferences, referenceFunction, field);
177+
}
178+
);
179+
180+
Object.entries(parent.config.referenceFunctions).forEach(
181+
([fieldName, referenceFunction]) => {
182+
const field = parent.config.fields[fieldName];
183+
addReferenceByFunction(parentReferences, referenceFunction, field);
184+
}
185+
);
186+
187+
if (
188+
!parentReferences[join.model.name] &&
189+
!joinReferences[parent.model.name]
190+
) {
191+
throw new Query.QueryError(
192+
`${parent.model.name}: there are no references to \`${
193+
join.model.name
194+
}\``
195+
);
196+
}
197+
198+
const isReverseJoin = !!parentReferences[join.model.name];
199+
const toModel = isReverseJoin ? join.model : parent.model;
200+
const mergedReferences = Object.assign(
201+
{},
202+
parentReferences[join.model.name],
203+
joinReferences[parent.model.name]
204+
);
205+
const mergedReferencesReversed = mapReferencesByReferencedField(
206+
mergedReferences,
207+
toModel
208+
);
209209
let references = [];
210210

211211
if (this.options.on) {
212212
this.options.on.forEach(field => {
213213
if (field instanceof Field) {
214-
if (field.model === this.parent.model) {
215-
if (this.config.mergedReferences[field.name]) {
216-
references.push(this.config.mergedReferences[field.name]);
214+
if (field.model === parent.model) {
215+
if (mergedReferences[field.name]) {
216+
references.push(mergedReferences[field.name]);
217217
} else {
218-
references.push(
219-
...this.config.mergedReferencesReversed[field.name]
220-
);
218+
references.push(...mergedReferencesReversed[field.name]);
221219
}
222220
return;
223221
}
@@ -226,37 +224,34 @@ class KnormRelations {
226224
field = field.name;
227225
}
228226

229-
if (this.config.mergedReferencesReversed[field]) {
230-
references.push(...this.config.mergedReferencesReversed[field]);
227+
if (mergedReferencesReversed[field]) {
228+
references.push(...mergedReferencesReversed[field]);
231229
} else {
232-
references.push(this.config.mergedReferences[field]);
230+
references.push(mergedReferences[field]);
233231
}
234232
});
235233
} else {
236-
references = Object.values(this.config.mergedReferences);
234+
references = Object.values(mergedReferences);
237235
}
238236

239-
const isReverseJoin = this.config.isReverseJoin;
240-
241237
return references.reduce((columns, field) => {
242238
const fromColumn = field.column;
243239
const references = isArray(field.references)
244240
? field.references
245241
: [field.references];
246242

247243
references.forEach(reference => {
248-
const toModel = isReverseJoin ? this.model : this.parent.model;
249244
if (reference.model.name === toModel.name) {
250245
const toColumn = reference.column;
251246
let from;
252247
let to;
253248

254-
if (this.config.isReverseJoin) {
255-
from = this.formatColumn(toColumn);
256-
to = this.parent.formatColumn(fromColumn);
249+
if (isReverseJoin) {
250+
from = join.formatColumn(toColumn);
251+
to = parent.formatColumn(fromColumn);
257252
} else {
258-
from = this.formatColumn(fromColumn);
259-
to = this.parent.formatColumn(toColumn);
253+
from = join.formatColumn(fromColumn);
254+
to = parent.formatColumn(toColumn);
260255
}
261256

262257
columns[from] = to;

test/KnormRelations.spec.js

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -450,17 +450,17 @@ describe('KnormRelations', () => {
450450
});
451451

452452
describe("with a 'leftJoin' configured", () => {
453-
it('throws if the models do not reference each other', () => {
453+
it('rejects if the models do not reference each other', async () => {
454454
class Foo extends Model {}
455455
Foo.table = 'foo';
456-
expect(
457-
() => new Query(User).leftJoin(new Query(Foo)),
458-
'to throw',
456+
await expect(
457+
new Query(User).leftJoin(new Query(Foo)).fetch(),
458+
'to be rejected with error satisfying',
459459
new Query.QueryError('User: there are no references to `Foo`')
460460
);
461-
expect(
462-
() => new Query(Foo).leftJoin(new Query(User)),
463-
'to throw',
461+
await expect(
462+
new Query(Foo).leftJoin(new Query(User)).fetch(),
463+
'to be rejected with error satisfying',
464464
new Query.QueryError('Foo: there are no references to `User`')
465465
);
466466
});
@@ -1494,7 +1494,7 @@ describe('KnormRelations', () => {
14941494
};
14951495
});
14961496

1497-
it('throws if models used in a join do not reference each other', () => {
1497+
it('rejects if models used in a join do not reference each other', async () => {
14981498
class Foo extends Model {}
14991499
Foo.table = 'foo';
15001500
Foo.fields = {
@@ -1503,14 +1503,14 @@ describe('KnormRelations', () => {
15031503
references: [Message.fields.id, OtherMessage.fields.id]
15041504
}
15051505
};
1506-
expect(
1507-
() => new Query(User).leftJoin(new Query(Foo)),
1508-
'to throw',
1506+
await expect(
1507+
new Query(User).leftJoin(new Query(Foo)).fetch(),
1508+
'to be rejected with error satisfying',
15091509
new Query.QueryError('User: there are no references to `Foo`')
15101510
);
1511-
expect(
1512-
() => new Query(Foo).leftJoin(new Query(User)),
1513-
'to throw',
1511+
await expect(
1512+
new Query(Foo).leftJoin(new Query(User)).fetch(),
1513+
'to be rejected with error satisfying',
15141514
new Query.QueryError('Foo: there are no references to `User`')
15151515
);
15161516
});
@@ -1823,7 +1823,7 @@ describe('KnormRelations', () => {
18231823
};
18241824
});
18251825

1826-
it('throws if models used in a join do not reference each other', () => {
1826+
it('rejects if models used in a join do not reference each other', async () => {
18271827
class Foo extends Model {}
18281828
Foo.table = 'foo';
18291829
Foo.fields = {
@@ -1834,14 +1834,14 @@ describe('KnormRelations', () => {
18341834
}
18351835
}
18361836
};
1837-
expect(
1838-
() => new Query(User).leftJoin(new Query(Foo)),
1839-
'to throw',
1837+
await expect(
1838+
new Query(User).leftJoin(new Query(Foo)).fetch(),
1839+
'to be rejected with error satisfying',
18401840
new Query.QueryError('User: there are no references to `Foo`')
18411841
);
1842-
expect(
1843-
() => new Query(Foo).leftJoin(new Query(User)),
1844-
'to throw',
1842+
await expect(
1843+
new Query(Foo).leftJoin(new Query(User)).fetch(),
1844+
'to be rejected with error satisfying',
18451845
new Query.QueryError('Foo: there are no references to `User`')
18461846
);
18471847
});
@@ -2059,7 +2059,7 @@ describe('KnormRelations', () => {
20592059
};
20602060
});
20612061

2062-
it('throws if models used in a join do not reference each other', () => {
2062+
it('rejects if models used in a join do not reference each other', async () => {
20632063
class Foo extends Model {}
20642064
Foo.table = 'foo';
20652065
Foo.fields = {
@@ -2070,14 +2070,14 @@ describe('KnormRelations', () => {
20702070
}
20712071
}
20722072
};
2073-
expect(
2074-
() => new Query(User).leftJoin(new Query(Foo)),
2075-
'to throw',
2073+
await expect(
2074+
new Query(User).leftJoin(new Query(Foo)).fetch(),
2075+
'to be rejected with error satisfying',
20762076
new Query.QueryError('User: there are no references to `Foo`')
20772077
);
2078-
expect(
2079-
() => new Query(Foo).leftJoin(new Query(User)),
2080-
'to throw',
2078+
await expect(
2079+
new Query(Foo).leftJoin(new Query(User)).fetch(),
2080+
'to be rejected with error satisfying',
20812081
new Query.QueryError('Foo: there are no references to `User`')
20822082
);
20832083
});

0 commit comments

Comments
 (0)