Hi Fred!

On 07/01/2008 10:50 PM Frederik Holljen wrote:

> First of all, great work on the requirements and design of Persistent
> Object 1.5. It sure has come a long way since 1.0 :)
> I started a new thread, the other one was becoming a bit crowded..

> Here are the comments from the Arab jury:

Great, thanks a lot! :)

>> The Identity Map support should be optional to not break BC and keep
>> flexibility. Therefore, a new class named ezcPersistentIdentityMapSession is
>> implemented. This extends the current implementation ezcPersistentSession to
>> make instanceof checks still work.

> Using a decorator here sounds like a good idea to me. So you will
> create your session something like this:
> $session = new ezcPersistentIdentitySession( new
> ezcPersistentSession() ); right? And you will create an interface that
> declares all the ezcPersistentSession methods (maybe except the ones
> that shoul not be callable from the identity session).

Exactly. :)

>> ------
>> Design
>> ------
>>
>> The Identity Map support should be optional to not break BC and keep
>> flexibility. Therefore, a new class named ezcPersistentIdentityMapSession is
>> implemented. This extends the current implementation ezcPersistentSession to
>> make instanceof checks still work. All of the methods will be overwritten and
>> new handler classes, extending the existing ones will be implemented to 
>> reflect
>> the actual additions. The methods that are not supported by the new class 
>> will
>> be declared private and throw an exception is used externally.

> You can't do this, you can only give more permissions, not less. I
> suggest making a real decorator out of the session and not declaring
> the "difficult" methods in the interface.

Ah, good that you detected that one. I actually missed to re-phrase this
paragraph. As seen below, the idea was not to purge the cache.

>> The use of the \*FromQuery() methods will therefore result in a complete 
>> purge
>> of the identity map to ensure consistency.

> Is it not possible to use the WHERE parts of those queries to create a
> select that will return the affected objects? It will be slower since
> it uses an extra query of course.

It might be possible to wrap a SELECT around the UPDATE/DELETE queries,
that returns the affected rows. Not sure here, but I will try to figure
out. Nice idea!

However, we might run into problems when people use custom joins here,
but that should be just a documentation issue.

> Later in the doc you note that the $refetch is dangerous. Isn't
> purging here in fact exactly the same?

Yes, it is. If $refetch is turned on or the cache is purged, the user
needs to take care that the identities - of which he still holds
references to - get unset properly.

I don't see a real choice here, do you? The problem is, that people can
use find() queries to read objects from the DB that might already be
loaded. In this case you need to decide if the loaded objects should be
replaced with their existing identity (normal case) or if you want them
to be reloaded. The latter case is usefull in case you want to update
multiple objects i case the database might have changed. This is quite
cumbersome with the refresh() method, since it uses single SELECT queries.

I think a solution here might be to replace just the values of existing
identities, in case $refetch is true. This might be a bit slower and
propably requires us to make some more parts of ezcPersistentSession as
public (and document them as protected of course).

Another problem is the refetching of relations here. The second part of
the design describes how pre-fetching of related objects will work. This
makes use of the identity map to store the results of the pre-fetch and
return them if getRelatedObjects() is called. However, people might want
to limit the pre-fetched result set using a WHERE condition. Then the
getRelatedObjects() method will not return the result set it would
usually have returned, but a limited one. In case the user wants to
fetch the original result set later, he needs the re-fetching mechanism.

>> ezcPersistentIdentityMap
>> ========================
>>
>> This class is the internal heart of the identity map enhancement. It handles
>> all caching and mapping activities globally for a session.
>>
>> Users shall usually not access the identity map object directly to fetch 
>> cached
>> objects or add new ones. On the other hand, the identity map should be
>> replaceable to allow more advanced implementations (e.g. involving the Cache
>> component). Therefore most methods of this class will public, but marked as
>> proteced via documentation.

> What does marking them protected gain you?

Preventing users from doing really dangerous stuff (adding objects
manually to the map, etc.). They still can do, but they notice that they
are not supposed to.

>> - It makes sense to use a decorated select query object to avoid the need for
>>  users to submit the $relations array twice: Once to the createFindQuery()
>>  method and once to the find() method. In ezcPersistentSession, it is
>>  currently required to submit the class name twice, which is redundant 
>> anyway.
>>  An idea would be to provide a generally decorated ezcQuerySelect class for
>>  PersistentObject, which would also orphan this redundant submission.

> How would you do that? ezcQuerySelect has subclasses itself?

> I'm a bit concerned about performance using this system. With _many_
> objects, sorting out the relations etc. could be very time consuming,
> and I'm slightly wondering how many objects this identity map system
> can support (especially with deep relations). I think it would be a
> good idea to test that thoroughly and post the results. If not people
> could get an unpleasant surprise.

I don't think the performance drawback will be that hard. I imagine
something like

ezcPersistentFindQuery
{
    protected $selectQuery;

    public function __construct( ezcQuerySelect $select )
    {
        $this->selectQuery = $select;
    }

    public function __call( ... )
    {
        // Dispatch to $this->selectQuery
    }

    public function __get( ... )
    {
        // Dispatch to $this->selectQuery
    }
    // ... __set(), __isset(), __unset(),...
}

The class code itself will be quite slim, so that parsing and loading
does not take a huge bit of time. I agree with you that we need to check
how much impact the additional __*() round trip takes. I'll do some
tests on that later on.

Thanks a lot for your input, Fred. It was, as always, very refreshing to
read your view point. :)

Cheers,
Toby
-- 
Mit freundlichen Grüßen / Med vennlig hilsen / With kind regards

Tobias Schlitt (GPG: 0xC462BC14) eZ Components Developer

[EMAIL PROTECTED] | eZ Systems AS | ez.no
-- 
Components mailing list
Components@lists.ez.no
http://lists.ez.no/mailman/listinfo/components

Reply via email to