Relate a BelongsToOneRelation model that upsertGraph() gets by its key alone

doc/api/types/index.md

@@ -88,7 +88,7 @@ Such relations can be fetched, and the related rows can be updated, patched and
 
 | Property      | Type                | Description                                                                                                                                                                                                                                                                                                                                                                                                     |
 | ------------- | ------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| relate        | boolean<br>string<br>string[] | If `true`, relations are related instead of inserted. Relate functionality can be enabled for a subset of relations of the graph by providing a list of relation paths or a relation expression. See the examples [here](/guide/query-examples.html#graph-upserts).                                                                                                                                                                  |
+| relate        | boolean<br>string<br>string[] | If `true`, relations are related instead of inserted. A `BelongsToOneRelation` model without an id that only gives the relation's key is related by writing that key to its owner, without looking up its row. Relate functionality can be enabled for a subset of relations of the graph by providing a list of relation paths or a relation expression. See the examples [here](/guide/query-examples.html#graph-upserts).                                                                                                                                                                  |
 | unrelate      | boolean<br>string<br>string[] | If `true`, relations are unrelated instead of deleted. Unrelate functionality can be enabled for a subset of relations of the graph by providing a list of relation paths or a relation expression. See the examples [here](/guide/query-examples.html#graph-upserts).                                                                                                                                                               |
 | insertMissing | boolean<br>string<br>string[] | If `true`, models that have identifiers _and_ are not found at their place in the database are inserted with these identifiers. A model whose row exists elsewhere is inserted too, which fails on its unique key unless the graph deletes the row in its other place first: use `relate` to move it instead. By default this is `false` and an error is thrown. This functionality can be enabled for a subset of relations of the graph by providing a list of relation paths or a relation expression. See the examples [here](/guide/query-examples.html#graph-upserts).                                                                              |
 | update        | boolean<br>string<br>string[] | If `true`, update operations are performed instead of patch when altering existing models, affecting the way the data is validated. With update operations, all required fields need to be present in the data provided. This functionality can be enabled for a subset of relations of the graph by providing a list of relation paths or a relation expression. See the examples [here](/guide/query-examples.html#graph-upserts). |

doc/release-notes/changelog.md

@@ -124,6 +124,7 @@
 - `patch()` and `merge()` with field expression keys like `'meta:color'`, and `relate()` through a relation whose key is in a JSON path, write into a `NULL` JSON column, starting from an empty object, or an empty array for a path that starts with an index, where they left it `NULL` and wrote nothing. A `null` written this way gives `{ "key": null }`, e.g. the unrelate of a `BelongsToOneRelation` with its key in a JSON path. On PostgreSQL and MySQL, a nested path still doesn't create its missing parent levels, so the column becomes `{}` without the value, as for columns that hold `{}`
 - `upsertGraph()` writes the foreign keys that it sets or clears in a JSON path of a column, e.g. `join: { from: 'person.id', to: 'pet.data:sitterId' }`, as path updates that keep the other values of the column, also with `noUpdate` and `update`, where it wrote the whole column with only the keys in it and lost the rest. A column that the graph gives other values than the keys, or whose key sits below its top level, e.g. `json:stuff.favoritePetId`, is still written whole. The hooks of such a write see the keys as field expression properties like `'pet.data:sitterId'`, and a change of the hooks to the column itself isn't written. [#415](https://github.com/ditojs/objection/issues/415)
 - `insertGraph()` and `upsertGraph()` throw a `ValidationError` of type `InvalidGraph` for a `#ref` at the root of a graph, which has no relation to place its model in, where they failed with a `TypeError`
+- `upsertGraph()` relates a `BelongsToOneRelation` model that only gives the relation's key, e.g. `pet: { code: 'c1' }` with `relate`, by writing the key to its owner without looking up its row, as with `noUpdate`, and throws a `ValidationError` of type `InvalidGraph` before anything is written when such a model gives other props that it would write, relations, or an `#id` that others refer to. Before, a model like this failed with an error about a missing id unless `noUpdate` was given, also in 3.x, and with `noUpdate`, its relations, e.g. `{ code: 'c1', parent: { id: 2 } }`, were silently left out
 
 ### Types
 

lib/queryBuilder/graph/GraphPlan.js

@@ -91,6 +91,19 @@ export class GraphPlan {
     );
   }
 
+  // Return true if the graph relates the row of `node` by the key of its
+  // BelongsToOneRelation alone, without an id or a reference.
+  isRelatedByKey(node) {
+    return (
+      this.actions.get(node) === 'relate' &&
+      node.relation.keySide === 'owner' &&
+      !node.hasId &&
+      !node.isReference &&
+      !node.isDbReference &&
+      node.propertyReferences.length === 0
+    );
+  }
+
   // Return true if the model of the `#ref` model `node` writes what `node`
   // would write to the row: The model is inserted, or it's written and its path
   // allows any write that the path of `node` allows.

lib/queryBuilder/graph/GraphUpsert.js

@@ -9,6 +9,7 @@ import {
   checkChangedIds,
   checkGraph,
   checkJoinTables,
+  checkKeyRelates,
   checkMissingModels,
   checkPlacements,
   checkReadKeys,
@@ -100,6 +101,7 @@ export class GraphUpsert {
       }
 
       checkChangedIds(builder, plan);
+      checkKeyRelates(builder, plan);
       checkReferences(builder, plan);
       checkJoinTables(builder, plan);
       await new GraphWriter(new GraphSchedule(plan, keys)).write(builder);

lib/queryBuilder/graph/GraphWriter.js

@@ -288,6 +288,12 @@ export class GraphWriter {
   _preparePatch(builder, node) {
     const { referencedNode } = node;
 
+    // A row that the graph relates by its key alone isn't looked up, its key is
+    // only written to its owner:
+    if (this.plan.isRelatedByKey(node)) {
+      return null;
+    }
+
     // The props that the graph fills into the model of a `#ref` model are only
     // final now:
     if (referencedNode) {

lib/queryBuilder/graph/graphChecks.js

@@ -445,6 +445,42 @@ export function checkReferences(builder, plan) {
   }
 }
 
+// A model that the graph relates by the key of its BelongsToOneRelation alone
+// isn't looked up: only its key is written to its owner. So it can't give other
+// props, unless they aren't written, relations or values that others refer to.
+export function checkKeyRelates(builder, plan) {
+  const { graphOptions } = plan;
+
+  for (const node of plan.activeNodes.filter((node) => plan.isRelatedByKey(node))) {
+    const { modelClass, relation, obj } = node;
+    const relationNames = modelClass.getRelationNames();
+    const idProps = modelClass.getIdPropertyArray();
+    const isWritten = graphOptions.shouldPatch(node) || graphOptions.shouldUpdate(node);
+    const name = Object.keys(obj).find((key) => {
+      const value = obj[key];
+      return (
+        value !== undefined &&
+        !isInternalProp(key) &&
+        !relation.relatedProp.props.includes(key) &&
+        !(value === null && idProps.includes(key)) &&
+        (relationNames.includes(key) || isWritten)
+      );
+    });
+    const isReferenced =
+      node.propertyReferencingNodes.length > 0 ||
+      plan.nodes.some((other) => other.referencedNode === node);
+
+    if (name || isReferenced) {
+      throw builder.modelClass().createValidationError({
+        type: ValidationErrorType.InvalidGraph,
+        message: `the ${modelClass.name} at ${formatPath(node.dataPathKey)} is related by its key alone, so ${
+          name ? `its ${name} can't be written` : "it can't be referenced"
+        }`,
+      });
+    }
+  }
+}
+
 // A `#ref{}` waits for the model it reads if the value only exists once that
 // model is written. These waits follow from the graph alone, so their cycles
 // are found before the first write.

tests/integration/misc/upsertGraphRelateByKey.js

@@ -0,0 +1,129 @@
+import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest';
+import { Model } from 'objection';
+
+export default (session) => {
+  describe('upsertGraph() relating a BelongsToOneRelation model by its key alone', () => {
+    const { knex } = session;
+
+    class Person extends Model {
+      static get tableName() {
+        return 'relate_key_person';
+      }
+
+      static get relationMappings() {
+        return {
+          parent: {
+            relation: Model.BelongsToOneRelation,
+            modelClass: Person,
+            join: { from: 'relate_key_person.parentId', to: 'relate_key_person.id' },
+          },
+        };
+      }
+    }
+
+    class Pet extends Model {
+      static get tableName() {
+        return 'relate_key_pet';
+      }
+
+      static get relationMappings() {
+        return {
+          owner: {
+            relation: Model.BelongsToOneRelation,
+            modelClass: Person,
+            join: { from: 'relate_key_pet.ownerCode', to: 'relate_key_person.code' },
+          },
+        };
+      }
+    }
+
+    beforeAll(() =>
+      knex.schema
+        .dropTableIfExists('relate_key_pet')
+        .dropTableIfExists('relate_key_person')
+        .createTable('relate_key_person', (table) => {
+          table.integer('id').primary();
+          table.string('code');
+          table.string('label');
+          table.integer('parentId');
+        })
+        .createTable('relate_key_pet', (table) => {
+          table.integer('id').primary();
+          table.string('ownerCode');
+        }),
+    );
+
+    afterAll(() =>
+      knex.schema.dropTableIfExists('relate_key_pet').dropTableIfExists('relate_key_person'),
+    );
+
+    beforeEach(async () => {
+      await knex('relate_key_pet').delete();
+      await knex('relate_key_person').delete();
+      await knex('relate_key_person').insert([
+        { id: 1, code: 'c1' },
+        { id: 2, code: 'c2' },
+      ]);
+      await knex('relate_key_pet').insert([{ id: 1 }, { id: 2 }]);
+    });
+
+    const fetchOwnerCodes = async () =>
+      (await knex('relate_key_pet').orderBy('id')).map(({ ownerCode }) => ownerCode);
+
+    it('should write the key of the model to its owner', async () => {
+      await Pet.query(knex).upsertGraph(
+        { id: 1, owner: { code: 'c1', label: undefined } },
+        { relate: true },
+      );
+
+      expect(await fetchOwnerCodes()).toEqual(['c1', null]);
+    });
+
+    it("should write the key without looking up the model's row", async () => {
+      await Pet.query(knex).upsertGraph({ id: 1, owner: { code: 'zz' } }, { relate: true });
+
+      expect(await fetchOwnerCodes()).toEqual(['zz', null]);
+    });
+
+    it('should leave out the other props of the model under noUpdate', async () => {
+      await Pet.query(knex).upsertGraph(
+        { id: 1, owner: { code: 'c1', label: 'l' } },
+        { relate: true, noUpdate: true },
+      );
+
+      expect(await fetchOwnerCodes()).toEqual(['c1', null]);
+      expect(await knex('relate_key_person').where('id', 1).first()).toMatchObject({
+        label: null,
+      });
+    });
+
+    for (const [title, graph, message] of [
+      [
+        'other props',
+        { id: 1, owner: { code: 'c1', label: 'l' } },
+        "the Person at 'owner' is related by its key alone, so its label can't be written",
+      ],
+      [
+        'relations',
+        { id: 1, owner: { code: 'c1', parent: { id: 2 } } },
+        "the Person at 'owner' is related by its key alone, so its parent can't be written",
+      ],
+      [
+        'a #id that a #ref{} refers to',
+        [
+          { id: 1, owner: { '#id': 'o', code: 'c1' } },
+          { id: 2, ownerCode: '#ref{o.code}' },
+        ],
+        "the Person at '[0].owner' is related by its key alone, so it can't be referenced",
+      ],
+    ]) {
+      it(`should reject a model with ${title} before any write`, async () => {
+        await expect(
+          Pet.query(knex).upsertGraph(graph, { relate: true, allowRefs: true }),
+        ).rejects.toThrow(message);
+
+        expect(await fetchOwnerCodes()).toEqual([null, null]);
+      });
+    }
+  });
+};