InventoryEntry attributes

[Jan Hudec]

On Tue, May 09, 2006 at 10:10:23 +1000, Robert Collins wrote:
> On Tue, 2006-05-09 at 00:24 +0200, Jan Hudec wrote:
> > Hello,
> > 
> > I already reported on the way InventoryLink.symlink_target is used in
> > InventoryEntry and works because the other entries don't have __slots__. Now
> > I've found a related issue.
> > 
> > There is a test, test_inv.TestInventoryEntry.test_dir_detect_changes, that
> > reads:
> > 
> >     def test_dir_detect_changes(self):
> >         left = inventory.InventoryDirectory('123', 'hello.c', ROOT_ID)
> >         left.text_sha1 = 123
> >         left.executable = True
> >         left.symlink_target='foo'..
> 
> > In other words, it sets 3 parameters that InventoryDirectory should not have
> > at all (which works because InventoryEntry does not declare __slots__, so it
> > can have attributes added) and checks whether they are ignored.
> > 
> > It also happens further down the line where InventoryLink entry get's it's
> > executable flag set (and is tested whether it ignores it).
> > 
> > But shouldn't it rather be an error to set these attributes?
> 
> The code that uses InventoryEntry was completely unprepared for
> subclasses, but could not be cleaned until there were subclasses.... The
> test tests that the API used by the calling code was preserved intact
> during the introduction of subclasses. The migration probably is not
> complete : there is possibly still code that assumes any InventoryEntry
> is of the base type and can handle any attribute. 
> 
> The test should be removed after an audit that all uses of
> InventoryEntry either create a subclass directory, or are given an
> instance from somewhere else and only use subclass specific features
> indirectly - either invoking a method on the base class that is
> overridden, or by doing a (yeuch) type code check.

I'd think doing:

	assert self.__class__ != InventoryEntry

in InventoryEntry.__init__() would check nothing creates raw
InventoryEntry objects pretty well and then declaring __slots__ in all
the subclasses would check noone sets inapropriate attributes.

-- 
						 Jan 'Bulb' Hudec <bulb at ucw.cz>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060509/b1c01a6c/attachment.pgp