[Zope-CMF] Re: GenericSetup improvements

[Florent Guillaume]

Hi Yuppie!

On 29 Dec 2005, at 21:20, yuppie wrote:
> Florent Guillaume wrote:
>> On 27 Dec 2005, at 10:51, yuppie wrote:
>>> Florent Guillaume wrote:
>>>> Also I'd like to change things like:
>>>>     node = property(_exportNode, _importNode)
>>>> into:
>>>>     def __exportNode(self):
>>>>         return self._exportNode()
>>>>     def __importNode(self, *args)
>>>>         self._importNode(*args)
>>>>     node = property(__exportNode, __importNode)
>>>> To allow proper subclass overriding and avoid the dead chicken  
>>>> of having to redefine the node property each time.
>> Done.
>> But in my (complex) setup it revealed an unintended side effect.  
>> There are actually 3 useful methods when doing export (also valid  
>> for import):
>> - export as simple node for its parent file (just the <object  
>> name="foo" meta_type="bar"> part),
>> - export as full node tree,
>> - export as XML body using the "full node" code.
>
> Correct.
>
>> Before, the fact that you defined or not
>>    node = property(_exportNode, _importNode)
>> made the export "do the right thing", as
>>  - _extractObjects calls exporter.node, thus calls the last node  
>> property defined, whereas
>>  - _exportBody calls self._exportNode(), thus inherits
>> This is *very* implicit and magic, and would benefit from some  
>> refactoring...
>
> Well. That's the way property() works. Your change adds magic  
> because it makes property() inherit its methods.

I know it's the way property works, but it's usually (and also by me)  
considered a hindrance, because people are used to see methods  
inherited.

>> The current status of the code is that exports of more that one  
>> level are currently not correct.
>> It seems there's no unit test for it...
>
> There are no real unit tests for it, but there are many unit tests  
> for adapters using those base classes. These are at the same time  
> integration tests for the base classes in GenericSetup.utils. Some  
> CMFCore tests are failing after your change.
>
>> I'll fix this in a few days unless someone is interested to do it  
>> before then.
>
> I don't think it's a good idea to make obviously incomplete  
> checkins to the trunk. Why didn't you create a branch?

At the time I made the proposal I really tought it had no impact. And  
I fucked up my testing due to the numerous sandboxes I have. I've  
reverted it for now.

I'll answer the rest of your email when I've thought more about this  
whole business :)

Florent

-- 
Florent Guillaume, Nuxeo (Paris, France)   Director of R&D
+33 1 40 33 71 59   http://nuxeo.com   fg at nuxeo.com