also looking at this
fix(orm): an eager load that resolves nothing should say so
#1075
2 files
+168
-1
| @@ -1451,6 +1451,69 @@ interface ResolvedRelation { | ||
| 1451 | 1451 | pivotTimestamps?: boolean |
| 1452 | 1452 | } |
| 1453 | 1453 | |
| 1454 | /** | |
| 1455 | * Relation kinds `resolveRelation` can actually resolve, and the ones a model | |
| 1456 | * may declare. `ModelDefinition` accepts considerably more than the resolver | |
| 1457 | * implements, which is the more confusing half of the silent-miss problem | |
| 1458 | * below: a `morphMany` is accepted by the types, encouraged by autocomplete, | |
| 1459 | * and then does nothing at all. | |
| 1460 | */ | |
| 1461 | const RESOLVABLE_RELATION_KINDS = ['hasMany', 'hasOne', 'belongsTo', 'belongsToMany'] as const | |
| 1462 | const DECLARABLE_RELATION_KINDS = [ | |
| 1463 | ...RESOLVABLE_RELATION_KINDS, | |
| 1464 | 'hasOneThrough', | |
| 1465 | 'hasManyThrough', | |
| 1466 | 'morphOne', | |
| 1467 | 'morphMany', | |
| 1468 | 'morphTo', | |
| 1469 | 'morphToMany', | |
| 1470 | 'morphedByMany', | |
| 1471 | ] as const | |
| 1472 | ||
| 1473 | /** Every relation name declared under `kind`, in declaration order. */ | |
| 1474 | function relationNamesFor(definition: ModelDefinition, kind: string): string[] { | |
| 1475 | const rel = (definition as unknown as Record<string, unknown>)[kind] | |
| 1476 | if (!rel) | |
| 1477 | return [] | |
| 1478 | if (Array.isArray(rel)) | |
| 1479 | return rel.map(item => (typeof item === 'string' ? item : String((item as any)?.model ?? ''))).filter(Boolean) | |
| 1480 | if (typeof rel === 'object') | |
| 1481 | return Object.keys(rel as Record<string, unknown>) | |
| 1482 | if (typeof rel === 'string') | |
| 1483 | return [rel] | |
| 1484 | return [] | |
| 1485 | } | |
| 1486 | ||
| 1487 | /** | |
| 1488 | * Why an eager load found nothing, phrased for the person who typed the name. | |
| 1489 | * | |
| 1490 | * `eagerLoadRelation` used to `return` here. A misspelled relation loaded | |
| 1491 | * nothing and said nothing — indistinguishable from a relation that genuinely | |
| 1492 | * has no rows — and so did every relation kind the resolver does not implement. | |
| 1493 | * Both are mistakes worth surfacing, and they need different wording: one is a | |
| 1494 | * typo, the other is a gap in this library that no amount of squinting at the | |
| 1495 | * model will explain. | |
| 1496 | */ | |
| 1497 | function unresolvedRelationMessage(definition: ModelDefinition, relationName: string): string { | |
| 1498 | const model = definition.name ?? definition.table ?? 'model' | |
| 1499 | ||
| 1500 | const declaredUnder = DECLARABLE_RELATION_KINDS.filter(kind => | |
| 1501 | relationNamesFor(definition, kind).some(n => n.toLowerCase() === relationName.toLowerCase()), | |
| 1502 | ) | |
| 1503 | const unsupported = declaredUnder.filter(k => !(RESOLVABLE_RELATION_KINDS as readonly string[]).includes(k)) | |
| 1504 | ||
| 1505 | if (unsupported.length > 0) { | |
| 1506 | return `[orm] '${model}' declares '${relationName}' as ${unsupported.join('/')}, which eager loading does not support yet. ` | |
| 1507 | + `Supported kinds: ${RESOLVABLE_RELATION_KINDS.join(', ')}. ` | |
| 1508 | + `Load it with a manual query for now — see stacksjs/bun-query-builder#1068.` | |
| 1509 | } | |
| 1510 | ||
| 1511 | const available = RESOLVABLE_RELATION_KINDS.flatMap(kind => relationNamesFor(definition, kind)) | |
| 1512 | return available.length > 0 | |
| 1513 | ? `[orm] '${model}' has no relation '${relationName}'. Available: ${available.join(', ')}.` | |
| 1514 | : `[orm] '${model}' has no relation '${relationName}' — it declares no relations at all.` | |
| 1515 | } | |
| 1516 | ||
| 1454 | 1517 | function resolveRelation(definition: ModelDefinition, relationName: string): ResolvedRelation | null { |
| 1455 | 1518 | const parentName = definition.name |
| 1456 | 1519 | const parentTable = definition.table |
| @@ -2527,7 +2590,8 @@ class ModelQueryBuilder< | ||
| 2527 | 2590 | relationCache.clear() |
| 2528 | 2591 | relationCache.set(cacheKey, rel) |
| 2529 | 2592 | } |
| 2530 | if (!rel) return | |
| 2593 | if (!rel) | |
| 2594 | throw new Error(unresolvedRelationMessage(this._definition as ModelDefinition, relationName)) | |
| 2531 | 2595 | |
| 2532 | 2596 | if (rel.type === 'hasMany' || rel.type === 'hasOne') { |
| 2533 | 2597 | // Get parent IDs |
| @@ -0,0 +1,103 @@ | ||
| 1 | /** | |
| 2 | * Regression coverage for stacksjs/bun-query-builder#1068. | |
| 3 | * | |
| 4 | * `eagerLoadRelation` used to `return` when it could not resolve a relation | |
| 5 | * name. A misspelling therefore loaded nothing and reported nothing, which is | |
| 6 | * indistinguishable from a relation that genuinely has no rows — the property | |
| 7 | * is simply absent from the result and no error is raised anywhere. | |
| 8 | * | |
| 9 | * The same silence covered every relation kind the resolver does not implement. | |
| 10 | * `ModelDefinition` accepts `morphOne`, `morphMany`, `morphTo`, `morphToMany`, | |
| 11 | * `morphedByMany`, `hasOneThrough` and `hasManyThrough`; `resolveRelation` | |
| 12 | * handles four kinds — `hasMany`, `hasOne`, `belongsTo`, `belongsToMany`. | |
| 13 | * Declaring any of the others type-checks, autocompletes, and does nothing. | |
| 14 | * | |
| 15 | * Both now throw, and they say different things: one is the caller's typo, the | |
| 16 | * other is a gap in this library that staring at the model will never explain. | |
| 17 | */ | |
| 18 | ||
| 19 | import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'bun:test' | |
| 20 | import { configureOrm, createModel, createTableFromModel, getDatabase } from '../src/orm' | |
| 21 | ||
| 22 | const Author = createModel({ | |
| 23 | name: 'RelAuthor', | |
| 24 | table: 'rel_authors', | |
| 25 | primaryKey: 'id', | |
| 26 | autoIncrement: true, | |
| 27 | hasMany: { books: 'RelBook' }, | |
| 28 | // Accepted by the types, never resolved by the loader. | |
| 29 | morphMany: { annotations: 'RelNote' }, | |
| 30 | attributes: { name: { type: 'string', fillable: true } }, | |
| 31 | } as const) | |
| 32 | ||
| 33 | const Book = createModel({ | |
| 34 | name: 'RelBook', | |
| 35 | table: 'rel_books', | |
| 36 | primaryKey: 'id', | |
| 37 | autoIncrement: true, | |
| 38 | attributes: { | |
| 39 | title: { type: 'string', fillable: true }, | |
| 40 | rel_author_id: { type: 'number', fillable: true }, | |
| 41 | }, | |
| 42 | } as const) | |
| 43 | ||
| 44 | const Loner = createModel({ | |
| 45 | name: 'RelLoner', | |
| 46 | table: 'rel_loners', | |
| 47 | primaryKey: 'id', | |
| 48 | autoIncrement: true, | |
| 49 | attributes: { name: { type: 'string', fillable: true } }, | |
| 50 | } as const) | |
| 51 | ||
| 52 | describe('unknown eager-load relations (#1068)', () => { | |
| 53 | beforeAll(() => { | |
| 54 | configureOrm({ database: ':memory:' }) | |
| 55 | }) | |
| 56 | ||
| 57 | beforeEach(async () => { | |
| 58 | const db = getDatabase() | |
| 59 | db.run('DROP TABLE IF EXISTS rel_authors') | |
| 60 | db.run('DROP TABLE IF EXISTS rel_books') | |
| 61 | db.run('DROP TABLE IF EXISTS rel_loners') | |
| 62 | await createTableFromModel(Author.getDefinition()) | |
| 63 | await createTableFromModel(Book.getDefinition()) | |
| 64 | await createTableFromModel(Loner.getDefinition()) | |
| 65 | await Author.create({ name: 'Le Guin' }) | |
| 66 | await Loner.create({ name: 'Solo' }) | |
| 67 | }) | |
| 68 | ||
| 69 | afterAll(() => getDatabase().close()) | |
| 70 | ||
| 71 | it('throws on a misspelled relation instead of loading nothing', async () => { | |
| 72 | // Previously: resolved to undefined, returned, and produced a result with no | |
| 73 | // `boks` property and no complaint. | |
| 74 | await expect(Author.with('boks').get()).rejects.toThrow(/no relation 'boks'/) | |
| 75 | }) | |
| 76 | ||
| 77 | it('lists the relations that do exist, so the typo is obvious', async () => { | |
| 78 | await expect(Author.with('boks').get()).rejects.toThrow(/Available: books/) | |
| 79 | }) | |
| 80 | ||
| 81 | it('says so plainly when the model declares no relations at all', async () => { | |
| 82 | await expect(Loner.with('anything').get()).rejects.toThrow(/declares no relations at all/) | |
| 83 | }) | |
| 84 | ||
| 85 | it('distinguishes a declared-but-unsupported kind from a typo', async () => { | |
| 86 | // `annotations` IS declared — as morphMany, which the loader cannot resolve. | |
| 87 | // Telling the user "no such relation" here would send them looking for a | |
| 88 | // misspelling that isn't there. | |
| 89 | const err = await Author.with('annotations').get().catch((e: Error) => e) | |
| 90 | expect(err).toBeInstanceOf(Error) | |
| 91 | expect((err as Error).message).toMatch(/declares 'annotations' as morphMany/) | |
| 92 | expect((err as Error).message).toMatch(/does not support yet/) | |
| 93 | expect((err as Error).message).not.toMatch(/no relation/) | |
| 94 | }) | |
| 95 | ||
| 96 | it('still loads a supported relation', async () => { | |
| 97 | // The guard must not break the working path — and note the relation | |
| 98 | // resolution is cached per model+name, so this also covers the cache. | |
| 99 | const rows = await Author.with('books').get() | |
| 100 | expect(rows.length).toBe(1) | |
| 101 | await expect(Author.with('books').get()).resolves.toBeDefined() | |
| 102 | }) | |
| 103 | }) | |