Skip to content

Minimal approach to remove metastore_resource_mapper_new_revision - #4751

Draft
dafeder wants to merge 1 commit into
4.xfrom
remove-static-variable-minimal
Draft

Minimal approach to remove metastore_resource_mapper_new_revision#4751
dafeder wants to merge 1 commit into
4.xfrom
remove-static-variable-minimal

Conversation

@dafeder

@dafeder dafeder commented Jul 3, 2026

Copy link
Copy Markdown
Member

This is a more minimal approach to #4750. Also copilot-generated, please ignore until I've come back to this, just bookmarking an experiment while I go back to other things.

Need to look at this carefully, I really don't remember exactly why we needed this originally; where the thread was getting lost in the workflow.

Copilot's explanation of changes:

Implemented an incremental transition path that runs both mechanisms in parallel: explicit decision propagation is now supported, while the legacy static flag remains the fallback and source of backward compatibility.

Changed files:

  • DatastoreSubscriber.php
  • LifeCycle.php
  • Referencer.php

What this incremental path looks like:

  1. Subscriber computes decision once, publishes it two ways.
  • In DatastoreSubscriber.php, onPreReference now:
  • Accepts both payload shapes:
    • legacy event data: MetastoreItemInterface
    • new payload object: { item, createNewResourceVersion }
  • Computes should_create_new_resource_version as before.
  • Writes legacy static flag (metastore_resource_mapper_new_revision) for existing callers.
  • Also writes explicit createNewResourceVersion onto event payload for new callers.
  1. Lifecycle now sends/reads explicit payload but does not require it.
  • In LifeCycle.php, referenceMetadata now:
  • Dispatches pre-reference event with payload object containing item and nullable createNewResourceVersion.
  • Reads back createNewResourceVersion if subscriber sets it.
  • Passes nullable decision into referencer.
  1. Referencer accepts explicit decision with legacy fallback.
  • In Referencer.php:
  • Added optional ?bool $createNewResourceVersion = NULL through reference, referenceProperty, referenceMultiple, referenceSingle, distributionHandling, registerWithResourceMapper, and handleExistingResource.
  • If explicit decision is NULL, handleExistingResource falls back to ResourceMapper::newRevision() exactly as before.
  • If explicit decision is set, it takes precedence.

Validation run:

  • ddev phpunit DatastoreSubscriberTest.php --stop-on-error passed.
  • ddev phpunit ReferencerTest.php --stop-on-error passed.
  • ddev phpunit OnPreReferenceTest.php --stop-on-error passed.

This gives you a safe bridge state:

  • Old behavior still works unchanged.
  • New explicit-parameter flow is live and can be expanded.
  • Final cleanup later can remove static usage once all callers are explicit.

If you want, next I can do the follow-up phase that removes the static fallback from Referencer.php only after adding one targeted test that proves explicit-only behavior.

Completed: Fix subscriber unit tests for constructor signature change (1/4)

Made changes.

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.

1 participant