Skip to content
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

FEATURE: Create value objects from injected setting #3080

Open
1 task done
robertlemke opened this issue Jun 15, 2023 · 4 comments · May be fixed by #3092
Open
1 task done

FEATURE: Create value objects from injected setting #3080

robertlemke opened this issue Jun 15, 2023 · 4 comments · May be fixed by #3092

Comments

@robertlemke
Copy link
Member

Is there an existing issue for this?

  • I have searched the existing issues

Description

Consider the following injected property:

#[Flow\InjectConfiguration(path: 'defaultClusterIdentifier')]
protected string $defaultClusterIdentifier;

Because the setting is a string, we need to use string as the type of the property. Ideally though, we could use a value object and Flow instantiates it automatically using its fromString() method:

#[Flow\InjectConfiguration(path: 'defaultClusterIdentifier')]
protected ClusterIdentifier $defaultClusterIdentifier;

Likewise this should be supported for fromArray() and other methods, if they exist in the declared class.

Possible Solution

No response

@robertlemke
Copy link
Member Author

See also #3077

@mhsdesign
Copy link
Member

yesss ❤️ ,

other related discussion about setting validation (which will happen due to value objects and their constraints): #3043

and similar to what @bwaidelich explained there:

class ClusterIdentifier
{
public function __construct(private readonly string $foo) {}
}

and glue it together with a line in Objects.yaml like:

Some\Class\ClusterIdentifier:
  arguments:
    1:
      setting: Neos.Neos.foo

@mhsdesign mhsdesign linked a pull request Jun 16, 2023 that will close this issue
6 tasks
@mhsdesign
Copy link
Member

in light of #3043 i would really like speaking exceptions like for the following case:

class PrototypeClassM
{
    #[Flow\InjectConfiguration('tests.functional.settingInjection.someSetting')]
    public ValueObjectClassB $configuration;
}

when tests.functional.settingInjection.someSetting is an empty string.

And ValueObjectClassB validates, that it must not be empty:

class ValueObjectClassB
{
    public function __construct(
        readonly public string $value,
    ) {
        if ($value === '') {
            throw new \InvalidArgumentException('Value must not be empty', 1684166315);
        }
    }

    public static function fromString(string $value): self
    {
        return new self($value);
    }
}

i prototyped for #3092 to throw an exception like this:

Settings-Configuration "Neos.Flow.tests.functional.settingInjection.someSetting" with value "" could not be deserialized to type "Neos\Flow\Tests\Functional\ObjectManagement\Fixtures\ValueObjectClassB": "Value must not be empty". In Neos\Flow\Tests\Functional\ObjectManagement\Fixtures\PrototypeClassM::configuration.

@mhsdesign
Copy link
Member

as it seems this works already, by using the injectors, as they have a higher prio than a normal property set.

class PrototypeClassM
{
    #[Flow\InjectConfiguration('tests.functional.settingInjection.someSetting')]
    readonly protected ValueObjectClassB $myConfig;

    public function injectMyConfig(string $configuration): void
    {
        $this->myConfig = ValueObjectClassB::fromString($configuration);
    }

    public function getMyConfig(): ValueObjectClassB
    {
        return $this->myConfig;
    }
}

is this acutally correct behaviour? - didnt know about it until after reading the code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

2 participants