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