Skip to content

!!! FEATURE: Compatibility with neos/metadata 3 - #33

Draft
Sebobo wants to merge 9 commits into
masterfrom
feature/new-metadata-support
Draft

!!! FEATURE: Compatibility with neos/metadata 3#33
Sebobo wants to merge 9 commits into
masterfrom
feature/new-metadata-support

Conversation

@Sebobo

@Sebobo Sebobo commented Sep 2, 2026

Copy link
Copy Markdown
Member

This (possible breaking) change makes the package compatible with the changes made in neos/metadata#11

Notable changes:

  • bumped dependencies
  • phpstan max level checks
  • replaced travis and styles with GitHub action
  • adjusted API where required

Breaking changes:

As the API in the metadata changed, this package also works a bit different now. If the package was used "as is", it should be suffice to run ./flow metadata:extract to read the metadata again from the stored assets.
If the package was extended, f.e. with custom converters, those need to be adjusted probably.

@Sebobo
Sebobo force-pushed the feature/new-metadata-support branch from 6af592f to f4254a3 Compare September 2, 2026 11:51
@Sebobo
Sebobo force-pushed the feature/new-metadata-support branch from f4254a3 to 91ef4e1 Compare September 2, 2026 12:55
@Sebobo
Sebobo marked this pull request as ready for review September 2, 2026 14:30
@Sebobo
Sebobo requested review from bwaidelich and dlubitz September 2, 2026 14:30
@Sebobo
Sebobo marked this pull request as draft September 2, 2026 14:40
@Sebobo

Sebobo commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Turned it back to draft, as we haven't released the new metadata package version yet

@Sebobo

Sebobo commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

@kdambekalns I saw that you maintained this package in the last years a bit. So this is just a ping, that we make some bigger changes ;)

Comment thread Classes/Command/MetaDataCommandController.php Outdated
Comment on lines +85 to +92
$connection->executeStatement('CREATE TABLE IF NOT EXISTS neos_metadata_value (
`asset_source_id` VARCHAR(255) DEFAULT NULL,
`asset_id` VARCHAR(40) DEFAULT NULL,
`property_name` VARCHAR(40) NOT NULL,
`property_value` VARCHAR(250) NOT NULL,
`dimension_hash` VARCHAR(250) NOT NULL,
UNIQUE INDEX idx_unique (`asset_source_id`, `asset_id`, `property_name`, `dimension_hash`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE `utf8mb4_unicode_ci` ENGINE = InnoDB');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Uff, this is not good. This will break at any point. Why is this table not created by flow/neos initialization?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah thx! Sorry an oversight I saw that the agent added but then forgot to remove

@Sebobo Sebobo Sep 3, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hm the Neos.MetaData Tests do the same, as it doesn't use actual doctrine entities.
Ok I don't know how to solve this the "correct" way. I can neither mock the final MetaDataManager, nor can I rely on the tables to be created.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can mock the MetaDataManager by its interface .... it doesn't have 🙄 Maybe it makes sense to add an interface for the public methods. 🤷‍♂️

/cc @bwaidelich @c4ll-m3-j4ck

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

And now I have the same problem of course in the Media.Ui tests…

Maybe one day I will understand how final classes make my life easier, instead of cursing them regularly.

@kdambekalns

Copy link
Copy Markdown
Member

@kdambekalns I saw that you maintained this package in the last years a bit.

Thanks! I wouldn't have know. 😎

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.

3 participants