From fc694a275328510f6f73a7fe2049ebdbb204c4a9 Mon Sep 17 00:00:00 2001 From: Izel Nakri Date: Mon, 29 May 2023 04:04:04 +0200 Subject: [PATCH] initial validators for relationship & columns --- packages/@memoria/adapters/src/rest/index.ts | 52 ++++++++------ .../adapters/test/memory/insert-test.ts | 3 +- .../belongs-to-id-for-has-many-test.ts | 20 +++--- packages/@memoria/model/src/model.ts | 12 ++-- .../@memoria/model/src/setters/foreign-key.ts | 37 ++++++++-- .../@memoria/model/src/stores/instance/db.ts | 17 ++--- .../model/src/stores/relationship/db.ts | 30 ++++++-- .../model/src/stores/relationship/schema.ts | 15 ++++ packages/@memoria/model/test/build-test.ts | 71 ++++++++++++++++++- 9 files changed, 197 insertions(+), 60 deletions(-) diff --git a/packages/@memoria/adapters/src/rest/index.ts b/packages/@memoria/adapters/src/rest/index.ts index 2273e5d..fd399b5 100644 --- a/packages/@memoria/adapters/src/rest/index.ts +++ b/packages/@memoria/adapters/src/rest/index.ts @@ -243,7 +243,7 @@ export default class RESTAdapter extends MemoryAdapter { static fetchRelationship(model: MemoriaModel, relationshipName: string, relationshipMetadata?: RelationshipMetadata) { let Model = model.constructor as typeof MemoriaModel; let metadata = relationshipMetadata || RelationshipSchema.getRelationshipMetadataFor(Model, relationshipName); - let { relationshipType, RelationshipClass, reverseRelationshipName } = metadata; + let { SourceClass, relationshipType, RelationshipClass, reverseRelationshipName } = metadata; return new RelationshipPromise(async (resolve, reject) => { try { @@ -254,42 +254,48 @@ export default class RESTAdapter extends MemoryAdapter { } return resolve( - RelationshipDB.cacheRelationship(model, metadata, await RelationshipClass.find(model[foreignKeyColumnName])) + RelationshipDB.cacheRelationship( + model, + metadata, + await RelationshipClass.find(model[foreignKeyColumnName] as PrimaryKey) + ) ); } else if (relationshipType === "OneToOne") { - if (reverseRelationshipName) { - let reverseRelationshipForeignKeyColumnName = metadata.reverseRelationshipForeignKeyColumnName as string; - let relationship = model[Model.primaryKeyName] - ? await RelationshipClass.findBy({ - [reverseRelationshipForeignKeyColumnName]: model[Model.primaryKeyName], - }) - : null; - - return resolve(RelationshipDB.cacheRelationship(model, metadata, relationship || null)); + let reverseRelationshipForeignKeyColumnName = metadata.reverseRelationshipForeignKeyColumnName as string; + if (!reverseRelationshipForeignKeyColumnName || !reverseRelationshipName) { + throw new Error( + `${RelationshipClass.name} missing a foreign key column or @BelongsTo declaration for ${SourceClass.name} on ${relationshipName} @hasOne relationship!` + ); } - return reject(); + let relationship = model[Model.primaryKeyName] + ? await RelationshipClass.findBy({ + [reverseRelationshipForeignKeyColumnName]: model[Model.primaryKeyName], + }) + : null; + + return resolve(RelationshipDB.cacheRelationship(model, metadata, relationship)); } else if (relationshipType === "HasMany") { - if (reverseRelationshipName) { - let foreignKeyColumnName = metadata.foreignKeyColumnName as string; - return resolve( - RelationshipDB.cacheRelationship( - model, - metadata, - await RelationshipClass.findAll({ - [foreignKeyColumnName]: model[Model.primaryKeyName], - }) - ) + let reverseRelationshipForeignKeyColumnName = metadata.reverseRelationshipForeignKeyColumnName as string; + if (!reverseRelationshipForeignKeyColumnName) { + throw new Error( + `${RelationshipClass.name} missing a foreign key column for ${SourceClass.name} on ${relationshipName} @hasMany relationship!` ); } - return reject(); + let relationship = model[Model.primaryKeyName] + ? await RelationshipClass.findAll({ [reverseRelationshipForeignKeyColumnName]: model[Model.primaryKeyName] }) + : []; + // NOTE: peekAll generate new instances each time, this is a feature, not a bug(?). That way when we mutate foreignKey of existing record, hasMany array stays in tact + + return resolve(RelationshipDB.cacheRelationship(model, metadata, relationship)); } } catch (error) { return reject(error); } return reject("ManyToMany fetchRelationship not implemented yet"); + // return reject(null); // NOTE: ManyToMany not implemented yet. }); } } diff --git a/packages/@memoria/adapters/test/memory/insert-test.ts b/packages/@memoria/adapters/test/memory/insert-test.ts index a6060df..2d42b98 100644 --- a/packages/@memoria/adapters/test/memory/insert-test.ts +++ b/packages/@memoria/adapters/test/memory/insert-test.ts @@ -401,8 +401,9 @@ module("@memoria/adapters | MemoryAdapter | $Model.insert()", function (hooks) { } }); - let somePeekedModel = await MemoryGroup.peek(group.uuid); + let somePeekedModel = await MemoryGroup.peek(group.uuid); // TODO: PEEK GENERATES A RelationshipCache RECORD!! WTF + assert.equal(RelationshipDB.has(cachedReference, "owner"), false); assert.strictEqual(groupPhoto.group, insertedGroup); let newBuiltReference = MemoryGroup.build({ diff --git a/packages/@memoria/adapters/test/memory/relationships/belongs-to-id-for-has-many-test.ts b/packages/@memoria/adapters/test/memory/relationships/belongs-to-id-for-has-many-test.ts index d83f467..c5d2868 100644 --- a/packages/@memoria/adapters/test/memory/relationships/belongs-to-id-for-has-many-test.ts +++ b/packages/@memoria/adapters/test/memory/relationships/belongs-to-id-for-has-many-test.ts @@ -1,7 +1,7 @@ // ChatGPT: Build me test cases & implementation below on this file based on the previous tests & files: // NOTE: Maybe in future test quality upgrade: add one extra photo to the tests, make intermediary built instances, check their upgrade on fetch import { module, test } from "qunitx"; -import { RelationshipPromise, RelationshipDB, InstanceDB } from "@memoria/model"; +import { RelationshipPromise, RelationshipDB } from "@memoria/model"; import setupMemoria from "../../helpers/setup-memoria.js"; import generateModels from "../../helpers/models-with-relations/memory/id/index.js"; @@ -309,7 +309,7 @@ module( test("Fetched model can remove the relationship before update", async function (assert) { let { MemoryPhoto, MemoryUser } = generateModels(); - let user = await MemoryUser.insert({ name: "Some user" }); + let user = await MemoryUser.insert({ first_name: "Izel" }); let photo = await MemoryPhoto.insert({ name: "Dinner photo", owner_id: user.id }); assert.strictEqual(photo.owner, user); @@ -341,7 +341,7 @@ module( test("Fetched model can remove the relationship before delete", async function (assert) { let { MemoryPhoto, MemoryUser } = generateModels(); - let user = await MemoryUser.insert({ name: "Some user" }); + let user = await MemoryUser.insert({ first_name: "Izel" }); let photo = await MemoryPhoto.insert({ name: "Dinner photo", owner_id: user.id }); assert.strictEqual(photo.owner, user); @@ -374,7 +374,7 @@ module( test("Fallback behavior for fkey mutations clear the HasManyArray when there is cached & persisted but no instances of the reference with fkey pointing to it", async function (assert) { let { MemoryPhoto, MemoryUser } = generateModels(); - let user = await MemoryUser.insert({ name: "Some user" }); + let user = await MemoryUser.insert({ first_name: "Izel" }); let photo = await MemoryPhoto.insert({ name: "Dinner photo", owner_id: user.id }); assert.strictEqual(photo.owner, user); @@ -406,8 +406,8 @@ module( test("When related models reflective relationships are completely cleared it doesnt clear the foreign key, just the relationship(previous pointers) of and to the model", async function (assert) { let { MemoryPhoto, MemoryUser } = generateModels(); - let user = await MemoryUser.insert({ name: "Some user" }); - let secondUser = await MemoryUser.insert({ name: "Another user" }); + let user = await MemoryUser.insert({ first_name: "Izel" }); + let secondUser = await MemoryUser.insert({ first_name: "John" }); let thirdUser = MemoryUser.build({ id: 3, name: "Third user" }); let photo = await MemoryPhoto.insert({ name: "Dinner photo", owner_id: user.id }); @@ -546,8 +546,8 @@ module( test("Reverse relationship can be built, created, updated, deleted with correct changing relationships in one flow", async function (assert) { let { MemoryPhoto, MemoryUser } = generateModels(); - let firstUser = await MemoryUser.insert({ name: "Some group" }); - let secondUser = await MemoryUser.insert({ name: "Some group" }); + let firstUser = await MemoryUser.insert({ first_name: "Izel" }); + let secondUser = await MemoryUser.insert({ first_name: "John" }); let photo = MemoryPhoto.build({ name: "Dinner photo", owner: secondUser }); assert.strictEqual(photo.owner, secondUser); @@ -605,11 +605,11 @@ module( MemoryUser.cache([ { id: 1, - name: "Some group", + first_name: "Izel", }, { id: 2, - name: "Another group", + first_name: "John", }, ]); diff --git a/packages/@memoria/model/src/model.ts b/packages/@memoria/model/src/model.ts index f80c141..89f1d8e 100644 --- a/packages/@memoria/model/src/model.ts +++ b/packages/@memoria/model/src/model.ts @@ -14,7 +14,7 @@ import { InstanceDB, } from "./stores/index.js"; import { clearObject, primaryKeyTypeSafetyCheck } from "./utils/index.js"; -import { validatePartialModelInput } from "./validations/index.js"; +import { validatePartialModelInput } from "./validators/index.js"; // import ArrayIterator from "./utils/array-iterator.js"; import type { ModelReference, RelationshipType } from "./index.js"; import definePrimaryKeySetter from "./setters/primary-key.js"; @@ -113,7 +113,7 @@ export default class Model { } let buildOptions = { copy: false, revision: true, ...options }; - let model = new this(buildOptions); + let model = new this(buildOptions); // TODO: Move buildObject validations here if (buildObject) { if (buildObject.revisionHistory) { @@ -133,7 +133,7 @@ export default class Model { let belongsToColumnNames = RelationshipSchema.getBelongsToColumnNames(this); // NOTE: this creates Model.belongsToColumnNames once, which is needed for now until static { } Module init closure let belongsToTable = RelationshipSchema.getBelongsToColumnTable(this); - let existingInstances = InstanceDB.getOrCreateExistingInstancesSet(model, buildObject, buildObject[this.primaryKeyName] || null); + let existingInstances = InstanceDB.getOrCreateExistingInstancesSet(model, buildObject, buildObject[this.primaryKeyName] || null); // NOTE: This shouldnt create an empty set if validations fail Array.from(this.columnNames).forEach((columnName) => { if (columnName === this.primaryKeyName) { @@ -145,6 +145,10 @@ export default class Model { } }); + // NOTE: At this point model is not in existingInstances array because validations can run and throw exceptions! + // Removed the generation on InstanceDB.getOrCreateExistingInstancesSet when primaryKey is not there + existingInstances.add(model); + let relationshipTable = RelationshipSchema.getRelationshipTable(this); Object.keys(relationshipTable).forEach((relationshipName) => { let buildObjectType = getBuildObjectType(buildObject, this); @@ -166,7 +170,7 @@ export default class Model { }); }); - existingInstances.add(model); + // existingInstances.add(model); return revisionAndLockModel(model, options, buildObject); } diff --git a/packages/@memoria/model/src/setters/foreign-key.ts b/packages/@memoria/model/src/setters/foreign-key.ts index c0a4425..c30a2b6 100644 --- a/packages/@memoria/model/src/setters/foreign-key.ts +++ b/packages/@memoria/model/src/setters/foreign-key.ts @@ -1,8 +1,9 @@ import Model from "../model.js"; -import { RelationshipMutation } from "../stores/index.js"; +import { RelationshipMutation, RelationshipSchema } from "../stores/index.js"; import type { RelationshipMetadata } from "../stores/index.js"; import { transformValue } from "../serializer.js"; import type { ModelBuildOptions } from "../model.js"; +import { validateRelationshipInput } from "../validators/index.js"; type QueryObject = { [key: string]: any }; @@ -13,12 +14,11 @@ export default function defineForeignKeySetter( buildOptions: ModelBuildOptions, relationshipMetadata: RelationshipMetadata ) { - let { RelationshipClass, RelationshipCache, relationshipName } = relationshipMetadata; - let cache = buildObject && buildObject[relationshipName] && RelationshipClass.primaryKeyName in buildObject[relationshipName] - ? buildObject[relationshipName][RelationshipClass.primaryKeyName] || getTransformedValue(model, columnName, buildObject) + let { RelationshipCache } = relationshipMetadata; + let cache = hasProvidedRelationship(buildObject, relationshipMetadata) + ? generateForeignKeyValueFromRelationshipOrProvidedValue(model, columnName, buildObject, relationshipMetadata) : getTransformedValue(model, columnName, buildObject); - debugger; // TODO: add the mutation here for once, is this really needed(?) return Object.defineProperty(model, columnName, { configurable: false, @@ -54,6 +54,33 @@ export default function defineForeignKeySetter( }); } +function hasProvidedRelationship(buildObject: QueryObject | Model, { RelationshipCache, relationshipName }: RelationshipMetadata) { + return buildObject instanceof Model ? !!RelationshipCache.has(buildObject) : relationshipName in buildObject; +} + +function generateForeignKeyValueFromRelationshipOrProvidedValue( + model: Model, + columnName: string, + buildObject: QueryObject | Model, + relationshipMetadata : RelationshipMetadata +) { + let { RelationshipClass, relationshipName, reverseRelationshipName } = relationshipMetadata; + let relationshipReference = buildObject[relationshipName]; + if (!relationshipReference || !(RelationshipClass.primaryKeyName in relationshipReference)) { + return getTransformedValue(model, columnName, buildObject); + } else if (buildObject[columnName] !== undefined) { + let reverseMetadata = RelationshipSchema.getRelationshipMetadataFor(RelationshipClass, reverseRelationshipName); + + validateRelationshipInput( + reverseMetadata.relationshipType === 'HasMany' ? [buildObject] : buildObject, + model.constructor as typeof Model, + reverseMetadata + ); + } + + return relationshipReference[RelationshipClass.primaryKeyName] || getTransformedValue(model, columnName, buildObject); +} + function getTransformedValue(model: Model, keyName: string, buildObject?: QueryObject | Model) { return buildObject && keyName in buildObject ? transformValue(model.constructor as typeof Model, keyName, buildObject[keyName]) diff --git a/packages/@memoria/model/src/stores/instance/db.ts b/packages/@memoria/model/src/stores/instance/db.ts index 38474cc..e7b3d94 100644 --- a/packages/@memoria/model/src/stores/instance/db.ts +++ b/packages/@memoria/model/src/stores/instance/db.ts @@ -57,12 +57,13 @@ export default class InstanceDB { : this.getAllUnknownInstances(Class).find((modelSet) => modelSet.has(model)) as Set; } + // NOTE: This could be improved in terms of memory because thrown build() instanceSets are not deleted from memory or has side effects static getOrCreateExistingInstancesSet(model: Model, buildObject: JSObject, primaryKey?: PrimaryKey) { let Class = model.constructor as typeof Model; if (primaryKey) { - let references = this.getAllKnownReferences(Class); - let foundInstanceSet = references.get(primaryKey); + let knownReferences = this.getAllKnownReferences(Class); + let foundInstanceSet = knownReferences.get(primaryKey); if (!foundInstanceSet) { let unknownReferences = this.getAllUnknownInstances(Class); if (buildObject instanceof Model) { @@ -75,22 +76,22 @@ export default class InstanceDB { foundInstanceSet = new Set(); } - references.set(primaryKey, foundInstanceSet); + knownReferences.set(primaryKey, foundInstanceSet); } return foundInstanceSet; } else if (buildObject instanceof Model) { - let references = this.getAllUnknownInstances(Class); - let foundInstanceSet = references.find((modelSet) => modelSet.has(buildObject as Model)); + let unknownReferences = this.getAllUnknownInstances(Class); + let foundInstanceSet = unknownReferences.find((modelSet) => modelSet.has(buildObject as Model)); if (!foundInstanceSet) { - foundInstanceSet = new Set([model]); - references.push(foundInstanceSet); + foundInstanceSet = new Set(); + unknownReferences.push(foundInstanceSet); } return foundInstanceSet; } - let foundInstanceSet: Set = new Set([model]); + let foundInstanceSet: Set = new Set(); this.getAllUnknownInstances(Class).push(foundInstanceSet); return foundInstanceSet; diff --git a/packages/@memoria/model/src/stores/relationship/db.ts b/packages/@memoria/model/src/stores/relationship/db.ts index ff9cd5b..3c201ff 100644 --- a/packages/@memoria/model/src/stores/relationship/db.ts +++ b/packages/@memoria/model/src/stores/relationship/db.ts @@ -6,6 +6,7 @@ import InstanceDB from "../instance/db.js"; import { clearObject } from "../../utils/index.js"; import type { RelationshipMetadata } from "./schema.js"; import HasManyArray from "../../has-many-array.js"; +import { RelationshipPromise } from "../../promises/index.js"; type RelationshipTableKey = string; // Example: "MemoryUser:comments" type AnotherModel = Model; @@ -76,13 +77,17 @@ export default class RelationshipDB { }); } + // TODO: This is removed because let photo = await RESTPhoto.insert({ name: "Dinner photo", owner: user }); immediately clears owner cache for photo.owner! + // NOTE: just plain wrong logic, not needed // NOTE: TRY: dont delete references related to the same model, only delete references related to the same model with different id(?) - outputRecord.fetchedRelationships.forEach((relationshipName: string) => { - let relationship = RelationshipDB.findRelationshipFor(outputRecord, relationshipName); - if (relationship && relationship[(relationship.constructor as typeof Model).primaryKeyName]) { - RelationshipDB.findRelationshipCacheFor(Class, relationshipName).delete(outputRecord); - } - }); + // outputRecord.fetchedRelationships.forEach((relationshipName: string) => { + // let relationship = RelationshipDB.findRelationshipFor(outputRecord, relationshipName); + + // // TODO: This is messed up, because insert() does this! with assignment that shouldnt be cleared, it clears all assignments! + // // if (relationship && relationship[(relationship.constructor as typeof Model).primaryKeyName]) { + // // RelationshipDB.findRelationshipCacheFor(Class, relationshipName).delete(outputRecord); + // // } + // }); InstanceDB.makeModelPersisted(outputRecord); @@ -480,3 +485,16 @@ function generateNewArrayFromInputIfNeeded(input, model, metadata) { return input instanceof Model ? input : null; } + +const SINGLE_VALUE_RELATIONSHIPS = new Set(["BelongsTo", "OneToOne"]); + +function isInvalidRelationshipInput(input, metadata) { + if (input === null || input instanceof RelationshipPromise) { + return false; + } else if (SINGLE_VALUE_RELATIONSHIPS.has(metadata.relationshipType) && !(input instanceof Model)) { + return true; + } else if (metadata.relationshipType === "HasMany" && !(input instanceof Model || Array.isArray(input))) { + // TODO: also make sure if its array that all of its values are models + return true; + } +} diff --git a/packages/@memoria/model/src/stores/relationship/schema.ts b/packages/@memoria/model/src/stores/relationship/schema.ts index a30d88d..a12a737 100644 --- a/packages/@memoria/model/src/stores/relationship/schema.ts +++ b/packages/@memoria/model/src/stores/relationship/schema.ts @@ -22,6 +22,21 @@ export interface RelationshipMetadata { reverseRelationshipForeignKeyColumnName: null | string; } +export interface BelongsToRelationshipMetadata extends RelationshipMetadata { + foreignKeyColumnName: string; + reverseRelationshipForeignKeyColumnName: null; +} + +export interface HasOneRelationshipMetadata extends RelationshipMetadata { + foreignKeyColumnName: null; + reverseRelationshipForeignKeyColumnName: string; +} + +export interface HasManyRelationshipMetadata extends RelationshipMetadata { + foreignKeyColumnName: null; + reverseRelationshipForeignKeyColumnName: string; +} + export interface RelationshipTable { [relationshipName: string]: RelationshipMetadata; } diff --git a/packages/@memoria/model/test/build-test.ts b/packages/@memoria/model/test/build-test.ts index 1edd0b8..8b31cfe 100644 --- a/packages/@memoria/model/test/build-test.ts +++ b/packages/@memoria/model/test/build-test.ts @@ -1,5 +1,5 @@ import setupRESTModels from "@memoria/adapters/test/helpers/models-with-relations/rest/id/index.js"; -import Model, { Changeset, PrimaryGeneratedColumn, Column, Serializer } from "@memoria/model"; +import Model, { Changeset, PrimaryGeneratedColumn, Column, InstanceDB } from "@memoria/model"; import { module, test } from "qunitx"; import setupMemoria from "./helpers/setup-memoria.js"; @@ -212,10 +212,75 @@ module("@memoria/model | $Model.build() tests", function (hooks) { let firstPhoto = MemoryPhoto.build({ name: "Family photo", owner: users[0] }); let secondPhoto = MemoryPhoto.build({ name: "Trip photo", owner: users[1] }); - assert.propEqual(firstPhoto.owner, users[0]); assert.equal(firstPhoto.owner_id, users[0].id); - assert.propEqual(secondPhoto.owner, users[1]); + assert.propEqual(firstPhoto.owner, users[0]); assert.equal(secondPhoto.owner_id, users[1].id); + assert.propEqual(secondPhoto.owner, users[1]); + }); + + test('When $Model.build() is provided with a foreign key and the reference as pure object it sets it correctly', async function (assert) { + let { Server, RESTUser, RESTPhoto } = setupRESTModels(); + this.Server = Server; + + let user = await RESTUser.insert({ first_name: "Izel" }); + let copiedUser = RESTUser.build(user); + + assert.notStrictEqual(user, copiedUser); + assert.deepEqual(user.toJSON(), copiedUser.toJSON()); + + let builtPhoto = RESTPhoto.build({ name: "Dinner photo", owner: user, owner_id: user.id }); + + assert.equal(builtPhoto.owner_id, 1); + assert.strictEqual(builtPhoto.owner, user); + + let anotherBuiltPhoto = RESTPhoto.build({ + name: "Dinner photo", + owner: { id: 1, first_name: "Izel", last_name: null }, + owner_id: user.id + }); + + assert.equal(anotherBuiltPhoto.owner_id, 1); + assert.notStrictEqual(anotherBuiltPhoto.owner, user); + assert.deepEqual(anotherBuiltPhoto.owner.toJSON(), user.toJSON()); + }); + + // NOTE: Move this to validation tests along with other validation tests. + test('When $Model.build() is provided with mismatched foreign key and reference with wrong primary key it throws!', async function (assert) { + let { Server, RESTUser, RESTPhoto } = setupRESTModels(); + this.Server = Server; + + debugger; + let user = await RESTUser.insert({ first_name: "Izel" }); + let copiedUser = RESTUser.build(user); + + assert.notStrictEqual(user, copiedUser); + assert.deepEqual(user.toJSON(), copiedUser.toJSON()); + + try { + RESTPhoto.build({ name: "Dinner photo", owner: user, owner_id: 99 }); + } catch (error) { + assert.equal(error.message, 'You cannot provide different owner_id: 99 and owner.id: 1 for RESTPhoto partial!'); + } + + try { + RESTPhoto.build({ + name: "Dinner photo", + owner: { id: 55, first_name: "Izel", last_name: null }, + owner_id: user.id + }); + } catch (error) { + assert.equal(error.message, 'You cannot provide different owner_id: 1 and owner.id: 55 for RESTPhoto partial!'); + } + + assert.equal(InstanceDB.getAllUnknownInstances(RESTPhoto).length, 2); // NOTE: Make this 0 in future + assert.ok(InstanceDB.getAllUnknownInstances(RESTPhoto).every((set) => set.size === 0)); + + let builtPhoto = RESTPhoto.build({ name: "Dinner photo", owner: user, owner_id: user.id }); + + assert.equal(InstanceDB.getAllUnknownInstances(RESTPhoto).length, 3); // NOTE: Make this 1 in future + assert.deepEqual(InstanceDB.getAllUnknownInstances(RESTPhoto)[2], new Set([builtPhoto])); + assert.equal(builtPhoto.owner_id, 1); + assert.strictEqual(builtPhoto.owner, user); }); });