-
-
Notifications
You must be signed in to change notification settings - Fork 6
FEATURE: Add additional data-types array, float, dateTime #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature/extensible-metadata-properties
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,13 +4,20 @@ | |
|
|
||
| namespace Neos\MetaData\Domain\Dto; | ||
|
|
||
| use DateTimeImmutable; | ||
| use DateTimeInterface; | ||
| use InvalidArgumentException; | ||
| use JsonException; | ||
| use Throwable; | ||
|
|
||
| /** | ||
| * Type of a custom asset metadata property. | ||
| * | ||
| * Values are stored as strings, so this is also what turns a value into its stored representation and | ||
| * back: {@see self::coerceForStorage()} on the way in, {@see self::fromStoredValue()} on the way out. | ||
| * The concrete PHP type of a value is case-dependent - see the private `to*()` helpers below for what | ||
| * each case accepts and returns. Callers elsewhere in the domain therefore type a logical value as | ||
| * `mixed` rather than repeating a union of every case's type. | ||
| * | ||
| * The two directions are deliberately not equally strict. Writing rejects what it cannot interpret, | ||
| * because a caller passing "abc" for an integer property has made a mistake that should not be | ||
|
|
@@ -22,57 +29,86 @@ enum MetaDataPropertyType | |
| case string; | ||
| case integer; | ||
| case boolean; | ||
| case float; | ||
| case array; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we really need to support arrays? if so I think that we should limit it to |
||
| case dateTime; | ||
|
|
||
| /** | ||
| * The given value in the representation it is stored as. | ||
| * | ||
| * Unambiguous conversions are applied, so that callers which only ever have strings – the command | ||
| * line, form input, Fusion – do not have to cast: "42" is a valid integer, "true", "on" and "yes" | ||
| * are a valid boolean, as are their negative counterparts. Anything else is rejected. | ||
| * are a valid boolean, as are their negative counterparts, a JSON encoded string is a valid array | ||
| * and an ISO 8601 string is a valid date and time. Anything else is rejected. | ||
| * | ||
| * @throws InvalidArgumentException if the value cannot be interpreted as this type | ||
| */ | ||
| public function coerceForStorage(string|int|bool $value): string | ||
| public function coerceForStorage(mixed $value): string | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't |
||
| { | ||
| $coerced = $this->tryCoerce($value); | ||
| if ($coerced === null) { | ||
| throw new InvalidArgumentException(sprintf('Value %s cannot be interpreted as %s', json_encode($value), $this->name), 1785715201); | ||
| } | ||
| return is_bool($coerced) ? ($coerced ? '1' : '0') : (string)$coerced; | ||
| return match (true) { | ||
| is_bool($coerced) => $coerced ? '1' : '0', | ||
| is_array($coerced) => json_encode($coerced, JSON_THROW_ON_ERROR), | ||
| $coerced instanceof DateTimeInterface => $coerced->format(DATE_ATOM), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we agree that DATE_ATOM is the right way to represent datetime always? |
||
| default => (string)$coerced, | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * The given stored value as this type, or NULL if it cannot be interpreted as one | ||
| */ | ||
| public function fromStoredValue(string|int|bool $value): string|int|bool|null | ||
| public function fromStoredValue(string $value): mixed | ||
| { | ||
| return $this->tryCoerce($value); | ||
| } | ||
|
|
||
| // ----------------------- | ||
|
|
||
| private function tryCoerce(string|int|bool $value): string|int|bool|null | ||
| private function tryCoerce(mixed $value): mixed | ||
| { | ||
| if ($value === null) { | ||
| return null; | ||
| } | ||
| return match ($this) { | ||
| self::string => is_bool($value) ? ($value ? '1' : '0') : (string)$value, | ||
| self::string => self::toString($value), | ||
| self::integer => self::toInteger($value), | ||
| self::boolean => self::toBoolean($value), | ||
| self::float => self::toFloat($value), | ||
| self::array => self::toArray($value), | ||
| self::dateTime => self::toDateTime($value), | ||
| }; | ||
| } | ||
|
|
||
| private static function toInteger(string|int|bool $value): ?int | ||
| private static function toString(mixed $value): ?string | ||
| { | ||
| if (is_bool($value)) { | ||
| return $value ? '1' : '0'; | ||
| } | ||
| if (is_string($value) || is_int($value) || is_float($value)) { | ||
| return (string)$value; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private static function toInteger(mixed $value): ?int | ||
| { | ||
| if (is_int($value)) { | ||
| return $value; | ||
| } | ||
| if (is_bool($value)) { | ||
| return $value ? 1 : 0; | ||
| } | ||
| if (!is_string($value)) { | ||
| return null; | ||
| } | ||
| $trimmed = trim($value); | ||
| return preg_match('/^-?\d+$/', $trimmed) === 1 ? (int)$trimmed : null; | ||
| } | ||
|
|
||
| private static function toBoolean(string|int|bool $value): ?bool | ||
| private static function toBoolean(mixed $value): ?bool | ||
| { | ||
| if (is_bool($value)) { | ||
| return $value; | ||
|
|
@@ -84,10 +120,66 @@ private static function toBoolean(string|int|bool $value): ?bool | |
| default => null, | ||
| }; | ||
| } | ||
| if (!is_string($value)) { | ||
| return null; | ||
| } | ||
| return match (strtolower(trim($value))) { | ||
| '1', 'true', 'on', 'yes' => true, | ||
| '0', 'false', 'off', 'no' => false, | ||
| default => null, | ||
| }; | ||
| } | ||
|
|
||
| private static function toFloat(mixed $value): ?float | ||
| { | ||
| if (is_float($value) && !is_nan($value) && !is_infinite($value)) { | ||
| return $value; | ||
| } | ||
| if (is_int($value)) { | ||
| return (float)$value; | ||
| } | ||
| if (!is_string($value)) { | ||
| return null; | ||
| } | ||
| $trimmed = trim($value); | ||
| return preg_match('/^-?\d+(\.\d+)?$/', $trimmed) === 1 ? (float)$trimmed : null; | ||
| } | ||
|
|
||
| private static function toArray(mixed $value): ?array | ||
| { | ||
| if (is_array($value)) { | ||
| return $value; | ||
| } | ||
| if (!is_string($value)) { | ||
| return null; | ||
| } | ||
| try { | ||
| $decoded = json_decode($value, true, flags: JSON_THROW_ON_ERROR); | ||
| } catch (JsonException) { | ||
| return null; | ||
| } | ||
| return is_array($decoded) ? $decoded : null; | ||
| } | ||
|
|
||
| private static function toDateTime(mixed $value): ?DateTimeImmutable | ||
| { | ||
| if ($value instanceof DateTimeImmutable) { | ||
| return $value; | ||
| } | ||
| if ($value instanceof DateTimeInterface) { | ||
| return DateTimeImmutable::createFromInterface($value); | ||
| } | ||
| if (!is_string($value)) { | ||
| return null; | ||
| } | ||
| $parsed = DateTimeImmutable::createFromFormat(DATE_ATOM, $value); | ||
| if ($parsed !== false) { | ||
| return $parsed; | ||
| } | ||
| try { | ||
| return new DateTimeImmutable($value); | ||
| } catch (Throwable) { | ||
| return null; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,22 +22,22 @@ | |
| final readonly class MetaDataPropertyValue | ||
| { | ||
| /** | ||
| * @param string|int|bool|null $value the effective value, i.e. the own value falling back to the inherited one | ||
| * @param string|int|bool|null $ownValue the value stored for the dimension space point that was asked for | ||
| * @param string|int|bool|null $inheritedValue the value stored for the closest fallback dimension space point | ||
| * @param mixed $value the effective value, i.e. the own value falling back to the inherited one - see {@see MetaDataPropertyType} for the concrete type | ||
| * @param mixed $ownValue the value stored for the dimension space point that was asked for | ||
| * @param mixed $inheritedValue the value stored for the closest fallback dimension space point | ||
| * @param MetaDataDimensionSpacePoint|null $inheritedFrom the dimension space point the inherited value stems from | ||
| */ | ||
| private function __construct( | ||
| public string|int|bool|null $value, | ||
| public string|int|bool|null $ownValue, | ||
| public string|int|bool|null $inheritedValue, | ||
| public mixed $value, | ||
| public mixed $ownValue, | ||
| public mixed $inheritedValue, | ||
|
Comment on lines
+31
to
+33
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mixed is the broadest type that we can use. I would prefer union so that consumers can |
||
| public ?MetaDataDimensionSpacePoint $inheritedFrom, | ||
| ) { | ||
| } | ||
|
|
||
| public static function create( | ||
| string|int|bool|null $ownValue, | ||
| string|int|bool|null $inheritedValue = null, | ||
| mixed $ownValue, | ||
| mixed $inheritedValue = null, | ||
| ?MetaDataDimensionSpacePoint $inheritedFrom = null, | ||
| ): self { | ||
| return new self( | ||
|
|
@@ -63,7 +63,7 @@ public function hasOwnValue(): bool | |
| } | ||
|
|
||
| /** Fusion getter access */ | ||
| public function getOwnValue(): string|int|bool|null | ||
| public function getOwnValue(): mixed | ||
| { | ||
| return $this->ownValue; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these? Can we add some
@paramannotation with type + descriptionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this related and used by the new code at all?