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

[Brian Sutherland]

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?

> 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.

> 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.

Also, perhaps the new function should be private?

> 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.
> 
> 
> 
> >> 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.

> 
> 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.

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


-- 
Brian Sutherland

Metropolis - "it's the first movie with a robot. And she's a woman.
              And she's EVIL!!"