ILIAS
ILIAS copied to clipboard
`Refinery`: Add more transformations (to improve `Refinery` usage in `ILIAS\Setup\Agent::getArrayToConfigTransformation`)
This PR introduces new transformations for the refinery, mostly designed to be usable for the ILIAS Setup:
$refinery->to()->memberOf()
Enum validation: Checks if a value is in a specified set.
Example use case in the config would be: chatroom.log_level
which must be one of emerg
, alert
, crit
, error
, warning
, notice
, info
, debug
or silly
$refinery->executable()
Checks if the given string specifies an executable path.
I could not find a suitable place for this transformation so I placed it on the top level of the refinery.
An example use case would be for config paths like: mediaobject.path_to_ffmpeg
, style.path_to_scss
, virusscanner.path_to_scan
etc.
$refinery->int()->isBetween()
This is just a convenience transformation but IMO very nice to have. There is no actual class for this constraint as it can be composed from other transformations and constraints.
Example use case would be in the config would be: chatroom.deletion_interval.deletion_value
which have both min and max values.
Other changes
DeriveTransformWithProblem
This trait is implemented to prevent boilerplate when implementing new transformations, as some methods are always implemented the same.
ProblemBuilder
No behavior is changed, just some simplifications (with one exception (pun intended): the InvalidArgumentExcpetion
is now a ArgumentCountError
)
Just FYI: Another PR will follow which will introduce deeper changes to the refinery and the relation between Transformations and Constraints but that PR will be independent of the changes made in this PR.
Hi @lscharmer,
as with #6314, please assign back if back if this is ready for the next iteration.
Kind regards!
Hi @klees,
this PR is updated an can be reviewed again. Below are my answers to your review:
- [x] ProblemBuilder: You seem to be fixing some problem with the
ProblemBuilder
, but do not really understand what it is. Or is it just a refactoring? Please split that change into a single commit. If it is a problem: please add unit test, that fail without the changes.
Yes, it is just refactoring. It is now placed in it's own commit.
- [x] DefaultToNull: I also came accross this problem several times. I'm not convinced that "all fields might be null" is the thing that really needs to be expressed. Instead we would want to say: this and that field could be null (and it is fine). Also, we would not only want to say it for the strict
to
variant, but also for the lesskindlyTo
variant. Hence I propose the following implementation: On toplevel, we add a transformationwithDefault($transformation, $default)
that replaces anull
value with$default
and passes non-null values to the$transformation
. We could deriveoptional($transformation)
from there. We could then introcude special cases forWithDefault
in therecordOf
-transformations for theto
andkindlyTo
variant. We could the writerecordOf(["key" => $f->optional(...)]);
or something similar forwithDefault
. Maybe we should discuss this, but I would at least suggest to split this topic from the PR at hand.
I will extract the DefaultToNull
stuff into it's own PR (it is removed from this one). The Transformation was not ment to address the default
/ optional
problem, but I see how this is relevant when one chooses a specific approach on how one wants to pursue to implement such a concept.
We can further discuss the details in that PR (coming soon).
- [x] MemberOf: Although I would personally also prefer
memberOf
, I think we should name this transformationinArray
. This is something most PHP developers will recognize immediately, while "member of" comes from a more mathy background imo.
Done
- [x] Executable: Please rename to
IsExecutable
. We use thishas
/is
naming scheme for constraints consistently, mostly.
Done
Best regards @lscharmer
Hi @lscharmer,
thanks again and sorry that it took me so long to rereview this. I just merged this manually with two rather minor changes.
Kind regards!
Thx @klees for your efforts