Skip to content

Rewrite (M0): strip old plugin, scaffold resource-agnostic engine#110

Merged
loevgaard merged 6 commits into
1.xfrom
feature/v1-rewrite
Jun 18, 2026
Merged

Rewrite (M0): strip old plugin, scaffold resource-agnostic engine#110
loevgaard merged 6 commits into
1.xfrom
feature/v1-rewrite

Conversation

@loevgaard

Copy link
Copy Markdown
Member

Complete rewrite — new major version (M0)

This branch begins the complete rewrite of the plugin into a resource-agnostic feed
generation engine (design: .notes/sylius-feed-plugin-spec.md). It targets 1.x as the new
major; the old plugin remains on 0.6.x (default) and 0.7.x. There is no upgrade path
the old domain code does not carry forward.

Work is milestone-gated (.notes/rewrite-progress.md). This PR contains M0 only.

M0 — strip old plugin + scaffold the engine skeleton

  • Delete the old Google-Shopping-specific domain, pipeline, templates, grids, routing, config.
  • Add the 7 extension-point interfaces + registries (FeedType, MappingPreset, ValueResolver,
    LookupSource, Transformation, FilterOperator, Writer), auto-tagged via _instanceof and wired
    with tagged_iterator injection.
  • Foundational value objects: FeedContext, bag-backed FeedItem, FieldType/ScopeDimension/
    SourceType enums, FieldDefinition, FieldMapping, TransformationConfig, FilterSet.
  • New domain model: Feed (+ translation), FeedSource, FeedField, FeedFilter
    (setono_sylius_feed__ table prefix), registered as Sylius resources.
  • New workflow graph: ready → processing → completed | failed.
  • composer: add league/csv + symfony/property-access, drop spatie/enum.
  • Disable the Infection mutation-test CI job (mutation testing is out of scope for the rewrite).

Verified locally

ECS · PHPStan max (cleared baseline) · PHPUnit · lint:container · lint:yaml/lint:twig ·
Rector · composer validate --strict/normalize · Doctrine mapping validate.

Known: dependency-analysis will fail on this commit

M0's minimal src/ does not yet use several declared runtime deps (doctrine/orm, messenger,
liip-imagine, twig, league/csv, property-access, …). These are consumed by M1, so the
dependency-analysis job is expected to report unused dependencies until M1 lands. Not a
regression — a milestone-sequencing artifact.

Next

M1 — core engine: product-variant resolvers, streaming XmlWriter + google_rss,
FeedGenerator, ProductVariantFeedType + DoctrineDataSource, GoogleShoppingMappingPreset,
fixtures + a functional test producing a valid Google RSS feed.

Complete-rewrite groundwork (new major on feature/v1-rewrite):

- Delete the old Google-Shopping-specific domain, pipeline, templates and config.
- Add the 7 extension-point interfaces + registries (FeedType, MappingPreset,
  ValueResolver, LookupSource, Transformation, FilterOperator, Writer), wired via
  _instanceof autoconfiguration + tagged_iterator injection.
- Add foundational value objects: FeedContext, FeedItem (bag), FieldType/ScopeDimension/
  SourceType enums, FieldDefinition, FieldMapping, TransformationConfig, FilterSet.
- New domain model: Feed (+translation), FeedSource, FeedField, FeedFilter with
  setono_sylius_feed__ table prefix; registered as Sylius resources.
- Rewrite the workflow graph to ready/processing/completed/failed (process/complete/
  fail/reset).
- composer: add league/csv + symfony/property-access, drop spatie/enum.
- Disable the Infection mutation-test CI job (out of scope for the rewrite).

Green locally: ECS, PHPStan max (cleared baseline), PHPUnit, lint:container,
lint:yaml/twig, Rector, composer validate/normalize, Doctrine mapping validate.
Dependency-analyser deferred to the M1 boundary (deps consumed by M1).
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.46465% with 14 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (1.x@f52ea30). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...dencyInjection/Compiler/RegisterFilesystemPass.php 47.05% 9 Missing ⚠️
src/Model/FeedSource.php 90.69% 4 Missing ⚠️
src/Workflow/FeedGraph.php 50.00% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##             1.x     #110   +/-   ##
======================================
  Coverage       ?   97.01%           
  Complexity     ?      178           
