Add deep clone to OAS 3.0 models

[arturdzm]

Implement clone() method for OpenAPI model and children

[cbornet]

I need this so I'll do a PR if you're OK. What do you prefer : a clone method (implementing Cloneable I guess) or a copy constructor ?

[frantuma]

Thanks! I would go for a copy constructor

[cbornet]

Just to be sure : are the models generated from the OAI spec or written by hand ?

[cbornet]

Also, some properties are Object types so can't easily be cloned (eg. extensions). What should be the behavior for these ones ? Ignore and not copy ? Copy the reference (with the problem that modifying the clone will modify the original...) ? Check for some known types (String, JsonNode, Cloneable, ...) that will be copied while unknowned are ignored ? The problem of detecting JsonNode is that it would make Jackson a dependency of swagger-models which is probably not desirable.

[cbornet]

So I have this sample code in swagger models

public class Example {
    private String summary = null;
    private String description = null;
    private Object value = null;
    private String externalValue = null;
    private String $ref = null;
    private java.util.Map<String, Object> extensions = null;

    public Example() {
    }

    public Example(Example example) {
        this(example, ObjectCopyUtil::copy);
    }

    public Example(Example example, Function<Object, Object> objectCopy) {
        this.summary = example.getSummary();
        this.description = example.getDescription();
        this.value = objectCopy.apply(example.getValue();
        this.externalValue = example.getExternalValue();
        this.$ref = example.get$ref();
    }

with ObjectCopyUtil

public abstract class ObjectCopyUtil {

    public static Object copy(Object o) {
        // Immutable types
        if (o instanceof String || o instanceof UUID || o instanceof Enum || o instanceof Boolean || o instanceof Integer
                || o instanceof BigDecimal || o instanceof Float || o instanceof Double) {
            return o;
        }
        // Mutable types
        else if (o instanceof Date) {
            return ((Date) o).clone();
        }
        else if (o instanceof byte[]) {
            return ((byte[]) o).clone();
        }
        return null;
    }

This way swagger-models doesn't depend on Jackson. You can provide your own copy function that will copy other objects such as JsonNode (such a function could be added as utility to swagger-core). But it makes it rely on on Java 8 (whereas I believe the current works with Java7). And also users may be confused by the fact that JsonNode objects are not copied. So maybe it would be better to have these copy functions not in the models but as utility in swagger-core ? WDYT ?

[frantuma]

Thanks for your effort in this area, which I believe is not really trivial. About your question, models are written by hand.

About deep copy strategies, possibly this is not so easy to address consistently, may I also ask in which scenarios you would need this functionality?

Atm, when we need a deep copy we are relying on Jackson serialization/deserialization to achieve that, e.g. here. This is done outside swagger-models module and supports any "jackson serializable" class; it uses however Jackson (and its reflection), and is probably not so efficient.

A copy constructor sounded like an improvement, but as you pointed out it doesn't on its own address e.g. unknown/polymorphic members, where we would need to apply some kind of reflection and/or allow to externally define cloner functions. This is getting possibly too complicated and it probably would make sense to implement any improvement in this area in a more "functional" (brr...) way, by keeping the logic fully outside of swagger-models (therefore no copy constructor, but as functions alternative to the current Jackson based serialization/deserialization and to other existing cloning libs like e.g. dozer/kryo etc.

Don't know if this matches what you mentioned in So maybe it would be better to have these copy functions not in the models but as utility in swagger-core, it might be even better not to implement this within swagger-core, as it possibly highly depends on user scenarios, and we don't seem to get many requests in this area. Please let me know what you think

[cbornet]

Yes it's probably better to have it outside of swagger-models ATM. I think it could be useful to have it in swagger-core as, as you said, there are some places where swagger-core would benefit from copy methods instead of relying on Jackson ser/deser. The copy method could look if the Object instance is of a known type and if not fallback to Jackson ser/deser.

[frantuma]

yes this probably makes sense; similarly to what you mentioned above, IMO the approach would include a mechanism for alternative pluggable implementations of deepCopy (e.g. implemented with "custom" full reflection, known types, external lib, jackson/xstream ser/deser, etc), also used internally in swagger-core when a deep copy is needed.

[DrSatyr]

Hi there, any updates on this topic? Seems @cbornet provides implementation through CopyUtils. Looks good and quite helpful from my perspective. Any chance to merge it?