Thanks for checking this Aled, I'll create the transformers. Svet.
> On 20.02.2015 г., at 22:55, Aled Sage <[email protected]> wrote: > > Svet, Sam, > > I've tested the backwards compatibility, and it works with your sub-classing > approach, along with PR#38 which has just been merged [1]. > > I was wrong about the errors when assigning to deserialized fields etc. > The deserialized objects are instances of the old class name, which is a > sub-type of the new class. These objects are thus usable everywhere - as > return values, as parameters etc. > > Longer term, we do need a transformer [2,3] so that we can delete the old > classes. > > Aled > > [1] https://github.com/brooklyncentral/advanced-networking/pull/38 > <https://github.com/brooklyncentral/advanced-networking/pull/38> > [2] > https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/CompoundTransformerLoaderTest.java > > <https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/CompoundTransformerLoaderTest.java> > [3] > https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/impl/XsltTransformerTest.java#L67-95 > > <https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/impl/XsltTransformerTest.java#L67-95> > > > On 06/02/2015 22:17, Aled Sage wrote: >> Hi all, >> >> Svet + Sam have been doing great work on Brooklyn (including >> advanced-networking and clocker) to make it work better with OSGi [1]. >> >> This includes ensuring that packages have different names in different OSGi >> bundles (very important to know if you're thinking about package names in >> the future!). >> >> In downstream projects that depend on advanced-networking, you may see >> compilation errors when the import is for PortForwarder in the old package >> (e.g. brooklyn.networking.subnet.PortForwarder instead of >> brooklyn.networking.common.subnet.PortForwarder). >> >> --- >> Svet, Sam, >> >> We should discuss backwards compatibility some more. >> >> You've renamed the classes, and created sub-classes with the old names - the >> aim being to support deserialization of previously persisted state. >> >> However, various fields and method parameters now expect the new class. >> Therefore the deserialized old class will likely not be usable (e.g. errors >> when trying to assign the deserialized value to a field, or >> ClassCastExceptions later). >> >> That's my opinion; I haven't actually tried to produce the error from >> previously-serialized state. >> So first, do you agree? >> >> I see three options for downstream projects (where PortForwarder instances >> are persisted): >> >> 1. Write a transformer that will upgrade the persisted state, switching >> the old package for the new package. >> (That is what the transformer stuff is designed for [2,3]). >> 2. Be very careful about the types of fields, parameters and return >> types so that the old class is likely to work. >> (sounds fiddly to get right; and without doing (1) we are left with >> that code until the old entities die a natural death!). >> 3. Tell them it's not backwards compatible. >> (That's a last resort, and is unacceptable for anyone running in >> production.) >> >> Of these, I favour (1). >> >> If you agree, then we need to delete the shims (i.e. the classes with the >> old names) and write a transformer. And write better docs describing how to >> use the transformer. >> >> Thoughts? >> >> Aled >> >> [1] https://github.com/brooklyncentral/advanced-networking/pull/21 >> [2] >> https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/CompoundTransformerLoaderTest.java >> [3] >> https://github.com/apache/incubator-brooklyn/blob/master/core/src/test/java/brooklyn/entity/rebind/transformer/impl/XsltTransformerTest.java#L67-95
