[Z3-sqlos] Re: [z3] [Fwd: Re: [Zope3-Users] sqlos - getting factory of NoneType]

[Christian Lück]

Brian Sutherland wrote:
> On Mon, Jan 09, 2006 at 07:52:38PM +0000, Christian Lück wrote:
>> Hi Brian!
>> 
>> Brian Sutherland wrote:
>> > On Sat, Jan 07, 2006 at 06:26:19PM +0000, Christian L�ck wrote:
>> >> Hi
>> >> 
>> >> The problem with sqlos seems: It does only work if ONE AND ONLY ONE
>> >> factory for the content component (interface) in question is registered.
>> >> And this one-and-only factory has to be registered with the
>> >> zcml-directive in a special sqlos namespace, ie.
>> >> 
>> >>   <sqlos:factory
>> >>       id="quotationtool.BookSQLObject"
>> >>       component=".book.Book"
>> >>       title="BookSQLObject"
>> >>       description="A book object for the sql resourcedb."
>> >>       />
>> >> 
>> >> When a second factory is registered for the same interface, ie.
>> >> 
>> >>   <factory
>> >>       id="quotationtool.BookWithInitialValues"
>> >>       component=".book.BookFactory"
>> >>       title="BookWithInitialValues"
>> >>       description="A factory for book object with initial values."
>> >>       />
>> >> 
>> >> sqlos fails to get the objects stored on the database (you get blank
>> >> browser pages).
>> >> The reason seems to be the implementation of
>> >> sqlos.container.SQLObjectContainer, which maps between sql-unique-keys
>> >> (SQLObject-names) and zope-object-names. The mapping mechanism makes use
>> >> of the factory.
>> > 
>> > Yes, I have seen this problem before. You cannot mix sqlos:factory and
>> > factory and expect things to work.
>> > 
>> > And yes, this is a _big_ problem which I think requires a re-think of
>> > how we do this. But that re-think is not yet thunk.
>> > 
>> > So please try this patch (untested). If it works for you, I will add it
>> > to the source.
>> > 
>> > Index: container.py
>> > ===================================================================
>> > --- container.py        (revision 21506)
>> > +++ container.py        (working copy)
>> > @@ -106,7 +106,9 @@
>> >          (name, object) for the objects that appear in the folder.
>> >          """
>> >          for factoryName in self.allowedFactories():
>> > -            factory = zapi.getUtility(IISQLObject, factoryName, context=self)
>> > +            factory = zapi.queryUtility(IISQLObject, factoryName, context=self)
>> > +            if factory is None:
>> > +                continue
>> >              for obj in factory.select():
>> >                  name = '%s.%s' % (factoryName, obj.id)
>> >                  yield (name, contained(obj, parent=self, name=name))
>> > 
>> 
>> No, this does not work for me.
> 
> Why?

Well, as you can see from the debugging in my first email,
getFactoriesFor() does not return me a None type. I have registered a
factory with <zope:factory>. So in the list of returned factories there
is a
<quotationtool.resourcedb.book.BookFactory object at 0xb602236c> object.

The point is that a factory component has a getInterfaces() method, that
returns all interfaces implemented by an object which can be created by
this factory. See zope.component.interfaces.IFactory and p. 69.

That's why allowedFactories() as currently implemented returns a set
union of factories for components quontained in an SQLObjectContainer of
<sqlos:factory> AND <zope:factory>.

If you have the 'Person' example from README.txt as a tar ball, I would
beg you to send it to me. Then we could discuss a common example.

> 
>> The problem with the "None type" was not
>> mine. I was answering to an other email. - I was facing the analgue
>> problem of several factories returned by the allowedFactories() method
>> which caused errors.
>> 
>> Well I have found a solution now. See the diff I have attached.
>> 
>> 1) First of all we define a marker Interface ISQLObjectFactory, see what
>> I added to interfaces.__init__.py.
> 
> The problem I have with a marker interface is that it introduces a
> new concept, increasing the complexity of the code. If we can do the
> same without this concept, and with less lines of code, we should.
> 
> It's a little concept, I know, but still something extra you need to
> understand when reading the code.

Ok, so lets formulate the problem:

We have to get allowedFactories() to know which factory to pick. That
means there has to be a information sticking on the right factory that
makes the difference.
Is it already there? Well, I don't know. Maybe, because of the
registration via the sqlos namespace.  I did not dig deep into
metaconfiguration.

Now lets talk about possible implementations:

An argument for a marker interface would be, that it is a peace of
information that is as much independent from the implementation of the
factory as could be. It's a merely semantic concept. Marking makes a
difference.
(Maybe I like it because I'm a student of linguistics :)

> 
>> 2) Then we implement this marker interface by factories registered by
>> the <sqlos:factory> directive. See what I added to metaconfigure.py.
>> 
>> 3) Then we check for this marker interface in the allowedFactories()
>> method of SQLObjectContainer. Have a look at what I added to container.py
>> Well, in your diff you added the check for "None type" to the items()
>> method. I think one should rather check the factories in the
>> allowedFactories() method already.
> 
> Yes, I agree, but I also know that there are people who use
> allowedFactories in their own code. So just changing the meaning of the
> function is a non flyer.
> 
> So what I would like is to deprecate it and introduce a new function
> getAllowedIISQLObjectUtilities, which would return a generator of tuples
> (utility name, utility) and incorporate the check for None. All users of
> allowedFactories in the sqlos source can then be converted to this
> function.
> 
> So instead of introducing a new concept, we can take away an old one and
> replace it with a new one.
> 

I think that's a very good point. Well, I admit, that I haven't got
enough experience to decide such things.

> Also, perhaps the new function should be private?
> 

Ok. Good!

>> For a) allowedFactories() could be
>> and in fact is called somewhere else and allways only our sqlos:factory
>> is an allowed factory for things relavat in SQLObjectContainer; b) the
>> allowedFactory() method is sqlos specific, it's not a common thing
>> specified in any zope interface.
>> 
>> This way we achieve this:
>> 
>> - sqlos gets more tyres on the standard zope3 highway: We can now
>> register <zope:factory> etc. besides <sqlos:factory> without getting errors.
>> 
>> - Since all changes are sqlos internal sqlos applications keep running
>> without any changes -- provided zcml (the <sqlos:factory> - directive)
>> was used to wire things together.
>> 

I have to correct myself: Even registration via phython works, because
the marker interface is implemented in the class.

>> 
>> 
>> >> This way it works, I see the sql-objects and can even edit them :) - But
>> >> works yust to run into the next bigger problem... (I can't create
>> >> objects due to rollback problems) :((((
>> > 
>> > If you post a traceback here, there is a chance someone can give you a
>> > quick answer.
>> > 
>> 
>> Well, this was due to the incompatibility regarding the z3 standard two
>> step process of adding content. You already read about my solution...
> 
> Yeah, but what I want to know is why the two step process works for me,
> but not for you.
> 

Please send me your Person example.

>> 
>> Kind Regards,
>> Christian
>> 
>> PS. It was the first time I created a diff. I'm not sure if one can use
>> it at all, because of the filenames.
> 
> Thanks for the diff it makes it very clear what you are talking about!
> And normally the filenames are no problem as one can use the -p argument
> to patch. Though it might be easier for you to generate them using 'svn
> diff'.
> 
>> Do you know a good HOWTO, maybe zope specific?
>> 
>> PS2. Maybe we should check for "None type", too.
> 
>> diff -urN sqlos-21844/container.py sqlos-work/container.py
>> --- sqlos-21844/container.py	2006-01-09 18:13:45.000000000 +0000
>> +++ sqlos-work/container.py	2006-01-09 18:20:03.000000000 +0000
>> @@ -29,6 +29,7 @@
>>  from sqlos.interfaces import ISQLObject, ISQLObjectIsolated, IISQLObject
>>  from sqlos.interfaces.container import ISQLObjectContainer
>>  from sqlos.interfaces.container import IIsolatedSQLContainer
>> +from sqlos.interfaces import ISQLObjectFactory
>>  
>>  def contained(obj, parent=None, name=None):
>>      """An implementation of zope.app.container.contained.contained
>> @@ -84,7 +85,9 @@
>>      def allowedFactories(self):
>>          for name, factory in zapi.getFactoriesFor(ISQLObject, context=self):
>>              if checkFactory(self, None, factory):
>> -                yield name
>> +                # check if registered with <sqlos:factory>
>> +                if ISQLObjectFactory.providedBy(factory):
>> +                    yield name
>>  
>>      def keys(self):
>>          """ Return a sequence-like object containing the names
>> diff -urN sqlos-21844/interfaces/__init__.py sqlos-work/interfaces/__init__.py
>> --- sqlos-21844/interfaces/__init__.py	2006-01-09 18:13:45.000000000 +0000
>> +++ sqlos-work/interfaces/__init__.py	2006-01-09 17:44:26.000000000 +0000
>> @@ -335,3 +335,9 @@
>>  
>>      def next():
>>          """ Iterator """
>> +
>> +class ISQLObjectFactory(Interface):
>> +    """This is a marker interface. A Factory which is registered
>> +    by the <sqlos:factory> directive gets marked and can therefore
>> +    be distinguished from other factories for a content component."""
>> +    pass
>> diff -urN sqlos-21844/metaconfigure.py sqlos-work/metaconfigure.py
>> --- sqlos-21844/metaconfigure.py	2006-01-09 18:13:45.000000000 +0000
>> +++ sqlos-work/metaconfigure.py	2006-01-09 17:33:07.000000000 +0000
>> @@ -18,9 +18,10 @@
>>  from zope.component.factory import Factory, IFactory
>>  from sqlos.interfaces import IISQLObject, IConnectionName
>>  from sqlos.interfaces import IReadSQLObjectClass, IWriteSQLObjectClass
>> +from sqlos.interfaces import ISQLObjectFactory
>>  
>>  class SQLObjectFactory(Factory):
>> -    implements(IFactory)
>> +    implements(IFactory, ISQLObjectFactory)
> 
> Actually, we could probably get rid of SQLObjectFactory as well.

How would that look like? Don't we need it to make python/z3 objects
from rdb columns?

> 
>>  
>>  def handler(_context, component, id, title=None, description=None):
>>      if isinstance(component, basestring):
> 
>