Allow to merge models

[GuilhemN]

This PR would allow to merge models with two different strategies:

• PREFER_ORIGINAL which would keep the original fields except if they aren't defined yet.
• PREFER_EXTERNAL which would use the provided fields when they are available.

This is useful when you want to merge different documentations or in my case to merge the output of different libraries generating the doc of an app.

[gossi]

I like having a merge function, even on multiple objects and to make it traverse down hence I dislike it's replacing the parsing method. I'd rather like to keep it as two concepts: parse for everything that comes from json and merge which takes objects.

The strategy would (at the moment) come down to an $overwrite parameter with either true or false which would make the API easier to use:

$info->merge($anotherInfo, true);

And then in Info::merge it would be:

public function merge(Info $info, $overwrite = false) {
    $this->url = ($overwrite && !Text::create($info->getUrl())->isEmpty()) || empty($this->url) ? $info->getUrl() : $this->url;
    // ...
}

well, the "getting the value part" could be extracted into its own method which takes 3 arguments:
$original, $external and $overwrite, and then you call it: <method-name>($this->url, $info->getUrl(), $overwrite). Dunno, maybe getMergeValue() as method name.

Your comments on that?

[GuilhemN]

@gossi I would just replace the function signature by public function merge(static $object, $overwrite = false) so that it can be abstracted and I would also use something stricter than an empty check such as null !== so that we can be sure that a field wasn't already defined.
Otherwise it looks good to me 👍

Do you want me to work on that ?

[gossi]

Great. Yes, I can merge it by monday and make a release, I will be on holidays afterwards.

AFAIK: empty() also checks for !== null (and Text::isEmpty() is using empty())

[GuilhemN]

@gossi i know but if a value isn't exactly null it was defined by the user so if you use empty you can't be certain that a value that shouldn't be overriden always isn't.

I'm especially thinking about integer values that could be 0 but shouldn't be overriden.

[gossi]

Ah, yes; You are right, good one.

[GuilhemN]

@gossi After all, I'm not so sure it's a good idea to merge objects as it causes many reference problems. It's much easier to merge an array in an object.

Edit: always instantiating new objects should fix this.

Edit 2: I pushed some changes, this is a bit long to do...

[gossi]

You should probably do something like:

$this->exclusiveMaximum = $data->get('exclusiveMaximum', false);

anyway the default value would be null and as such not bool.

[GuilhemN]

Yes but if you set false in your model, you don't know if the user already defined that value or not whereas with null :)

[gossi]

The code so far looks good to me. I made a comment on one line, where you replaced some old code (Map::get() didn't had a default value as second param ever since) with some better one.

Apart form that, are your original concerns about objects resolved?

[GuilhemN]

@gossi I don't think i'll have the time to finish this soon :/ This is much longer than what i first thought...

[gossi]

easy. I would have a released a version for you before my holidays. But you can still use your own branch, so no worries.