petehamilton / citier

CITIER (Class Inheritance & Table Inheritance Embeddings for Rails) is a solution for simple Multiple Class Inheritance in Rails.
88 stars 24 forks source link

Auto-Inc keys could get out of alignment #6

Closed petehamilton closed 13 years ago

petehamilton commented 13 years ago

Example Scenario

Assume that Entity and User have auto incremented id fields Entity - root User - child of entity Administrator - child of entity

Add a user, Entity id+1 = 1, User id +1 = 1 Add an administrator, Entity id+1 = 2, Administrator id+1 = 1

The id's are now out of sync and so cannot be traversed back up the tree.

Current Fix/Method

Currently this has been fixed by forcing the child to take the parent's ID on save. Since a child always requires a prent, and a parent can have many children, the parent auto ID will always be > child auto ID. As a result, there will never be conflicts. However, this is not completely optimal as the child ID can sometimes jump, not causing errors, but meaning in the child table the id's are not always in sequence (1, 2, skip, 5 not 1, 2, 3).

Desired (better) solution

My long term plan for this is to have a much nicer implementation where instead of using 'type' you simply have a parent id for child entities. This is how I have implemented it in python (pylons, django) and PHP before and it feels a lot nicer and like a more rounded and complete implementation.

e.g:

class Entity < ActiveRecord::Base
  acts_as_citier
  validates_presence_of :name
end
class User < Entity
  #Somehow citier would also create an entity_id field as well during the 'User' migration.
  #Since it will definitely be needed if doing this style of inheritance, the user shouldn't have 
  # to handle this themselves, citier *should* do it for them. Will investigate further after exams.

  acts_as_citier
  validates_presence_of :title
end

This would mean that when selecting all entities, the parent id could be used to link back through the hierarchies.

Potential Issues

At the moment, citier is really nice for determining classes on the fly. For example, if I have 6 types of notification all with slightly unique fields, I could still do Notification.all() and would get a list of all notifications, regardless of type, but still the correct class and with their own unique fields. It does this by examining the 'type' of the table of the root entity, and proceeding to define the class for the entry accordingly.

Using the id method, there is no way of knowing what class an entry is from a parent. User.all() would be fine, but Entities.all() could encounter issues.

Solutions to these

  1. Keep the type field, after all, it is only one field and while not needed when traversing up the hierarchy, it's really nice to be able to traverse down and this is a simple way of allowing it.
  2. Query tables of all children and see which one's have the root entry as a parent. Then for each of those which are prents themselves, repeat recursively, then when you get to a point where the table you are looking at is a child with no children itself, look at the table name and create an instance of the related class using the attributes you have collected while traversing the relationships.
  3. Another way would be to keep a view for the root class (currently not done) which does something similar to 1, displaying the table name of the child class alongside the entries, this wouldn't entail a field for the table, but would still require a lot of queries for investigating where a hierarchy stops (finding final children).

    Conclusion

Personally I am inclined to leave the type field in and force the ID down the hierarchy dwn the chain for now, while it doesn't seem to make sense if all you want to do is do User.all(), if you want to be able to process all children of a particular model together (different user types for instance) it is very useful.

If anyone has any alternatives, I would love to hear them. If I implement a nicer method in future, I will just allow auto-increment to take over again alongside the fix and you will be sorted from then on.

Hope that made sense :)

Pete

petehamilton commented 13 years ago

I fixed this in V0.1.11 by forcing a child to take it's parent's ID. This is not optimal and I have updated this ticket, but it means people can continue to develop (with auto incrementation on id fields) and when a better implementation comes along there will be no adverse effects.