Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Automatic handling of to_one conflicting foreign keys #78

Open
coeit opened this issue Oct 28, 2021 · 3 comments
Open

Feature: Automatic handling of to_one conflicting foreign keys #78

coeit opened this issue Oct 28, 2021 · 3 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@coeit
Copy link
Member

coeit commented Oct 28, 2021

Summary

Currently when managing a x_to_one association there is no mechanism in place to alert the user of possible ramifications when updating the association.

many_to_one

In case of a many_to_one this means zendro will just overwrite the foreignKey without warning the user, which could be seen as an intended feature. There will be no "faulty" data written

one_to_one

Since Zendro handles a one_to_one very much the same to a many_to_one (via a foreignKey in one of the models) currently there is no checks done that a create or update mutation could lead to any unexpected data. The user can counteract this behavior by specifically setting a unique constraint on the foreignKey column. Zendro will warn the user on code-generation about this. If ignored this can lead to unexpected "faulty" data where the uniqueness of the association is violated.

DDM

In case of a ddm it is possible, though probably not very common, to using zendro to distribute data of a single table over multiple databases. In this case a unique constraint on the database column directly would not be sufficient to ensure the one_to_one integrity.

Solutions

Backend

On code-generation use the asyncValidatorFunction to automatically mimic a unique constraint on foreignKey columns (one_to_one). This would prevent creation/update of records by throwing a validationError, independent of storageType. Note that this is a performance-hampering process and should be documented as such.

Frontend

