I found some flaws in the entity engine while working on the entity
cache bugs.
The main problem is with the GenericEntity class and its subclasses -
GenericPK and GenericValue. When you get a GenericPK instance from
GenericEntity or GenericValue, a new GenericPK instance is created and
initialized with values from the GenericEntity (GenericValue) instance.
There is no "connection" from the new GenericPK instance to the
GenericEntity that created it. Then you use the GenericPK instance in
some entity engine method calls. Some of those method calls might cast
GenericPK to GenericEntity. In either case, the methods treat the
GenericPK as if it was the same instance as the GenericEntity instance
that created it - but it isn't, it's a copy with no connection to the
GenericEntity instance that created it. Consequently, changes intended
for the original GenericEntity instance are never made.
I can't see a reason for having the two GenericEntity subclasses.
GenericPK is a GenericEntity with a subset of the entity's fields (the
PK fields), otherwise the two are identical. GenericValue is a
GenericEntity with a few convenience methods and a copy of the original
field values. From my perspective, it would be better if GenericEntity
included all of GenericValue's features (I can't see why it shouldn't) -
eliminating the need for GenericValue.
So, the entity engine API is a bit muddled in this area and it is
causing some implementation problems.
Another problem I found was in the
AbstractEntityConditionCache.storeHook method(s). This method attempts
to perform "smart" cache clearing by analyzing all of the cached
conditions to see if they match the GenericEntity (or one of its
subclasses) that is being operated on. Any condition matches are cleared
from the cache. From my perspective, the overhead caused by all of this
analysis cancels any benefit from performing fine-grained cache clearing
- but let's assume I'm wrong and carry on...
If you paid close attention to the previous paragraphs, you should be
able to see immediately why these methods don't work. If I pass a
GenericPK instance, it isn't going to match the cached conditions
because it contains a subset of the fields contained in the conditions.
If I pass a GenericValue instance, it compares the *current* field
values with the cached conditions. Oops, they won't match if any of the
GenericValue fields were modified. It would be better to compare the
*original* field values (GenericValue has them) with the cached
conditions. The code could be modified to fix that problem but that
doesn't help GenericEntity - which doesn't contain a copy of the
original field values.
In revision 1476296 I modified GenericDelegator to always clear the
cache *after* the entity operation - so the cache will be loaded with
the changed values. With that revision, comparing GenericValue to the
cached conditions will always fail - because its original values are
lost. I bypassed using the AbstractEntityConditionCache.storeHook method
to work around that.
So, there needs to be some entity engine API changes. I have some ideas,
but I will wait for comments before proceeding further.
-Adrian
- Entity engine design/implementation problems Adrian Crum
-
