Cloner replacement strategies

Related issues 

MGNLUI-3431 - Getting issue details... STATUS

MAGNOLIA-6693 - Getting issue details... STATUS

Context

There are use case when a dynamically modified copy of a definition is needed (e.g. Choose/Move dialog definitions which a by default derived from a sub-app descriptor + some other cases). In order to obtain such a copy we used to use the Cloner library which is able to provide deep recursive copies of Java beans. Problems started to arise when definitions stopped being mere POJO under the hood: i18n'ized definitions are enhanced with CGLIB and => contain some meta information internally, which may contain circular cross-references causing cloner to go into an infinite loop. 

Another problem imposed by the same cases - in order to mutate the clone object some setters are required. Normally definition interfaces do not provide any setters, but the default implementations (so-called Configured* classes) do and => we cast an instance of an interface to them. Problem here is that such casts never were safe and become more and more dangerous: definitions are now often not a pre-configured bean, but something else (like a proxy again).

So far two approaches to overcome these two connected issues have been considered. Both of them involve wrapping the source definition instead of cloning. The first approach involves generation of dynamic mutable wrappers (via CGLIB), the other one aims to avoid proxies for the sake of simplicity by means of the explicit wrappers with mutation capabilities.

Proxy-based approach

PRs: https://git.magnolia-cms.com/projects/PLATFORM/repos/main/pull-requests/206/overview  https://git.magnolia-cms.com/projects/PLATFORM/repos/ui/pull-requests/150/commits

public interface Mutable<T> {
    T getObject();
    void setProperty(String propertyName, Object value);
}

Proxy-based solution consists of the two parts: 

• MutableWrapper utility - takes an instance and creates a proxy of the same target type. Proxy internally has a simple cache of modified property values. The setter invocations contribute entries to the cache, getter calls are first addressed to the cache, if a miss occurs - then a getter is delegated to the source object and the result is also wrapped into a mutable wrapper and stored in cache. Besides that MutableWrapper injects Mutable<T> interface implementation into the proxy class, allowing for 'dynamic setter' invocations which opens the door for easy mutator implementations.
• Mutator objects take an instance of Mutable and are able to provide simple typesafe setter API's based on the generic Mutable#setProperty() API.

The following snippet provides an example of how it is possible to expose a simple setter API for the FieldDefinition:

public class FieldDefinitionMutator extends Mutator<FieldDefinition> {
 
	// we assume here that fieldDefinition is pumped-up with MutableWrapper and implements Mutable interface
    public static FieldDefinitionMutator accessMutable(FieldDefinition fieldDefinition) {
        return new FieldDefinitionMutator(fieldDefinition);
    }

    FieldDefinitionMutator(FieldDefinition fieldDefinition) {
        super(fieldDefinition);
    }

	// with setProperty API - we do not have to specify the fields in the mutator impls, avoiding some boilerplate
    public void setReadOnly(boolean isReadOnly) {
        setProperty("readOnly", isReadOnly);
    }
}

The following example takes a form definition and sets the readOnly property to true for all the fields in all of the form's tabs (w/out modifying the source definition of course):

// wrap form definition into a proxy, all the sub-defs will also be wrapped on demand
final FormDefinition formDefinitionClone = MutableWrapper.wrap(formDefinition);

for (TabDefinition tab : formDefinitionClone.getTabs()) {
    for (FieldDefinition field : tab.getFields()) {
		// mutate a field sub-definition via the dedicated mutator API
		FieldDefinitionMutator.accessMutable(field).setReadOnly(true);
		//Mutable.from(field).setProperty("readOnly", true); 
    }
}
...

Pros:

• Wrapping and mutating the objects is really easy, almost no boilerplate code needed.
• Generated proxies have exactly the same type as the source object, so the potential (bad) situations when a type check, which would pass on the source, is done on the wrapped object - would pass as well.
• When it is needed to modify a sub-definition somewhere deep in the tree, developer wouldn't have to also provide wrappers for all of the parent sub-definitions since MutableWrapper does that automatically (see 'Explicit wrapper usage in Java 7 style' for issue demonstration).
• All the magic happens under the hood, almost no understanding from the dev point of view is needed (maybe some basic insight required to create a mutator, but still it should be really simple to grasp)
  
  

Cons:

• Proxy magic involved (CGLIB) (though operations involved are very infrequent compared to our existing use-cases of proxies)
• A bit conventional mutator API implementation (properties are bound via strings, which should be fairly easy to cover with component/unit tests)

Explicit wrappers