The SPA can automatically prevent writing of faulty data by preventing the user to use the functions or automatically correct data (if a new associated record is created from the end that doesn't hold the foreignKey we need to manually remove a possible already existing association).

Notes

Make sure the user is aware of zendro's handling of one_to_one association by documenting the above.

@coeit coeit added bug Something isn't working enhancement New feature or request labels Oct 28, 2021
@coeit coeit self-assigned this Oct 28, 2021
@coeit
Copy link
Member Author

coeit commented Oct 29, 2021

To handle this we can in the resolver (e.g. country.prototype.add_unique_capital), before calling the add_assoc function call a remove_assoc. The remove_assoc implementations need to be changed to handle passing a null or undefined for the first argument like below:

one_to_one remove associations:

sql

let updated = await country.update({
    continent_id: null
}, {
    where: {
        // country_id: country_id,
        continent_id: continent_id
    }
});

updated contains the number of removed assocs in array: e.g [4]

neo4j

let foreignKey = `MATCH (n:Movie ) SET n.director_id = $target RETURN head(collect(n))`;

let delete_relationships = `MATCH (a:Movie)-[r:${ "director".toUpperCase() + "_EDGE"
    }]-> (b:Director) WHERE b.${models.director.idAttribute()} = $target DELETE r`;

delete_relationships.summary.counters.relationshipsDelete holds the counter of updated records

mongodb

const response = await collection.updateMany(
        {
          //   animal_id: animal_id,
          farm_id: farm_id,
        },
        {
          $set: updatedContent,
        }
      );

response.modifiedCount holds the counter

cassandra

not possible. need to fetch inside model, i guess...

@wunderbarr
Copy link
Member

Picking up on the above CE discussion:
Step 4, namely thePerson.parrot().getIdValue() call actually carries out a "global" search in all Parrot records to find those that have person_id set to the respective associated value. Thus the inefficiency in this CE step cannot be avoided. In fact, even in the setup of a local relational database the UPDATE parrots SET person_id = NULL WHERE person_id = <associated_value> SQL statement will cause the whole parrots table to be searched, hopefully using an index on the person_id column.

In conclusion, looking at this from a conceptual point of view, the single-end foreign-key implementation of one-to-one associations requires a to a degree inefficient consistency insurance.

And correctly did we previously identify a single-end foreign key implementation of a one-to-one association as transition to the paired-end implementations of associations, because this specific case implies the requirement for updating two ends of the associations, even though just a single foreign key is used.

There is in non-distributed setup and non-Cassandra storages the option to implement the CE in one less step, i.e. executing an UPDATE statement like the above example in SQL.
Asis assumes that currently the CE is implemented in the add__id data model function. In the above Person one-to-one Parrot (association is called "unique_person") example:

// Parrot, holding the foreign key 'person_id'
// Data Model Parrot
async add_unique_person( person_id ) {
  // Implement consistency insurance here:
  const removed = await parrot.remove_person_id( parrot_id: null, person_id: person_id, ...); 
  // then do the "normal" association stuff
}

This issue shows that the above "extension" of the parrot.remove_person_id does not work in all storages, i.e. distributed setup and Cassandra.
We could thus modify the above to the following:

// Parrot Data Model
// =================
// this is a STATIC function only generated for single-end foreign key implementations of one-to-one associations (please put into the generated comments)
async function disassociate_person_id( person_id )
  // DETAILED BEHAVIOR:
  // ------------------
  // in all cases that are NOT distributed NOR Cassandra, do something NOT depending on the parrot_id argument:
  await raw_query( `UPDATE parrots SET person_id = NULL WHERE person_id = ${person_id}` );
  // in Cassandra or Distributed setup, we DO NEED TO GET all parrots that have Person of person_id argument associated:
  const parrots = await readAllCursor( search: {field: 'person_id', operator: 'eq', value: person_id} );
  const updates = await Promise.all( parrots.map( x => remove_person_id( x.id, person_id ) );

  // *OR* INVOKE THE EXTENDED VERSION OF remove_person_id as follows,
  // and depending on the storage type:
  // all storage types _except_ distributed and Cassandra:
  const removed = await remove_person_id( parrot_id: null, person_id: person_id, ...)
  // DDMs and Cassandra models:
  const parrots = await readAllCursor( search: {field: 'person_id', operator: 'eq', value: person_id} );
  const updates = await Promise.all( parrots.map( x => remove_person_id( x.id, person_id ) );
}

// Parrot resolver (MODIFIED version)
// ==================================
async add_unique_person( person_id ) {
  // Ensure consistency here:
  disassociate_person_id( person_id );
  const removed = await parrot.remove_person_id( parrot_id: null, person_id: person_id, ...); 
  // then do the "normal" association stuff
}

How could we generate the above code using our Zendro tools (templates)?
Resolver-Template: Extend add_unique_person to invoke disassociate_person_id as shown if and only if the association is single-end one-to-one
DataModel-Template: Introduce the function disassociate_person_id( person_id ) if and only if the association is single-end one-to-one

Possible solutions to the CE conundrum:

  1. There can only be one: Do it for all storage types and DDM or Local DM identical: Resolver fetches associated to be updated - Pro: KISS, Elegant, Con: inefficient

  2. Introduce disassociate_id function that is almost identical to removeid function with the difference, that it does not require the root-model-id argument. Use this function instead of remove_id

  3. Extend remove__id to accept null or undefined as the "root-model-id" (parrot_id in the above example) arg for all storages except DDM and Cassandra. In the latter extend add_unique_person to fetch parrots and invoke remove_person_id with the correct parrot_id's

  4. Almost as 3., but if "root-model-id" is null or undefined AND storage is DDM or Cassandra, do the fetch of the parrot_ids (plural) in the remove_person_id function (iff person_id is null or undef)

Two groups: KISS, but inefficient (1), and Efficient, but complex (2-4)

Final decision on the CE for single-end FK one-to-one associations (SE O2O):
Implement above solution 2. so that the function disassociate__id( assoc_id ) , e.g. Parrato.disassociate_person_id( person_id ) , will only be generated in the case of SE O2O assocs and only in the model that holds the foreign key. But make sure that this information is in the JSDoc comments, i.e.:

  • This function is only generated for SE O2O associations to implement consistency ensurance
  • This function is only present in the model that holds the foreign-key of the assoc

@wunderbarr
Copy link
Member

Asis' suggested solution for DDM:

  1. Use the same approach as in one-to-one read with a limit of 2
  2. If more than one record is found throw an error, informing that the data is inconsistent and suggest manual fixing this problem
  3. Inform also that any updates to fields will have been successful, but the assoc remains unchanged.

keep consistency in behavior would be ideal.
implement the discussed solution from above for all storages.
we could even move this into the resolvers, now that it would be storage agnostic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants