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

Improve property configuration #128

Merged
merged 30 commits into from
Apr 18, 2024
Merged

Conversation

dresslerdemos
Copy link
Contributor

No description provided.

The code may have no problems to work with Symfony 7,
so we allow this dependency for now.

The same goes for `php-parser` and `phpunit`.

`phpdocumentator` can be removed, as this is not the
proper way to install it to generate documentation.
Instead a decoupled executable is to be used.
This commit attempts to solve the concerns in the referenced issue.
A follow-up commit will be necessary to bring this work closer to
completion.

Refs: #115
Continue to solve the concerns in the referenced issue.
Replacement of all behavior factories with static `createFactory`
methods in the corresponding behavior implementation
was not possible, because their parent interfaces require
typehinting, that can not be done using anonymous classes.

Maybe after the removal of `TCondition` and `TSorting` it
becomes possible to remove additional factories, but
other will need to remain.

Refs: #115
These are initial, but relatively complete versions.
To open/edit them UMLet is needed.
Properly integrating them into the documentation
articles is intended to be done at some point.
@dresslerdemos dresslerdemos self-assigned this Apr 3, 2024
# Conflicts:
#	CHANGELOG.md
#	composer.json
#	docs/README.adoc
#	phpstan.neon
To reduce compatibility breaking changes, the
previously replaced methods were added again,
but marked as deprecated.

A template parameter was adjusted as well. Neither
having it defined in the method nor having it
defined in the class docblock seem correct. This
needs to be investigated further for all behavior
factory classes, but for now the (understandable)
concern was added to the phpstan baseline of
concerns.

Refs: #115
Instead of coupling the `AbstractResourceType` to
some kind of configuration instance, we instead
prepare it to be a configuration itself. To do so,
we leave it to extending classes to provide
behaviors like readability or updatability and
remove the abstract `getResourceConfig` that
so far provided these.

Refs: #115
To reduce compatibility breaking changes, the
previously replaced method was added again,
but marked as deprecated.

A template parameter was adjusted as well. Neither
having it defined in the method nor having it
defined in the class docblock seem correct. This
needs to be investigated further for all behavior
factory classes, but for now the (understandable)
concern was added to the phpstan baseline of
concerns.

Refs: #115
* Typehint with interfaces instead of `callable`
* improve documentation wording
* add `removeAliasedPath` method

Refs: #115
Copy link

@martendemos martendemos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only read it

Passing and using a config builder as relationship
type (as shown in the README.adoc) does only work
if we postpone actually
requiring the relationship type. E.g. if
the config builder for `Book` resources references
authors and the authors have a reference to the books,
then on building the `Book` config builder it needs the
`Author` relationship type from the `Author`
config builder, so the author config needs to be build too,
which in turn requires the `Book` config builder to be
build first.

By not requiring the actual relationship resource type
when building, but a provider instead, all the configs
can be build first and only when the relationship type
is actually needed for some task it is requested from
the provider.

Keeping the possibility to initialize relationship
config builders from the actual relationship still
works outside of the context of resource config
builders, e.g. when only using schema config
builders.

Refs: #115
Allows to configure a resource type without
having to create a resource type class.

Refs: #115
Config-builder classes are to be refactored
to be simply configs, from which type instances
can be created. Naming the method to retrieve
such types `getType` instead of `getConfig`
makes more sense. The interface names were
adjusted accordingly.
Do not release `addConstructorCreationBehavior` with
a constructor behavior as parameter. There should
be a single `addCreationBehavior` taking a behavior
factory that may (or may not) execute the constructor
behavior as well as the post-constructor behavior.

`addConstructorBehavior` will be kept for backward
compatibility for now.

Refs: #115
@dresslerdemos dresslerdemos merged commit 6a356a2 into main Apr 18, 2024
0 of 2 checks passed
@dresslerdemos dresslerdemos deleted the wip-115-property-configuration branch April 18, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants