Closed joachimvh closed 8 months ago
Makes sense indeed.
It may not be super easy to implement though, as we have different types of arrays (explicit RDF lists, multiple predicate values).
Having had a look, arrays definitely seem quite feasible. The elements get concatenated immediately through the properties
field of the RDF object. And when you overwrite that field you get the correct result. Similarly for key/value lists, although these seem to potentially have multiple entries in the array where the actual elements can be found deeper using the list
field. But similarly, everything seem to work when replacing values in there.
One issue will potentially be the verbosity when there are several steps that you want to take. E.g., remove element A, insert element B before X, and replace another field of the same object with something else. Preferably 1 Override could do all this at once, so users don't have to create 2-3 Overrides to change 1 object, with these Overrides then also targeting each other as only 1 can target the main resource. Will look into that.
I've had a look at how this could look in practice, taking the above considerations into account. Below is what I'm currently thinking of.
As this would introduce several variations of what you can do with an Override, I'm thinking of deprecating the overrideParameters
field and instead introducing overrideSteps
, which would take an array of steps and execute those in order on the target object. The @type
of that object would then define which action would be taken. Types would be OverrideMapEntry
/OverrideListInsertBefore
/OverrideListInsertAt
/OverrideListRemove
, and OverrideParameters
for the original Override behaviour.
These objects would make use of the parameters overrideParameter
/overrideTarget
/overrideValue
.
overrideParameter
points to the specific parameter that contains the list/map in the object.overrideTarget
determines where the change occurs.
OverrideMapEntry
: the key in the map to update.OverrideListInsertBefore
: the identifier of the resource before which the new value should be inserted.OverrideListInsertAt
: an integer identifying the position where the value should be inserted.OverrideListRemove
: the identifier of the resource which should be removed from the list.overrideValue
would be the new value to insert at that location. For an OverrideMapEntry
this can be omitted to remove the value for that key. This is the only parameter that is necessary for the old OverrideParameters
.This could result in, for example, the following Override:
{
"@id": "ex:myObjectOverride",
"@type": "Override",
"overrideInstance": { "@id": "ex:myClassInstance" },
"overrideSteps": [
{
"@type": "OverrideMapEntry"
"overrideParameter": { "@id": "MyClass:_mapParameter" },
"overrideTarget": "mapKey"
"overrideValue": "newMapValue"
},
{
"@type": "OverrideListInsertBefore",
"overrideParameter": { "@id": "MyClass:_listParameter" },
"overrideTarget": { "@id": "idOfElementAlreadyInList" },
"overrideValue": { "@id": "idOfNewElementToPutInListBeforeTheOtherOne" }
}
]
}
Object ex:myClassInstance
, which is an instance of MyClass
, and at the very least has parameters mapParameter
and listParameter
, which take a key/value object and a list respectively. This Override would change the value for key "mapKey"
in the map with "newMapValue"
, and would insert a new element in its list before the one specified.
This seems like a good way to extend Overrides with these new features while minimizing verbosity and also not introducing too many new keywords. It also allows for easily adding new Override types in the future. One big disadvantage is that you can't use type scoping for the parameters in overrideParameter
, so the uglier version has to be used, but I don't really see a way around this. We could just take a string as input there which is the name how it appears in the code and then determine the rest based on the fact that we know which class the object is, but that seems more hacky.
As mentioned, I would deprecate the overrideParameters
field so there is only 1 way to override, but not remove to prevent breaking existing solutions. So the following example:
{
"@id": "ex:myObjectOverride",
"@type": "Override",
"overrideInstance": { "@id": "ex:myHelloWorldWithOverride" },
"overrideParameters": {
"@type": "ex:GreetingsWorld",
"greetings:say": "GREETINGS WORLD!"
}
}
would become
{
"@id": "ex:myObjectOverride",
"@type": "Override",
"overrideInstance": { "@id": "ex:myHelloWorldWithOverride" },
"overrideSteps": [
{
"@type": "OverrideParameters",
"overrideValue": {
"@type": "ex:GreetingsWorld",
"greetings:say": "GREETINGS WORLD!"
}
}
]
}
All of this assuming that it is feasible to support this, but my experience with the initial Override implementation and the feasibility check I did above makes me think this would work.
I'm also not sure if OverrideListInsertAt
would actually be that useful, I'm mostly adding it because it is possible and perhaps there are some use cases for it.
Your approach sounds very reasonable to me!
Some minor thoughts:
overrideParameters
, I would just keep it around as a shorter form of overriding.OverrideListInsertBefore
, OverrideListInsertAfter
could be useful.OverrideListRemoveAt
may also be useful, perhaps with an optional range parameter? (but then something like OverrideListRemoveFrom
may be a better name...)But I certainly like the fact that we can define different types of overriding with this approach.
- Instead of deprecating
overrideParameters
, I would just keep it around as a shorter form of overriding.
I was planning to just note in the documentation that this still works, but is not really recommended, just because it's a bit inconsistent to have 1 separate solution for this specific type of override. And perhaps even log a warning.
Could also not do that, but I thought it might be confusing if there are 2 ways to do this.
Issue type:
Description:
The idea is to extend the override functionality to somehow allow more flexibility when extending arrays from different configurations. I was thinking of adding another field to an
Override
object that indicates how array elements should be added to an existing array. Features I was thinking of:Not sure yet how feasible it would be to add these as I haven't looked at the array code yet. I will probably look soon at a potential implementation (or decide to give up if the complexity would be too high).