PR: https://git.magnolia-cms.com/projects/PLATFORM/repos/ui/pull-requests/154/overview

This approach aims to do the same as discussed above but without usage of proxies. The crux of this effort is to provide the explicitly specified wrapper classes for definitions which would allow to either delegate the getter calls to the source object or provide something custom (overriden value). Let us consider a sample wrapper class:

public class FieldDefinitionWrapper {
	// factory method 
    public static FieldDefinitionWrapper fieldWrapper(FieldDefinition fieldDefinition) {
        return new FieldDefinitionWrapper(fieldDefinition);
    }

	// wrapped instance
    private final FieldDefinition fieldDefinition;

	// For Java 7 - that'd be just a function
    private UnaryOperator<Boolean> isReadOnlyModification = identity();

    FieldDefinitionWrapper(FieldDefinition fieldDefinition) {
        this.fieldDefinition = fieldDefinition;
    }

	// It would be possible to provide modification operators in a functional style.
    // Tracking modifications via functions also solves the default value ambiguity problem (what value would indicate that boolean 'isReadOnly' was not modified).
    public FieldDefinitionWrapper modifyReadOnly(UnaryOperator<Boolean> isReadOnlyModification) {
        this.isReadOnlyModification = isReadOnlyModification;
        return this;
    }

	// It would be possible to provide explicit overriding values
    public FieldDefinitionWrapper modifyReadOnly(boolean isReadOnly) {
        this.isReadOnlyModification = readOnly -> isReadOnly;
        return this;
    }

    public FieldDefinition getField() {
		// We override a default delegator implementation: for some properties we use an explicitly provided value (or fallback to the delegate property)
        return new WrapperImpl(fieldDefinition) {
            @Override
            public boolean isReadOnly() {
                return isReadOnlyModification.apply(super.isReadOnly());
            }
        };
    }

	// Base implementation of a delegating FieldDefinition. @lombok.experimental.Delegate annotations forces delegate methods to be generated,
    // this later allows us to expose only part of definition API for 'mutability' (the object in #getField() already extends a class, not an interface).
    private static class WrapperImpl implements FieldDefinition {

        public WrapperImpl(FieldDefinition delegate) {
            this.fieldDefinition = delegate;
        }

        @Delegate
        private FieldDefinition fieldDefinition;
    }
}

Same experiment as above: DetailsPresenter#cloneFormDefinitionReadOnly

final List<TabDefinition> tabs = formDefinition.getTabs();
final List<TabDefinition> modifiedTabs = new ArrayList<>(tabs.size());
for (final TabDefinition tab : tabs) {
    final List<FieldDefinition> fields = tab.getFields();
    final List<FieldDefinition> modifiedFields = new ArrayList<>(fields.size());
    for (final FieldDefinition field : fields) {
        modifiedFields.add(FieldDefinitionWrapper.fieldWrapper(field).modifyReadOnly(true).getField());
    }
    modifiedTabs.add(TabDefinitionWrapper.tabWrapper(tab).modifyFields(modifiedFields).getTab());
}

final FormDefinition formDefinitionClone = FormDefinitionWrapper.formWrapper(formDefinition).modifyTabs(modifiedTabs).getForm();
...

This is how the same could look in Java 8 - instead of the cycles we could use Stream API

...
formWrapper(formDefinition)
  .modifyTabs(
	 formDefinition.getTabs().stream()
		.map(tab -> tabWrapper(tab)
        .modifyFields(
        	tab.getFields().stream()
              .map(field -> fieldWrapper(field)
              .modifyReadOnly(true).getField())
            .collect(toList()))
         .getTab()
      ).collect(toList()))
.getForm();
...

• Explicitness, no magic
• Since there are not so many use cases - maybe it'd do a trick?

cons:

• Lot's of boilerplate. For every definition type which we would want to expose for wrapping/mutation we'd have to provide the following:
  • delegating class (fairly simple with @Delegate annotation)
  • 'mutable' class which uses an explicitly set value or falls back to delegation
  • fields for the mutable properties (some default values indicating that value setting actually happened might be a problem, but not if mods are stored as functions though)
• When it is needed to modify a sub-definition somewhere deep in the tree, it is a developer's job to take care of the parents' wrapping (hence the bulkiness of the snippets above)
• All of the modifications have to be resolved before the actual wrapper is created.
• The runtime type of the wrapped/cloned definition is different from the source. This ideally should not be a problem, but might be. At least should some sub-type need a wrapper - the whole boilerplate would have to be repeated. One example - we need to get a slightly modified version of ContentConnector (for the choose dialog) which is often cast to JcrContentConnector.