======================================
  Files          ?       24           
  Lines          ?      502           
  Branches       ?        0           
======================================
  Hits           ?      487           
  Misses         ?       15           
  Partials       ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The SCA CI job removes sylius/sylius, resolving the standalone resource-bundle
TranslatableTrait (Sylius\Resource\Model, class_alias'd) whose only constructor
is __construct (no initializeTranslationsCollection). Aliasing the trait
constructor therefore produced an undefined method under PHPStan. Initialize the
translations collection directly in the constructor instead — works for both the
monorepo and standalone trait variants.
Comment thread .github/workflows/build.yaml
Comment thread src/Context/FeedContext.php Outdated
Comment thread src/DataSource/DataSourceInterface.php
Comment thread src/FeedType/FeedTypeRegistry.php Outdated
Comment thread src/Filter/FilterOperatorInterface.php Outdated
Comment thread src/Item/FeedItem.php Outdated
Comment thread src/Lookup/LookupSourceRegistry.php Outdated
Comment thread phpstan-baseline.neon Outdated
- Keep the mutation-tests CI job but mark it continue-on-error (ignored, not removed).
- Lowercase FeedContext::key() for case-insensitive filesystem/URL stability.
- Narrow resource-class types with class-string phpdoc (DataSource, ValueResolver).
- Introduce an abstract, iterable+countable Registry base; all 7 registries now
  extend it and their interfaces extend IteratorAggregate (iterable by nature).
- Rename the shared FilterOperator* to Operator* (new Operator namespace, tag
  setono_sylius_feed.operator) since the operator vocabulary is shared by filters,
  field conditions and conditional transforms.
- FeedItem now implements IteratorAggregate, ArrayAccess and Countable over its bag.
- Split the PHPUnit suite into 'unit' (tests/Unit, no kernel) and 'functional'
  (tests/Functional, boots the test-app kernel via FunctionalTestCase); add
  composer phpunit-unit / phpunit-functional scripts.
- Unit tests for every M0 class: FeedContext, FeedItem, FieldMapping,
  TransformationConfig, FieldDefinition, FilterSet, all 7 registries (+ abstract
  Registry base), FeedGraph, and the Feed/FeedSource/FeedField/FeedFilter/
  FeedTranslation models.
- Functional tests: registry service wiring (all 7 tags), the feed state-machine
  registration, and the Doctrine mappings (table names + Feed associations).
- Mark the registry services public so they're retrievable (they are the plugin's
  central registry API, injected from M1 onward).
- CLAUDE.md: require every feature to be tested, document the unit/functional split,
  fix the single-test example, and correct the now non-blocking mutation-testing note.

68 tests (54 unit + 54 assertions... 14 functional) green; ECS, PHPStan max, Rector clean.
Comment thread src/ValueResolver/ValueResolverRegistryInterface.php Outdated
Flysystem (modernize & keep the storage seam):
- RegisterFilesystemPass now targets FilesystemOperator only (drop the league/flysystem
  v1 / FilesystemInterface dual-support carried over from the old plugin).
- composer: require league/flysystem ^2.1||^3.0 and flysystem-bundle ^2.4||^3.0.
- Drop the now-unused v1 ignores from composer-dependency-analyser.php and the
  League\Flysystem class.notFound ignore from phpstan.neon.

PR review:
- Remove phpstan-baseline.neon and its include — the rewrite keeps zero errors, no baseline.
- Add a generic Registry\RegistryInterface (iterable + countable) extended by all seven
  registry interfaces; lift the shared has()/all() into the abstract Registry so each
  concrete registry is just a narrowed get() + key extractor.
We won't use the batcher for batch processing. Drop both packages from composer
(and the lock), unregister SetonoDoctrineORMBatcherBundle from the test app, drop the
batcher-bundle entry from the dependency-analyser ignore list, and remove it from the
README install instructions.

The M1 DoctrineDataSource will stream via Query::toIterable() + EntityManager::clear()
per read-batch instead (plan notes updated).
@loevgaard loevgaard merged commit 9666d65 into 1.x Jun 18, 2026
22 of 30 checks passed
@loevgaard loevgaard deleted the feature/v1-rewrite branch June 18, 2026 09:45
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