Skip to content

Conversation

@mhsdesign
Copy link
Member

Followup to #5682

Separated because discussions might block main thread.

Upgrade instructions

Review instructions

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@mhsdesign mhsdesign changed the base branch from 9.0 to feature/new-link-editor-link-value-object November 17, 2025 14:39
/**
* @return list<AssetId>
*/
public function getAssetIds(): array;
Copy link
Member Author

Choose a reason for hiding this comment

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

i went with an "always plural" interface as we have to account for the empty case and that a value object handles more than one asset too. So this seems the best fit.

Also we should probably specify iterable so people could yield the asset ids for simplicity during nesting.

Copy link
Member

Choose a reason for hiding this comment

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

Iterable wound indeed be a better fit than array.

return $this->href->getHost() . $this->href->getPath();
}

public function extractNodeAggregateId(): ?NodeAggregateId
Copy link
Member

Choose a reason for hiding this comment

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

I'm a big fan of tryExtractWhatever if whatever is nullable, for readability


use Neos\Media\Domain\Model\AssetId;

interface ProvidesAssetIdsInterface
Copy link
Member

Choose a reason for hiding this comment

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

The name sounds a bit strange why not AssetIdProviderInterface

Copy link
Member

@mficzel mficzel left a comment

Choose a reason for hiding this comment

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

IUntil now i have seen the following patterns for handlers / extractors / converter … thingys

  1. A chain of objects that get asked wether they can handle the type
  2. A configuration that specifies the extractor for each type

The approach here would be a third pattern but it has the clear benefit allowing a really type safe implementation. Only downside is that there is no delegation as everything is implemented by the object itself. Actually this might be the biggest advantage aswell.

I think in total it makes sense. If there is prior art of this pattern in the codebase i would agree even more

Base automatically changed from feature/new-link-editor-link-value-object to 9.1 November 18, 2025 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants