Skip to content

!!! FEATURE: Extensible meta data properties - #11

Open
bwaidelich wants to merge 44 commits into
mainfrom
feature/extensible-metadata-properties
Open

!!! FEATURE: Extensible meta data properties#11
bwaidelich wants to merge 44 commits into
mainfrom
feature/extensible-metadata-properties

Conversation

@bwaidelich

@bwaidelich bwaidelich commented Apr 15, 2026

Copy link
Copy Markdown
Member

Major rewrite to support extensible, configurable meta data properties

Todos

  • Do we want to support more complex property types and conversion? (e.g. DateTime)?
    – suggestion: no, let's keep it simple
  • Validation is currently out of scope, but it could be implemented via ui.editorOptions
    • For now the editor view should take care of validation
    • Hard input validation could be a problem with migrating and importing data
  • Should dimension fallbacks be supported or is that responsibility of the consumer?
  • Include Asset Source Id
  • Global vs "localized" scope for metadata properties (similar to CR property scopes) => FEATURE: Respect globalScope and type, and find assets by metadata #13
    • Setting and getting metadata should respect this
    • Probably needs a migration when changing dimensions
  • Check inputs against configuration! => FEATURE: Respect globalScope and type, and find assets by metadata #13
    • Convert property values when setting them
  • ACL
    • Not planned for the first version -> integrators can check this in the metadata editor view
  • Extend Eel helper to access individual properties => FEATURE: Respect globalScope and type, and find assets by metadata #13
    • To decide: Move Eel helper to "glue package"?
  • Support search in metadata => FEATURE: Respect globalScope and type, and find assets by metadata #13
    • Filter by asset source id
    • Dimension coordinate
    • Respect scope and fallbacks
    • Searchterm
    • List of properties to search in (can be different for Media.Ui searches and in the Eel helper)
  • We need to think about how to integrate the package with Neos, also neos/metadata-extractor has a dependency and might need an adjustment
    • adjust neos/metadata-extractor to use new MedaDataManager
  • We need to trigger f.e. AssetService->emitAssetUpdated to flush caches etc. when metadata is updated
    • To decide: Signal dispatching in Package.php of neos/metadata or "glue package"
  • Agree on the API scope, i.e. forward-compatibility with a CR based storage (see comment from @dlubitz at FEATURE: Respect globalScope and type, and find assets by metadata #13 (comment) and follow-ups)

Major rewrite to support extensible, configurable meta data properties
@bwaidelich

Copy link
Copy Markdown
Member Author

NOTE: This is Work in Progress. Before anything gets merged (and potentially released as new major version) we will evaluate if we can re-implement support for the previous features (such as metadata extraction, ...) or whether this new functionality should be moved to a separate package

Comment thread Classes/MetaDataManager.php Outdated
}

public function setMetaDataPropertyValue(
string $assetId,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we always need a full asset identity consisting of the asset id and the asset source id.

I'm also currently checking all code in the Media.Ui to provide both or at least the asset source id to support new implementations like @nezaniel builds.

@bwaidelich bwaidelich Apr 16, 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.

@Sebobo good call.
Would it make sense to abstract that here already as AssetUri?
i.e. instead of

doSomething('some-asset-source', 'some-asset-id')

it would be

doSomething('some-asset-source://some-asset-id')

or maybe rather

doSomething('asset://some-asset-source/some-asset-id')

/cc @nezaniel ?

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.

nevermind – I think, I'll go for two new VOs in this package. They probably need to be stored in separate columns anyways (in the DBAL implementation) and the URI representation is just an implementation detail anyways

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.

I have realized that in the form of a MetaDataAssetReference VO now.
If already installed, make sure to run

./flow doctrine:migrationexecute 20260415145934 --direction down
./flow doctrine:migrationexecute 20260415145934 --direction up

and simplify property value implementation
Comment thread Classes/DimensionSpacePointProvider/DimensionSpacePointProvider.php Outdated
@dlubitz dlubitz moved this from Backlog to In progress in PostCon Sprint 2026 Jun 5, 2026
@bwaidelich

Copy link
Copy Markdown
Member Author

@Sebobo @dlubitz @c4ll-m3-j4ck (mentioning you as fellow contributors) – I'm sorry for the silence around this one. I did not lose track, but focus..
Is anyone of you willing to finalize this one together with me? I'm happy to invest some more time but I could use a sparring partner

@dlubitz

dlubitz commented Jul 22, 2026

Copy link
Copy Markdown

I can help here. I've planned some more Neos time in August anyways.

{
foreach ($this->assetRepository->findAll() as $asset) {
/** @var Asset $asset */
$title = $asset->getTitle();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the title being ignored here?


public function migrateExistingAssetPropertiesCommand(): void
{
foreach ($this->assetRepository->findAll() as $asset) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need to check what happens with assets from other asset sources

`createAllPresetCombinations()` built its coordinates from the preset
identifiers, while everything else in the adapter works with dimension values:
the fallback chain matches on `values[0]` and the default dimension space point
is built from the `default` of each dimension.

For the common configuration where a preset is named after its primary value the
two coincide, but as soon as they differ - a preset "german" with the values
["de"] - the dimension space points returned by `getDimensionSpacePoints()` were
rejected by `isDimensionSpacePointValid()`, and the default dimension space point
was not among them.

Presets without values are now skipped as well, as they could never be matched.
Reading the label raised an "Undefined array key" warning for properties that
are declared without a `ui` section. The label already falls back to the property
name when it is NULL.
Properties declared with `globalScope: true` now have a single value that is
shared by all dimensions instead of the flag being ignored. Such values are
stored under the reserved dimension hash `global`, so that for a given asset and
property the storage holds either one shared value or one value per dimension
space point, never both. Reads always look up the scope a property is configured
for, which makes values of the respective other shape unreachable rather than
wrong after a configuration change.

The four read methods of the MetaDataManager are replaced by one, because they
were three views of the same answer rather than three questions:
`MetaDataPropertyValue` now carries the own value (for editing, without
shine-through), the inherited value and its origin (for the translation hint)
and the effective value (for rendering) side by side. This also halves the
number of queries an inspector needs.

Resolution rules move out of the storage: it no longer picks a winner via a
MySQL specific `FIELD()` ordering but returns all values of the requested scope,
leaving fallback priority to the MetaDataManager.

Adds `assetmetadata:repair` to consolidate values whose scope contradicts the
current configuration, plus unit and functional test suites.
Adds regression tests for 7d4c8f1: a configuration whose preset identifiers
differ from the primary values of those presets, asserting that dimension space
points are enumerated by value, that every enumerated point is considered valid
and includes the default one, and that presets without values are skipped.

All three fail when 7d4c8f1 is reverted.
bwaidelich and others added 11 commits August 3, 2026 16:57
Adds `MetaDataManager::findAssets()`, returning the references of all assets
that have a matching metadata value. The criteria of a `MetaDataAssetFilter`
are all optional: asset source id, dimension space point, search term and the
properties to search in.

A value only counts if it is the one `getMetaDataPropertyValue()` would return
for the filter's dimension space point, so the search agrees with what an
editor working in that dimension sees: an asset whose caption is inherited
from a fallback dimension is found, one whose inherited caption is overridden
by a non matching value of its own is not. Properties with a global scope are
matched on their shared value regardless of the dimension space point, just
like they are read regardless of it.

As everywhere else in this package, an omitted dimension space point means the
*default* one rather than "any dimension" - a search is always carried out as
seen from one dimension space point.

Resolving the precedence per asset in PHP would mean a query per candidate, so
it happens in SQL: the manager hands the storage the fallback chain ordered,
from the most to the least specific dimension space point, and the storage
applies that ranking rather than deriving one. The "order is meaningless"
rule therefore moves from the interface docblock onto the method it actually
describes.

!!! `MetaDataStorage` gained a method, so third party implementations of that
interface have to be extended.
Allows a single metadata property to be read from Fusion without fetching all
of them first:

    caption = ${AssetMetaData.getMetaDataProperty(asset, 'caption', {language: 'de'})}

Like `getMetaData()` it returns the effective value, i.e. with dimension
fallbacks applied, and empty coordinates mean the default dimension.

`allowsCallOfMethod()` now allows every method, so that helper methods added
in the future are usable from Fusion without having to be listed twice.
Values used to be written and read as provided, so the `type` a property is
declared with was exposed to consumers but never applied. It is now enforced
in both directions: on the way in, so that nothing but a value of that type is
ever stored, and on the way out, so that `MetaDataPropertyValue::$value` means
what its `string|int|bool|null` signature says.

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 with an
`InvalidArgumentException` rather than silently turned into a wrong value -
"abc" is not 0.

Reading is deliberately more forgiving, because it meets values that were
written before a property was given its current type: a value that cannot be
interpreted reads as NULL. Such a value is skipped rather than treated as an
empty one, so that it does not shadow a fallback that is still readable.

Booleans are stored as `1`/`0` and integers in decimal, which is what was
already written before, so existing values stay readable.
There are two kinds of tests now: one for the `MetaDataManager`, with its
dependencies as test doubles, and one per implementation of those
dependencies.

The manager tests no longer round trip through an in-memory storage. Each
states the values the storage holds and asserts what resolves from them, so a
test says what a rule *is* rather than demonstrating it through a write
followed by a read. Writes are asserted as the calls they make. The repair
tests work the same way and record those calls in order, which finally states
outright that promotions happen before the deletions of the rows they came
from.

What a storage does with a lookup is its own business and is covered once per
implementation. The tests of the DBAL adapter therefore also took over what
used to be tested through the manager: the shadowing of values further down
the fallback chain, the escaping of LIKE wildcards, distinctness, ordering and
the separation of the localized and the global scope - plus the isolation of
assets and asset sources, which was never a rule of the manager to begin with.
They also cover the `MetaDataStorageMaintenance` surface that
`assetmetadata:repair` is built on, including the values of unconfigured
dimensions and undefined properties that reads can never return.

`MetaDataConfigurationProviderYamlAdapter` had no test at all and has one now.

The adapter stays MySQL specific, so its tests remain the only ones that need
a database.
They describe where dimensions come from and what happens when the scope of a
property is changed, so they belong to the configuration section as a whole
rather than under the subsection about property types that was inserted above
them.
…a-properties

FIX: Add getOwnValue method to MetaDataPropertyValue for fusion access
This allows to disable property definitions that are configured
elsewhere:

    Neos:
      MetaData:
        metaDataProperties:
          'copyright': ~
…a-properties

FEATURE: Allow nullable ui configuration
@bwaidelich
bwaidelich marked this pull request as ready for review August 6, 2026 14:09
@Sebobo

Sebobo commented Aug 21, 2026

Copy link
Copy Markdown
Member

Thanks for all the changes, is this now a finalish reviewable state on which I can fully build upon in the Media.Ui ?

@dlubitz

dlubitz commented Aug 26, 2026

Copy link
Copy Markdown

@Sebobo I think there are some things left. But this shouldn't effect the integration into Media.UI.

`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,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

250 characters is not enough. I already had some files in my own website with a longer caption and the migration command failed.
Neos used longtext but that is too much. So I discussed with @dlubitz that we should use text instead for 65k characters.

Suggested change
`property_value` VARCHAR(250) NOT NULL,
`property_value` TEXT NOT NULL,

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

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

4 participants