From 6096803fb9d201764efd0f1f99260bb1ab492635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Thu, 2 Jul 2026 15:49:32 +0200 Subject: [PATCH 1/4] M6: FeedFilter evaluation (pre/post stages) - FilterEvaluator resolves a filter's field/value via the reference-resolution rule + the shared operator vocabulary; exclude drops on match, include drops on non-match. Wired into the generator: pre filters run before mapping, post filters on the mapped output; excluded items are counted and skipped. - SourceResolverBinder: bind the item's on-demand source resolver once (before pre-filters, reused by mapping) preserving lookup: delegation; idempotent so FieldMappingEvaluator is unchanged. - FeedFilterType admin form + a filters collection on FeedSourceType. - exclude-out-of-stock proven end to end. +9 tests (512 total); PHPStan max (incl. PHP 8.1) + ECS + Rector + container green. --- src/Filter/FilterEvaluator.php | 62 ++++ src/Filter/FilterEvaluatorInterface.php | 23 ++ src/Form/Type/FeedFilterType.php | 85 ++++++ src/Form/Type/FeedSourceType.php | 7 + src/Generator/FeedGenerator.php | 24 +- src/Generator/FieldMappingEvaluator.php | 14 +- src/Generator/SourceResolverBinder.php | 47 +++ src/Item/FeedItem.php | 10 + src/Resources/config/services.xml | 1 + src/Resources/config/services/filter.xml | 13 + src/Resources/config/services/form.xml | 5 + src/Resources/config/services/generator.xml | 2 + src/Resources/translations/messages.en.yaml | 14 + tests/Unit/Filter/FilterEvaluatorTest.php | 144 +++++++++ .../Generator/FeedGeneratorFilterTest.php | 276 ++++++++++++++++++ .../Generator/FeedGeneratorMemoryTest.php | 3 + tests/Unit/Generator/FeedGeneratorTest.php | 4 + 17 files changed, 722 insertions(+), 12 deletions(-) create mode 100644 src/Filter/FilterEvaluator.php create mode 100644 src/Filter/FilterEvaluatorInterface.php create mode 100644 src/Form/Type/FeedFilterType.php create mode 100644 src/Generator/SourceResolverBinder.php create mode 100644 src/Resources/config/services/filter.xml create mode 100644 tests/Unit/Filter/FilterEvaluatorTest.php create mode 100644 tests/Unit/Generator/FeedGeneratorFilterTest.php diff --git a/src/Filter/FilterEvaluator.php b/src/Filter/FilterEvaluator.php new file mode 100644 index 00000000..e564c62a --- /dev/null +++ b/src/Filter/FilterEvaluator.php @@ -0,0 +1,62 @@ +excludes($filter, $item)) { + return $filter; + } + } + + return null; + } + + private function excludes(FeedFilterInterface $filter, FeedItem $item): bool + { + $field = $filter->getField(); + $operator = $filter->getOperator(); + + // An incomplete filter (no field/operator) is inert — it never excludes anything. + if (null === $field || null === $operator) { + return false; + } + + $left = $this->referenceResolver->resolve($field, null, $item); + + $rawValue = $filter->getValue(); + $operand = is_string($rawValue) ? $this->referenceResolver->resolve($rawValue, null, $item) : $rawValue; + + $matched = $this->operatorRegistry->get($operator)->matches($left, ['value' => $operand]); + + return FeedFilterInterface::ACTION_EXCLUDE === $filter->getAction() ? $matched : !$matched; + } +} diff --git a/src/Filter/FilterEvaluatorInterface.php b/src/Filter/FilterEvaluatorInterface.php new file mode 100644 index 00000000..3d62797b --- /dev/null +++ b/src/Filter/FilterEvaluatorInterface.php @@ -0,0 +1,23 @@ + $filters + */ + public function excludedBy(FeedItem $item, iterable $filters): ?FeedFilterInterface; +} diff --git a/src/Form/Type/FeedFilterType.php b/src/Form/Type/FeedFilterType.php new file mode 100644 index 00000000..0d0c6146 --- /dev/null +++ b/src/Form/Type/FeedFilterType.php @@ -0,0 +1,85 @@ +add('field', TextType::class, [ + 'label' => 'setono_sylius_feed.form.feed_filter.field', + ]) + ->add('operator', ChoiceType::class, [ + 'label' => 'setono_sylius_feed.form.feed_filter.operator', + 'choices' => $this->operatorChoices(), + 'choice_translation_domain' => false, + ]) + ->add('value', TextType::class, [ + 'required' => false, + 'label' => 'setono_sylius_feed.form.feed_filter.value', + 'help' => 'setono_sylius_feed.form.feed_filter.value_help', + ]) + ->add('action', ChoiceType::class, [ + 'label' => 'setono_sylius_feed.form.feed_filter.action', + 'choices' => [ + 'setono_sylius_feed.form.feed_filter.action_choices.include' => FeedFilterInterface::ACTION_INCLUDE, + 'setono_sylius_feed.form.feed_filter.action_choices.exclude' => FeedFilterInterface::ACTION_EXCLUDE, + ], + ]) + ->add('stage', ChoiceType::class, [ + 'label' => 'setono_sylius_feed.form.feed_filter.stage', + 'choices' => [ + 'setono_sylius_feed.form.feed_filter.stage_choices.pre' => FeedFilterInterface::STAGE_PRE, + 'setono_sylius_feed.form.feed_filter.stage_choices.post' => FeedFilterInterface::STAGE_POST, + ], + ]) + ; + } + + public function configureOptions(OptionsResolver $resolver): void + { + $resolver->setDefault('data_class', FeedFilter::class); + } + + public function getBlockPrefix(): string + { + return 'setono_sylius_feed_feed_filter'; + } + + /** + * @return array + */ + private function operatorChoices(): array + { + $choices = []; + foreach ($this->operatorRegistry->all() as $operator) { + $name = $operator->getName(); + $choices[$name] = $name; + } + + ksort($choices); + + return $choices; + } +} diff --git a/src/Form/Type/FeedSourceType.php b/src/Form/Type/FeedSourceType.php index 2305f3c5..3becaca8 100644 --- a/src/Form/Type/FeedSourceType.php +++ b/src/Form/Type/FeedSourceType.php @@ -40,6 +40,13 @@ public function buildForm(FormBuilderInterface $builder, array $options): void 'allow_delete' => true, 'by_reference' => false, ]) + ->add('filters', CollectionType::class, [ + 'label' => 'setono_sylius_feed.form.feed_source.filters', + 'entry_type' => FeedFilterType::class, + 'allow_add' => true, + 'allow_delete' => true, + 'by_reference' => false, + ]) ; $builder->addEventListener(FormEvents::POST_SUBMIT, function (FormEvent $event): void { diff --git a/src/Generator/FeedGenerator.php b/src/Generator/FeedGenerator.php index 19183525..f30aef26 100644 --- a/src/Generator/FeedGenerator.php +++ b/src/Generator/FeedGenerator.php @@ -8,9 +8,11 @@ use Setono\SyliusFeedPlugin\Context\FeedContext; use Setono\SyliusFeedPlugin\Event\FeedItemBuiltEvent; use Setono\SyliusFeedPlugin\FeedType\FeedTypeRegistryInterface; +use Setono\SyliusFeedPlugin\Filter\FilterEvaluatorInterface; use Setono\SyliusFeedPlugin\Filter\FilterSet; use Setono\SyliusFeedPlugin\Format\FormatRegistryInterface; use Setono\SyliusFeedPlugin\Item\FeedItem; +use Setono\SyliusFeedPlugin\Lookup\LookupReferenceResolverInterface; use Setono\SyliusFeedPlugin\Mapping\FieldMapping; use Setono\SyliusFeedPlugin\MappingPreset\MappingPresetRegistryInterface; use Setono\SyliusFeedPlugin\Model\FeedFieldInterface; @@ -35,6 +37,8 @@ public function __construct( private readonly FormatRegistryInterface $formatRegistry, private readonly FeedWriterRegistryInterface $writerRegistry, private readonly FieldMappingEvaluatorInterface $fieldMappingEvaluator, + private readonly FilterEvaluatorInterface $filterEvaluator, + private readonly LookupReferenceResolverInterface $lookupReferenceResolver, private readonly RequiredFieldsValidatorInterface $requiredFieldsValidator, private readonly EventDispatcherInterface $eventDispatcher, private readonly UrlGeneratorInterface $urlGenerator, @@ -68,12 +72,30 @@ public function generate(FeedInterface $feed, FeedContext $context): GenerationR $feedType = $this->feedTypeRegistry->get((string) $source->getFeedType()); $availableFields = $feedType->getAvailableFields(); $mappings = $this->resolveMappings($feed, $source); + $filterSet = new FilterSet($source->getFilters()); - foreach ($feedType->getDataSource()->getItems($context, new FilterSet($source->getFilters())) as $entity) { + foreach ($feedType->getDataSource()->getItems($context, $filterSet) as $entity) { $item = new FeedItem($entity, $context); + // Bind the source resolver up front so pre-filters can resolve raw source fields; + // mapping reuses the same (idempotent) binding. NOTE: all filters are evaluated per + // item — pushing `pre` filters into the data source query is a future optimization. + SourceResolverBinder::bind($item, $availableFields, $this->lookupReferenceResolver); + + if (null !== $this->filterEvaluator->excludedBy($item, $filterSet->getPreFilters())) { + ++$excludedCount; + + continue; + } + $this->fieldMappingEvaluator->apply($item, $mappings, $availableFields); + if (null !== $this->filterEvaluator->excludedBy($item, $filterSet->getPostFilters())) { + ++$excludedCount; + + continue; + } + $this->eventDispatcher->dispatch(new FeedItemBuiltEvent($item)); if ($item->isSkipped() || [] !== $this->requiredFieldsValidator->findMissingFields($item, $requiredFields)) { diff --git a/src/Generator/FieldMappingEvaluator.php b/src/Generator/FieldMappingEvaluator.php index 2158eaa2..c47ee522 100644 --- a/src/Generator/FieldMappingEvaluator.php +++ b/src/Generator/FieldMappingEvaluator.php @@ -32,17 +32,9 @@ public function __construct( public function apply(FeedItem $item, iterable $mappings, array $availableFields): void { - $entity = $item->getEntity(); - $context = $item->getContext(); - $item->setSourceResolver(function (string $field) use ($entity, $context, $availableFields): mixed { - if ($this->lookupReferenceResolver->supports($field)) { - return $this->lookupReferenceResolver->resolve($field, $entity, $context, $availableFields); - } - - return isset($availableFields[$field]) - ? $availableFields[$field]->getResolver()->resolve($entity, $context) - : null; - }); + // Idempotent: the generator binds this up front (so pre-filters can resolve source fields); + // this call is the safety net when apply() runs standalone, e.g. in unit tests. + SourceResolverBinder::bind($item, $availableFields, $this->lookupReferenceResolver); foreach ($mappings as $mapping) { if (!$this->conditionSatisfied($mapping, $item)) { diff --git a/src/Generator/SourceResolverBinder.php b/src/Generator/SourceResolverBinder.php new file mode 100644 index 00000000..6509b871 --- /dev/null +++ b/src/Generator/SourceResolverBinder.php @@ -0,0 +1,47 @@ + $availableFields + */ + public static function bind(FeedItem $item, array $availableFields, LookupReferenceResolverInterface $lookupReferenceResolver): void + { + if ($item->hasSourceResolver()) { + return; + } + + $entity = $item->getEntity(); + $context = $item->getContext(); + + $item->setSourceResolver(static function (string $field) use ($entity, $context, $availableFields, $lookupReferenceResolver): mixed { + if ($lookupReferenceResolver->supports($field)) { + return $lookupReferenceResolver->resolve($field, $entity, $context, $availableFields); + } + + return isset($availableFields[$field]) + ? $availableFields[$field]->getResolver()->resolve($entity, $context) + : null; + }); + } +} diff --git a/src/Item/FeedItem.php b/src/Item/FeedItem.php index 19e48d1c..04695ec0 100644 --- a/src/Item/FeedItem.php +++ b/src/Item/FeedItem.php @@ -64,6 +64,16 @@ public function setSourceResolver(\Closure $sourceResolver): void $this->sourceResolver = $sourceResolver; } + /** + * Whether an on-demand source resolver has already been bound (see $sourceResolver). Lets the + * binding be established once per item — up front by the generator (so pre-filters can resolve + * raw source fields) and re-requested by the mapping evaluator without rebinding. + */ + public function hasSourceResolver(): bool + { + return null !== $this->sourceResolver; + } + /** * Resolve a named source field / value resolver against this item on demand. Returns null when * no resolver is bound or the name is unknown — a miss is never an error (§10). diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index aae03888..65562545 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -6,6 +6,7 @@ + diff --git a/src/Resources/config/services/filter.xml b/src/Resources/config/services/filter.xml new file mode 100644 index 00000000..aab601fa --- /dev/null +++ b/src/Resources/config/services/filter.xml @@ -0,0 +1,13 @@ + + + + + + + + + + + + diff --git a/src/Resources/config/services/form.xml b/src/Resources/config/services/form.xml index b7cd873a..52e1e6f2 100644 --- a/src/Resources/config/services/form.xml +++ b/src/Resources/config/services/form.xml @@ -29,6 +29,11 @@ + + + + + %setono_sylius_feed.model.lookup_table.class% diff --git a/src/Resources/config/services/generator.xml b/src/Resources/config/services/generator.xml index 0ac64f2e..d67fb35c 100644 --- a/src/Resources/config/services/generator.xml +++ b/src/Resources/config/services/generator.xml @@ -23,6 +23,8 @@ + + diff --git a/src/Resources/translations/messages.en.yaml b/src/Resources/translations/messages.en.yaml index 6ea62387..6d02e75e 100644 --- a/src/Resources/translations/messages.en.yaml +++ b/src/Resources/translations/messages.en.yaml @@ -31,6 +31,20 @@ setono_sylius_feed: feed_source: feed_type: Feed type fields: Field mappings + filters: Filters + feed_filter: + field: Field + operator: Operator + value: Value + value_help: 'The value to compare against — a field name, an earlier output field, or a quoted literal (e.g. ''out_of_stock''). Leave empty for operators that take no value (e.g. empty / not_empty).' + action: Action + stage: Stage + action_choices: + include: Include matching + exclude: Exclude matching + stage_choices: + pre: Before mapping + post: After mapping feed_field: output_field: Output field source_type: Source type diff --git a/tests/Unit/Filter/FilterEvaluatorTest.php b/tests/Unit/Filter/FilterEvaluatorTest.php new file mode 100644 index 00000000..825e234e --- /dev/null +++ b/tests/Unit/Filter/FilterEvaluatorTest.php @@ -0,0 +1,144 @@ +item(['availability' => 'out_of_stock']); + $filter = $this->filter('availability', 'equals', "'out_of_stock'", FeedFilterInterface::ACTION_EXCLUDE); + + self::assertSame($filter, $this->evaluator()->excludedBy($item, [$filter])); + } + + /** + * @test + */ + public function it_keeps_an_item_when_an_exclude_filter_does_not_match(): void + { + $item = $this->item(['availability' => 'in_stock']); + $filter = $this->filter('availability', 'equals', "'out_of_stock'", FeedFilterInterface::ACTION_EXCLUDE); + + self::assertNull($this->evaluator()->excludedBy($item, [$filter])); + } + + /** + * @test + */ + public function it_excludes_an_item_when_an_include_filter_does_not_match(): void + { + $item = $this->item(['availability' => 'out_of_stock']); + $filter = $this->filter('availability', 'equals', "'in_stock'", FeedFilterInterface::ACTION_INCLUDE); + + self::assertSame($filter, $this->evaluator()->excludedBy($item, [$filter])); + } + + /** + * @test + */ + public function it_keeps_an_item_when_an_include_filter_matches(): void + { + $item = $this->item(['availability' => 'in_stock']); + $filter = $this->filter('availability', 'equals', "'in_stock'", FeedFilterInterface::ACTION_INCLUDE); + + self::assertNull($this->evaluator()->excludedBy($item, [$filter])); + } + + /** + * @test + */ + public function it_returns_the_first_excluding_filter(): void + { + $item = $this->item(['availability' => 'out_of_stock', 'brand' => 'Acme']); + $passing = $this->filter('brand', 'equals', "'Acme'", FeedFilterInterface::ACTION_INCLUDE); + $excluding = $this->filter('availability', 'equals', "'out_of_stock'", FeedFilterInterface::ACTION_EXCLUDE); + + self::assertSame($excluding, $this->evaluator()->excludedBy($item, [$passing, $excluding])); + } + + /** + * @test + */ + public function it_skips_a_filter_with_a_null_operator(): void + { + $item = $this->item(['availability' => 'out_of_stock']); + + $filter = new FeedFilter(); + $filter->setField('availability'); + $filter->setOperator(null); + $filter->setValue("'out_of_stock'"); + $filter->setAction(FeedFilterInterface::ACTION_EXCLUDE); + + self::assertNull($this->evaluator()->excludedBy($item, [$filter])); + } + + /** + * A non-string filter value is compared verbatim rather than resolved as a reference. + * + * @test + */ + public function it_uses_a_non_string_value_verbatim(): void + { + $item = $this->item(['price' => 100]); + + $filter = new FeedFilter(); + $filter->setField('price'); + $filter->setOperator('gt'); + $filter->setValue(50); + $filter->setAction(FeedFilterInterface::ACTION_EXCLUDE); + + self::assertSame($filter, $this->evaluator()->excludedBy($item, [$filter])); + } + + private function evaluator(): FilterEvaluator + { + return new FilterEvaluator( + new ReferenceResolver(), + new OperatorRegistry([new Equals(), new In(), new GreaterThan()]), + ); + } + + private function filter(string $field, string $operator, mixed $value, string $action): FeedFilterInterface + { + $filter = new FeedFilter(); + $filter->setField($field); + $filter->setOperator($operator); + $filter->setValue($value); + $filter->setAction($action); + + return $filter; + } + + /** + * @param array $source + */ + private function item(array $source): FeedItem + { + $item = new FeedItem(new \stdClass(), new FeedContext()); + $item->setSourceResolver(static fn (string $field): mixed => $source[$field] ?? null); + + return $item; + } +} diff --git a/tests/Unit/Generator/FeedGeneratorFilterTest.php b/tests/Unit/Generator/FeedGeneratorFilterTest.php new file mode 100644 index 00000000..509a1089 --- /dev/null +++ b/tests/Unit/Generator/FeedGeneratorFilterTest.php @@ -0,0 +1,276 @@ +filesystem = new Filesystem(new LocalFilesystemAdapter(sys_get_temp_dir() . '/setono-feed-' . bin2hex(random_bytes(6)))); + } + + protected function tearDown(): void + { + $this->filesystem->deleteDirectory('shop'); + } + + /** + * @dataProvider stages + * + * @test + */ + public function it_excludes_the_out_of_stock_item(string $stage): void + { + $result = $this->createGenerator()->generate($this->feed($stage), new FeedContext(null, 'en_US', 'USD')); + + self::assertSame(1, $result->itemCount); + self::assertSame(1, $result->excludedCount); + + $rows = [...Reader::createFromString($this->filesystem->read($result->path))->getRecords()]; + self::assertCount(2, $rows, 'header + the single kept row'); + self::assertSame(['id', 'availability'], $rows[0]); + self::assertSame(['SKU-1', 'in_stock'], $rows[1]); + } + + /** + * @return iterable + */ + public function stages(): iterable + { + yield 'pre' => [FeedFilterInterface::STAGE_PRE]; + yield 'post' => [FeedFilterInterface::STAGE_POST]; + } + + private function createGenerator(): FeedGenerator + { + $feedTypeRegistry = $this->prophesize(FeedTypeRegistryInterface::class); + $feedTypeRegistry->get('product_variant')->willReturn($this->feedType()); + + $presetRegistry = $this->prophesize(MappingPresetRegistryInterface::class); + + $formatRegistry = $this->prophesize(FormatRegistryInterface::class); + $formatRegistry->get('csv')->willReturn(new CsvFormat()); + + $writerRegistry = $this->prophesize(FeedWriterRegistryInterface::class); + $writerRegistry->get('csv')->willReturn(new CsvWriter()); + + $urlGenerator = $this->prophesize(UrlGeneratorInterface::class); + $urlGenerator->getContext()->willReturn(new RequestContext()); + + return new FeedGenerator( + $feedTypeRegistry->reveal(), + $presetRegistry->reveal(), + $formatRegistry->reveal(), + $writerRegistry->reveal(), + new FieldMappingEvaluator( + new TransformationChain(new TransformationRegistry([])), + new ReferenceResolver(), + new OperatorRegistry([new IsTrue()]), + new ExpressionEvaluator(new InMemoryLookup()), + new SandboxedTwigRenderer(new FeedTemplateSecurityPolicy(), new InMemoryLookup()), + new NullLookupReferenceResolver(), + ), + new FilterEvaluator(new ReferenceResolver(), new OperatorRegistry([new Equals()])), + new NullLookupReferenceResolver(), + new RequiredFieldsValidator(), + new EventDispatcher(), + $urlGenerator->reveal(), + $this->filesystem, + ); + } + + private function feedType(): FeedTypeInterface + { + $fields = [ + 'id' => $this->field('id'), + 'availability' => $this->field('availability'), + ]; + + $dataSource = new class() implements DataSourceInterface { + public function getResourceClass(): string + { + return \stdClass::class; + } + + public function getItems(FeedContext $context, FilterSet $filters): iterable + { + yield (object) ['id' => 'SKU-1', 'availability' => 'in_stock']; + yield (object) ['id' => 'SKU-2', 'availability' => 'out_of_stock']; + } + + public function count(FeedContext $context, FilterSet $filters): int + { + return 2; + } + }; + + return new class($dataSource, $fields) implements FeedTypeInterface { + /** + * @param array $fields + */ + public function __construct( + private readonly DataSourceInterface $dataSource, + private readonly array $fields, + ) { + } + + public function getCode(): string + { + return 'product_variant'; + } + + public function getLabel(): string + { + return 'test.product_variant'; + } + + public function getDataSource(): DataSourceInterface + { + return $this->dataSource; + } + + public function getScopeDimensions(): array + { + return [ScopeDimension::CHANNEL, ScopeDimension::LOCALE, ScopeDimension::CURRENCY]; + } + + public function getAvailableFields(): array + { + return $this->fields; + } + }; + } + + /** + * A field whose resolver reads the same-named property off the entity, so the two stub entities + * resolve to different availability values. + */ + private function field(string $name): FieldDefinition + { + $resolver = new class($name) implements ValueResolverInterface { + public function __construct(private readonly string $name) + { + } + + public function getName(): string + { + return $this->name; + } + + public function getLabel(): string + { + return 'test.' . $this->name; + } + + public function getType(): FieldType + { + return FieldType::STRING; + } + + public function supports(string $resourceClass): bool + { + return true; + } + + public function resolve(object $entity, FeedContext $context): mixed + { + return ((array) $entity)[$this->name] ?? null; + } + }; + + return new FieldDefinition($name, $resolver->getLabel(), FieldType::STRING, $resolver); + } + + private function feed(string $stage): FeedInterface + { + $filter = new FeedFilter(); + $filter->setField('availability'); + $filter->setOperator('equals'); + $filter->setValue("'out_of_stock'"); + $filter->setAction(FeedFilterInterface::ACTION_EXCLUDE); + $filter->setStage($stage); + + $source = $this->prophesize(FeedSourceInterface::class); + $source->getFeedType()->willReturn('product_variant'); + $source->getPosition()->willReturn(0); + $source->getFilters()->willReturn(new ArrayCollection([$filter])); + $source->getFields()->willReturn(new ArrayCollection([ + $this->feedField('id', 'id', 0), + $this->feedField('availability', 'availability', 1), + ])); + + $feed = $this->prophesize(FeedInterface::class); + $feed->getCode()->willReturn('shop'); + $feed->getFormat()->willReturn('csv'); + $feed->getSources()->willReturn(new ArrayCollection([$source->reveal()])); + + return $feed->reveal(); + } + + private function feedField(string $outputField, string $sourceField, int $position): FeedFieldInterface + { + $field = new FeedField(); + $field->setOutputField($outputField); + $field->setSourceType('field'); + $field->setSourceValue($sourceField); + $field->setPosition($position); + + return $field; + } +} diff --git a/tests/Unit/Generator/FeedGeneratorMemoryTest.php b/tests/Unit/Generator/FeedGeneratorMemoryTest.php index 45fc5b18..2567afe9 100644 --- a/tests/Unit/Generator/FeedGeneratorMemoryTest.php +++ b/tests/Unit/Generator/FeedGeneratorMemoryTest.php @@ -13,6 +13,7 @@ use Setono\SyliusFeedPlugin\DataSource\DataSourceInterface; use Setono\SyliusFeedPlugin\FeedType\FeedTypeInterface; use Setono\SyliusFeedPlugin\FeedType\FeedTypeRegistryInterface; +use Setono\SyliusFeedPlugin\Filter\FilterEvaluator; use Setono\SyliusFeedPlugin\Filter\FilterSet; use Setono\SyliusFeedPlugin\Format\FormatRegistryInterface; use Setono\SyliusFeedPlugin\Format\GoogleRssFormat; @@ -106,6 +107,8 @@ private function createGenerator(Filesystem $filesystem): FeedGenerator new SandboxedTwigRenderer(new FeedTemplateSecurityPolicy(), new InMemoryLookup()), new NullLookupReferenceResolver(), ), + new FilterEvaluator(new ReferenceResolver(), new OperatorRegistry([])), + new NullLookupReferenceResolver(), new RequiredFieldsValidator(), new EventDispatcher(), $urlGenerator->reveal(), diff --git a/tests/Unit/Generator/FeedGeneratorTest.php b/tests/Unit/Generator/FeedGeneratorTest.php index 915da014..19163148 100644 --- a/tests/Unit/Generator/FeedGeneratorTest.php +++ b/tests/Unit/Generator/FeedGeneratorTest.php @@ -14,6 +14,7 @@ use Setono\SyliusFeedPlugin\DataSource\DataSourceInterface; use Setono\SyliusFeedPlugin\FeedType\FeedTypeInterface; use Setono\SyliusFeedPlugin\FeedType\FeedTypeRegistryInterface; +use Setono\SyliusFeedPlugin\Filter\FilterEvaluator; use Setono\SyliusFeedPlugin\Filter\FilterSet; use Setono\SyliusFeedPlugin\Format\CsvFormat; use Setono\SyliusFeedPlugin\Format\FormatRegistryInterface; @@ -33,6 +34,7 @@ use Setono\SyliusFeedPlugin\Model\FeedFieldInterface; use Setono\SyliusFeedPlugin\Model\FeedInterface; use Setono\SyliusFeedPlugin\Model\FeedSourceInterface; +use Setono\SyliusFeedPlugin\Operator\Equals; use Setono\SyliusFeedPlugin\Operator\IsTrue; use Setono\SyliusFeedPlugin\Operator\OperatorRegistry; use Setono\SyliusFeedPlugin\Reference\ReferenceResolver; @@ -265,6 +267,8 @@ private function createGenerator(?array $presets = null): FeedGenerator new SandboxedTwigRenderer(new FeedTemplateSecurityPolicy(), new InMemoryLookup()), new NullLookupReferenceResolver(), ), + new FilterEvaluator(new ReferenceResolver(), new OperatorRegistry([new Equals()])), + new NullLookupReferenceResolver(), new RequiredFieldsValidator(), new EventDispatcher(), $urlGenerator->reveal(), From 322d081e887fedf361159323146c7dc9470c8c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Thu, 2 Jul 2026 16:18:34 +0200 Subject: [PATCH 2/4] M6: typed-item validation + FeedContextResult + excluded reporting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - FeedType::createItem() lets a feed type provide a typed FeedItem (product_variant -> GoogleShoppingItem). Symfony constraints on GoogleShoppingItem (required g: fields + title<=150), applied format-scoped (FormatInterface::getItemValidationGroups; google_rss runs them, CSV/others fall back to the required-field list) so a CSV product_variant feed isn't held to the RSS spec. - FeedItemValidator: typed constraint violations + required-field fallback, deduped. - FeedContextResult resource (feed, contextKey, itemCount/excludedCount/bytes, errors json, createdAt) + repository (findLatestPublished) — the publish-gate baseline. Every exclusion is recorded with a reason (filter:{stage}:{field} / validation:{msg} / skipped), capped, via ExclusionCollector; recorded at finalize by FeedContextResultRecorder. +11 tests (523 total); PHPStan max (incl. 8.1) + ECS + Rector + container green. --- src/DependencyInjection/Configuration.php | 20 +++ src/FeedType/FeedTypeInterface.php | 9 ++ src/FeedType/ProductFeedType.php | 7 + src/FeedType/ProductVariantFeedType.php | 8 + src/Format/CsvFormat.php | 5 + src/Format/FormatInterface.php | 11 ++ src/Format/GenericXmlFormat.php | 5 + src/Format/GoogleRssFormat.php | 7 + src/Format/PartnerAdsFormat.php | 5 + src/Generator/ExclusionCollector.php | 42 +++++ src/Generator/FeedContextResultRecorder.php | 48 ++++++ .../FeedContextResultRecorderInterface.php | 17 ++ src/Generator/FeedGenerator.php | 50 ++++-- src/Generator/GenerationResult.php | 10 +- .../GenerateFeedContextHandler.php | 10 +- src/Model/FeedContextResult.php | 110 +++++++++++++ src/Model/FeedContextResultInterface.php | 61 ++++++++ .../FeedContextResultRepository.php | 28 ++++ .../FeedContextResultRepositoryInterface.php | 21 +++ .../doctrine/model/FeedContextResult.orm.xml | 28 ++++ src/Resources/config/services/generator.xml | 14 +- src/Resources/config/services/messenger.xml | 1 + .../config/validation/GoogleShoppingItem.xml | 86 +++++++++++ src/Resources/translations/validators.en.yaml | 18 +++ src/Validator/FeedItemValidator.php | 44 ++++++ src/Validator/FeedItemValidatorInterface.php | 27 ++++ tests/Functional/FeedContextResultTest.php | 96 ++++++++++++ .../Generator/FeedGeneratorFilterTest.php | 17 +- .../Generator/FeedGeneratorMemoryTest.php | 10 +- tests/Unit/Generator/FeedGeneratorTest.php | 10 +- tests/Unit/Model/FeedContextResultTest.php | 65 ++++++++ .../Unit/Validator/FeedItemValidatorTest.php | 146 ++++++++++++++++++ 32 files changed, 1018 insertions(+), 18 deletions(-) create mode 100644 src/Generator/ExclusionCollector.php create mode 100644 src/Generator/FeedContextResultRecorder.php create mode 100644 src/Generator/FeedContextResultRecorderInterface.php create mode 100644 src/Model/FeedContextResult.php create mode 100644 src/Model/FeedContextResultInterface.php create mode 100644 src/Repository/FeedContextResultRepository.php create mode 100644 src/Repository/FeedContextResultRepositoryInterface.php create mode 100644 src/Resources/config/doctrine/model/FeedContextResult.orm.xml create mode 100644 src/Resources/config/validation/GoogleShoppingItem.xml create mode 100644 src/Validator/FeedItemValidator.php create mode 100644 src/Validator/FeedItemValidatorInterface.php create mode 100644 tests/Functional/FeedContextResultTest.php create mode 100644 tests/Unit/Model/FeedContextResultTest.php create mode 100644 tests/Unit/Validator/FeedItemValidatorTest.php diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 3c665e7d..47eb746f 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -7,10 +7,13 @@ use Setono\SyliusFeedPlugin\Form\Type\FeedType; use Setono\SyliusFeedPlugin\Form\Type\LookupTableType; use Setono\SyliusFeedPlugin\Model\Feed; +use Setono\SyliusFeedPlugin\Model\FeedContextResult; +use Setono\SyliusFeedPlugin\Model\FeedContextResultInterface; use Setono\SyliusFeedPlugin\Model\FeedSource; use Setono\SyliusFeedPlugin\Model\FeedTranslation; use Setono\SyliusFeedPlugin\Model\LookupTable; use Setono\SyliusFeedPlugin\Model\LookupTableInterface; +use Setono\SyliusFeedPlugin\Repository\FeedContextResultRepository; use Setono\SyliusFeedPlugin\Repository\FeedRepository; use Setono\SyliusFeedPlugin\Repository\LookupTableRepository; use Sylius\Bundle\ResourceBundle\Controller\ResourceController; @@ -131,6 +134,23 @@ private function addResourcesSection(ArrayNodeDefinition $node): void ->end() ->end() ->end() + ->arrayNode('feed_context_result') + ->addDefaultsIfNotSet() + ->children() + ->variableNode('options')->end() + ->arrayNode('classes') + ->addDefaultsIfNotSet() + ->children() + ->scalarNode('model')->defaultValue(FeedContextResult::class)->cannotBeEmpty()->end() + ->scalarNode('interface')->defaultValue(FeedContextResultInterface::class)->cannotBeEmpty()->end() + ->scalarNode('controller')->defaultValue(ResourceController::class)->cannotBeEmpty()->end() + ->scalarNode('repository')->defaultValue(FeedContextResultRepository::class)->cannotBeEmpty()->end() + ->scalarNode('factory')->defaultValue(Factory::class)->end() + ->scalarNode('form')->defaultValue(DefaultResourceType::class)->cannotBeEmpty()->end() + ->end() + ->end() + ->end() + ->end() ->end() ->end() ->end() diff --git a/src/FeedType/FeedTypeInterface.php b/src/FeedType/FeedTypeInterface.php index 7c09458f..2d5e09b0 100644 --- a/src/FeedType/FeedTypeInterface.php +++ b/src/FeedType/FeedTypeInterface.php @@ -4,7 +4,9 @@ namespace Setono\SyliusFeedPlugin\FeedType; +use Setono\SyliusFeedPlugin\Context\FeedContext; use Setono\SyliusFeedPlugin\DataSource\DataSourceInterface; +use Setono\SyliusFeedPlugin\Item\FeedItem; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\ScopeDimension; @@ -29,6 +31,13 @@ public function getLabel(): string; public function getDataSource(): DataSourceInterface; + /** + * Create the per-item object for one entity of this feed type. A feed type MAY return a typed + * subclass of {@see FeedItem} (e.g. GoogleShoppingItem) so that named accessors and typed-item + * validation constraints apply; the default is a plain {@see FeedItem} over the generic bag (§4.2, §11). + */ + public function createItem(object $entity, FeedContext $context): FeedItem; + /** * @return list */ diff --git a/src/FeedType/ProductFeedType.php b/src/FeedType/ProductFeedType.php index a44f56a9..a96a798f 100644 --- a/src/FeedType/ProductFeedType.php +++ b/src/FeedType/ProductFeedType.php @@ -4,7 +4,9 @@ namespace Setono\SyliusFeedPlugin\FeedType; +use Setono\SyliusFeedPlugin\Context\FeedContext; use Setono\SyliusFeedPlugin\DataSource\DataSourceInterface; +use Setono\SyliusFeedPlugin\Item\FeedItem; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\ScopeDimension; use Setono\SyliusFeedPlugin\ValueResolver\ValueResolverRegistryInterface; @@ -56,6 +58,11 @@ public function getDataSource(): DataSourceInterface return $this->dataSource; } + public function createItem(object $entity, FeedContext $context): FeedItem + { + return new FeedItem($entity, $context); + } + public function getScopeDimensions(): array { return [ScopeDimension::CHANNEL, ScopeDimension::LOCALE, ScopeDimension::CURRENCY]; diff --git a/src/FeedType/ProductVariantFeedType.php b/src/FeedType/ProductVariantFeedType.php index 11b44b3a..80bc8bdd 100644 --- a/src/FeedType/ProductVariantFeedType.php +++ b/src/FeedType/ProductVariantFeedType.php @@ -4,7 +4,10 @@ namespace Setono\SyliusFeedPlugin\FeedType; +use Setono\SyliusFeedPlugin\Context\FeedContext; use Setono\SyliusFeedPlugin\DataSource\DataSourceInterface; +use Setono\SyliusFeedPlugin\Item\FeedItem; +use Setono\SyliusFeedPlugin\Item\Google\GoogleShoppingItem; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\ScopeDimension; use Setono\SyliusFeedPlugin\ValueResolver\ValueResolverRegistryInterface; @@ -56,6 +59,11 @@ public function getDataSource(): DataSourceInterface return $this->dataSource; } + public function createItem(object $entity, FeedContext $context): FeedItem + { + return new GoogleShoppingItem($entity, $context); + } + public function getScopeDimensions(): array { return [ScopeDimension::CHANNEL, ScopeDimension::LOCALE, ScopeDimension::CURRENCY]; diff --git a/src/Format/CsvFormat.php b/src/Format/CsvFormat.php index b4497cc9..15138cc1 100644 --- a/src/Format/CsvFormat.php +++ b/src/Format/CsvFormat.php @@ -34,4 +34,9 @@ public function getRequiredFields(): array { return []; } + + public function getItemValidationGroups(): array + { + return []; + } } diff --git a/src/Format/FormatInterface.php b/src/Format/FormatInterface.php index 24562455..402b745b 100644 --- a/src/Format/FormatInterface.php +++ b/src/Format/FormatInterface.php @@ -37,4 +37,15 @@ public function getConfig(): WriterConfigInterface; * @return list */ public function getRequiredFields(): array; + + /** + * The Symfony validation groups to run the typed per-item constraints under for this format + * (§11). A format that carries typed-item constraints (e.g. Google Shopping) returns the group(s) + * that activate them; a format that only relies on the generic required-field check returns an + * empty list, so a typed item (like GoogleShoppingItem) rendered under a non-matching format is + * not spuriously rejected. + * + * @return list + */ + public function getItemValidationGroups(): array; } diff --git a/src/Format/GenericXmlFormat.php b/src/Format/GenericXmlFormat.php index a2605b74..94607eb4 100644 --- a/src/Format/GenericXmlFormat.php +++ b/src/Format/GenericXmlFormat.php @@ -33,4 +33,9 @@ public function getRequiredFields(): array { return []; } + + public function getItemValidationGroups(): array + { + return []; + } } diff --git a/src/Format/GoogleRssFormat.php b/src/Format/GoogleRssFormat.php index a82856fc..e15bfda4 100644 --- a/src/Format/GoogleRssFormat.php +++ b/src/Format/GoogleRssFormat.php @@ -38,4 +38,11 @@ public function getRequiredFields(): array { return ['g:id', 'g:title', 'g:description', 'g:link', 'g:image_link', 'g:availability', 'g:price']; } + + public function getItemValidationGroups(): array + { + // Run the GoogleShoppingItem constraint mapping (Default group), which encodes the Google + // Merchant product spec on the typed item this format's product_variant source produces. + return ['Default']; + } } diff --git a/src/Format/PartnerAdsFormat.php b/src/Format/PartnerAdsFormat.php index e0cd3c7f..939bf311 100644 --- a/src/Format/PartnerAdsFormat.php +++ b/src/Format/PartnerAdsFormat.php @@ -32,4 +32,9 @@ public function getRequiredFields(): array { return []; } + + public function getItemValidationGroups(): array + { + return []; + } } diff --git a/src/Generator/ExclusionCollector.php b/src/Generator/ExclusionCollector.php new file mode 100644 index 00000000..782fdbf4 --- /dev/null +++ b/src/Generator/ExclusionCollector.php @@ -0,0 +1,42 @@ + */ + private array $errors = []; + + public function record(?string $item, string $reason): void + { + ++$this->count; + + if (count($this->errors) < self::MAX_RECORDED) { + $this->errors[] = ['item' => $item, 'reason' => $reason]; + } + } + + public function count(): int + { + return $this->count; + } + + /** + * @return list + */ + public function errors(): array + { + return $this->errors; + } +} diff --git a/src/Generator/FeedContextResultRecorder.php b/src/Generator/FeedContextResultRecorder.php new file mode 100644 index 00000000..582580a2 --- /dev/null +++ b/src/Generator/FeedContextResultRecorder.php @@ -0,0 +1,48 @@ +managerRegistry = $managerRegistry; + } + + public function record(FeedInterface $feed, string $contextKey, GenerationResult $result): FeedContextResultInterface + { + $feedContextResult = $this->feedContextResultFactory->createNew(); + Assert::isInstanceOf($feedContextResult, FeedContextResultInterface::class); + + $feedContextResult->setFeed($feed); + $feedContextResult->setContextKey($contextKey); + $feedContextResult->setItemCount($result->itemCount); + $feedContextResult->setExcludedCount($result->excludedCount); + $feedContextResult->setBytes($result->bytes); + $feedContextResult->setErrors($result->errors); + + $manager = $this->getManager($feedContextResult); + $manager->persist($feedContextResult); + $manager->flush(); + + return $feedContextResult; + } +} diff --git a/src/Generator/FeedContextResultRecorderInterface.php b/src/Generator/FeedContextResultRecorderInterface.php new file mode 100644 index 00000000..7e98e184 --- /dev/null +++ b/src/Generator/FeedContextResultRecorderInterface.php @@ -0,0 +1,17 @@ +withHeader($this->unionHeader($feed)); } $requiredFields = $format->getRequiredFields(); + $itemValidationGroups = $format->getItemValidationGroups(); $this->applyRequestContext($context); @@ -66,7 +67,7 @@ public function generate(FeedInterface $feed, FeedContext $context): GenerationR $writer->writePreamble(); $itemCount = 0; - $excludedCount = 0; + $exclusions = new ExclusionCollector(); foreach ($this->sortedSources($feed) as $source) { $feedType = $this->feedTypeRegistry->get((string) $source->getFeedType()); @@ -75,31 +76,40 @@ public function generate(FeedInterface $feed, FeedContext $context): GenerationR $filterSet = new FilterSet($source->getFilters()); foreach ($feedType->getDataSource()->getItems($context, $filterSet) as $entity) { - $item = new FeedItem($entity, $context); + $item = $feedType->createItem($entity, $context); // Bind the source resolver up front so pre-filters can resolve raw source fields; // mapping reuses the same (idempotent) binding. NOTE: all filters are evaluated per // item — pushing `pre` filters into the data source query is a future optimization. SourceResolverBinder::bind($item, $availableFields, $this->lookupReferenceResolver); - if (null !== $this->filterEvaluator->excludedBy($item, $filterSet->getPreFilters())) { - ++$excludedCount; + $excludingPreFilter = $this->filterEvaluator->excludedBy($item, $filterSet->getPreFilters()); + if (null !== $excludingPreFilter) { + $exclusions->record($this->resolveItemIdentifier($item), sprintf('filter:pre:%s', (string) $excludingPreFilter->getField())); continue; } $this->fieldMappingEvaluator->apply($item, $mappings, $availableFields); - if (null !== $this->filterEvaluator->excludedBy($item, $filterSet->getPostFilters())) { - ++$excludedCount; + $excludingPostFilter = $this->filterEvaluator->excludedBy($item, $filterSet->getPostFilters()); + if (null !== $excludingPostFilter) { + $exclusions->record($this->resolveItemIdentifier($item), sprintf('filter:post:%s', (string) $excludingPostFilter->getField())); continue; } $this->eventDispatcher->dispatch(new FeedItemBuiltEvent($item)); - if ($item->isSkipped() || [] !== $this->requiredFieldsValidator->findMissingFields($item, $requiredFields)) { - ++$excludedCount; + if ($item->isSkipped()) { + $exclusions->record($this->resolveItemIdentifier($item), 'skipped'); + + continue; + } + + $violations = $this->feedItemValidator->validate($item, $requiredFields, $itemValidationGroups); + if ([] !== $violations) { + $exclusions->record($this->resolveItemIdentifier($item), 'validation:' . implode('; ', $violations)); continue; } @@ -116,9 +126,27 @@ public function generate(FeedInterface $feed, FeedContext $context): GenerationR rewind($stream); $this->feedFilesystem->writeStream($path, $stream); + $stat = fstat($stream); + $bytes = false === $stat ? 0 : $stat['size']; fclose($stream); - return new GenerationResult($path, $itemCount, $excludedCount); + return new GenerationResult($path, $itemCount, $exclusions->count(), $bytes, $exclusions->errors()); + } + + /** + * The item's identity for exclusion reporting: its output id if the mapping has produced one, + * else null (§11). A pre-filter exclusion happens before mapping, so the id is usually null there. + */ + private function resolveItemIdentifier(FeedItem $item): ?string + { + foreach (['g:id', 'id'] as $key) { + $value = $item->get($key); + if (is_scalar($value) && '' !== (string) $value) { + return (string) $value; + } + } + + return null; } /** diff --git a/src/Generator/GenerationResult.php b/src/Generator/GenerationResult.php index a0710d8e..800d5792 100644 --- a/src/Generator/GenerationResult.php +++ b/src/Generator/GenerationResult.php @@ -5,15 +5,21 @@ namespace Setono\SyliusFeedPlugin\Generator; /** - * The outcome of generating one context's feed file: where it was written and how many items - * were included vs excluded. + * The outcome of generating one context's feed file (§11): where it was written, how many items + * were included vs excluded, the size of the produced file, and a bounded sample of per-item + * exclusion reasons. */ final class GenerationResult { + /** + * @param list $errors + */ public function __construct( public readonly string $path, public readonly int $itemCount, public readonly int $excludedCount, + public readonly int $bytes = 0, + public readonly array $errors = [], ) { } } diff --git a/src/MessageHandler/GenerateFeedContextHandler.php b/src/MessageHandler/GenerateFeedContextHandler.php index c897ceba..b34ad13a 100644 --- a/src/MessageHandler/GenerateFeedContextHandler.php +++ b/src/MessageHandler/GenerateFeedContextHandler.php @@ -7,6 +7,7 @@ use Doctrine\Persistence\ManagerRegistry; use Setono\Doctrine\ORMTrait; use Setono\SyliusFeedPlugin\Context\FeedContext; +use Setono\SyliusFeedPlugin\Generator\FeedContextResultRecorderInterface; use Setono\SyliusFeedPlugin\Generator\FeedGeneratorInterface; use Setono\SyliusFeedPlugin\Message\Command\GenerateFeedContext; use Setono\SyliusFeedPlugin\Model\FeedInterface; @@ -31,6 +32,7 @@ public function __construct( private readonly FeedRepositoryInterface $feedRepository, private readonly RepositoryInterface $channelRepository, private readonly FeedGeneratorInterface $feedGenerator, + private readonly FeedContextResultRecorderInterface $feedContextResultRecorder, private readonly Registry $workflowRegistry, ) { $this->managerRegistry = $managerRegistry; @@ -43,8 +45,10 @@ public function __invoke(GenerateFeedContext $message): void return; } + $context = $this->buildContext($message); + try { - $this->feedGenerator->generate($feed, $this->buildContext($message)); + $result = $this->feedGenerator->generate($feed, $context); } catch (\Throwable $exception) { $this->fail($message->feed); @@ -56,6 +60,10 @@ public function __invoke(GenerateFeedContext $message): void $feed = $this->feedRepository->find($message->feed); Assert::isInstanceOf($feed, FeedInterface::class); + // Record the excluded-item report + size/count for this context now that generation + // finished, against the managed feed (§11). + $this->feedContextResultRecorder->record($feed, $context->key(), $result); + $completed = $this->feedRepository->incrementCompletedContexts($feed); if (null !== $feed->getContextCount() && $completed >= $feed->getContextCount()) { $this->transition($feed, FeedGraph::TRANSITION_COMPLETE); diff --git a/src/Model/FeedContextResult.php b/src/Model/FeedContextResult.php new file mode 100644 index 00000000..2f821fc0 --- /dev/null +++ b/src/Model/FeedContextResult.php @@ -0,0 +1,110 @@ + */ + protected array $errors = []; + + protected \DateTimeInterface $createdAt; + + public function __construct() + { + $this->createdAt = new \DateTimeImmutable(); + } + + public function getId(): ?int + { + return $this->id; + } + + public function getFeed(): ?FeedInterface + { + return $this->feed; + } + + public function setFeed(?FeedInterface $feed): void + { + $this->feed = $feed; + } + + public function getContextKey(): ?string + { + return $this->contextKey; + } + + public function setContextKey(?string $contextKey): void + { + $this->contextKey = $contextKey; + } + + public function getItemCount(): int + { + return $this->itemCount; + } + + public function setItemCount(int $itemCount): void + { + $this->itemCount = $itemCount; + } + + public function getExcludedCount(): int + { + return $this->excludedCount; + } + + public function setExcludedCount(int $excludedCount): void + { + $this->excludedCount = $excludedCount; + } + + public function getBytes(): int + { + return $this->bytes; + } + + public function setBytes(int $bytes): void + { + $this->bytes = $bytes; + } + + public function getErrors(): array + { + return $this->errors; + } + + public function setErrors(array $errors): void + { + $this->errors = $errors; + } + + public function addError(?string $item, string $reason): void + { + $this->errors[] = ['item' => $item, 'reason' => $reason]; + } + + public function getCreatedAt(): \DateTimeInterface + { + return $this->createdAt; + } + + public function setCreatedAt(\DateTimeInterface $createdAt): void + { + $this->createdAt = $createdAt; + } +} diff --git a/src/Model/FeedContextResultInterface.php b/src/Model/FeedContextResultInterface.php new file mode 100644 index 00000000..15cc0bb2 --- /dev/null +++ b/src/Model/FeedContextResultInterface.php @@ -0,0 +1,61 @@ + + */ + public function getErrors(): array; + + /** + * @param list $errors + */ + public function setErrors(array $errors): void; + + public function addError(?string $item, string $reason): void; + + public function getCreatedAt(): \DateTimeInterface; + + public function setCreatedAt(\DateTimeInterface $createdAt): void; +} diff --git a/src/Repository/FeedContextResultRepository.php b/src/Repository/FeedContextResultRepository.php new file mode 100644 index 00000000..7ec487d9 --- /dev/null +++ b/src/Repository/FeedContextResultRepository.php @@ -0,0 +1,28 @@ +createQueryBuilder('o') + ->andWhere('o.feed = :feed') + ->andWhere('o.contextKey = :contextKey') + ->setParameter('feed', $feed) + ->setParameter('contextKey', $contextKey) + ->orderBy('o.createdAt', 'DESC') + ->addOrderBy('o.id', 'DESC') + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + + return $result instanceof FeedContextResultInterface ? $result : null; + } +} diff --git a/src/Repository/FeedContextResultRepositoryInterface.php b/src/Repository/FeedContextResultRepositoryInterface.php new file mode 100644 index 00000000..e90b25c8 --- /dev/null +++ b/src/Repository/FeedContextResultRepositoryInterface.php @@ -0,0 +1,21 @@ + + */ +interface FeedContextResultRepositoryInterface extends RepositoryInterface +{ + /** + * The most recent result recorded for the given feed and context key (§11) — the row the + * publish gate consults for the previously served output. Ordered by creation time, newest first. + */ + public function findLatestPublished(FeedInterface $feed, string $contextKey): ?FeedContextResultInterface; +} diff --git a/src/Resources/config/doctrine/model/FeedContextResult.orm.xml b/src/Resources/config/doctrine/model/FeedContextResult.orm.xml new file mode 100644 index 00000000..9a26c98c --- /dev/null +++ b/src/Resources/config/doctrine/model/FeedContextResult.orm.xml @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Resources/config/services/generator.xml b/src/Resources/config/services/generator.xml index d67fb35c..5e4e9578 100644 --- a/src/Resources/config/services/generator.xml +++ b/src/Resources/config/services/generator.xml @@ -7,6 +7,12 @@ + + + + + + @@ -25,11 +31,17 @@ - + + + + + + + diff --git a/src/Resources/config/services/messenger.xml b/src/Resources/config/services/messenger.xml index 773524f2..45be2613 100644 --- a/src/Resources/config/services/messenger.xml +++ b/src/Resources/config/services/messenger.xml @@ -18,6 +18,7 @@ + diff --git a/src/Resources/config/validation/GoogleShoppingItem.xml b/src/Resources/config/validation/GoogleShoppingItem.xml new file mode 100644 index 00000000..edf449cc --- /dev/null +++ b/src/Resources/config/validation/GoogleShoppingItem.xml @@ -0,0 +1,86 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Resources/translations/validators.en.yaml b/src/Resources/translations/validators.en.yaml index 6cfa3d25..efa7fe85 100644 --- a/src/Resources/translations/validators.en.yaml +++ b/src/Resources/translations/validators.en.yaml @@ -1,3 +1,21 @@ setono_sylius_feed: feed: resolve_required_fields_before_enabling: 'Resolve every field that requires input (set its source and clear the "requires input" flag) before enabling this feed.' + validation: + missing_field: 'The required field is missing or empty.' + google_shopping_item: + id: + not_blank: 'The item id (g:id) must not be blank.' + title: + not_blank: 'The item title (g:title) must not be blank.' + max_length: 'The item title (g:title) must not be longer than {{ limit }} characters.' + description: + not_blank: 'The item description (g:description) must not be blank.' + link: + not_blank: 'The item link (g:link) must not be blank.' + image_link: + not_blank: 'The item image link (g:image_link) must not be blank.' + availability: + not_blank: 'The item availability (g:availability) must not be blank.' + price: + not_blank: 'The item price (g:price) must not be blank.' diff --git a/src/Validator/FeedItemValidator.php b/src/Validator/FeedItemValidator.php new file mode 100644 index 00000000..6019a1b1 --- /dev/null +++ b/src/Validator/FeedItemValidator.php @@ -0,0 +1,44 @@ +validator->validate($item, null, $validationGroups) as $violation) { + $template = $violation->getMessageTemplate(); + $messages[] = '' !== $template ? $template : (string) $violation->getMessage(); + } + } + + foreach ($this->requiredFieldsValidator->findMissingFields($item, $requiredFields) as $field) { + $messages[] = 'setono_sylius_feed.validation.missing_field: ' . $field; + } + + return array_values(array_unique($messages)); + } +} diff --git a/src/Validator/FeedItemValidatorInterface.php b/src/Validator/FeedItemValidatorInterface.php new file mode 100644 index 00000000..2d3fb589 --- /dev/null +++ b/src/Validator/FeedItemValidatorInterface.php @@ -0,0 +1,27 @@ + $requiredFields + * @param list $validationGroups + * + * @return list the (deduplicated) violation messages / translation keys; empty when valid + */ + public function validate(FeedItem $item, array $requiredFields, array $validationGroups = ['Default']): array; +} diff --git a/tests/Functional/FeedContextResultTest.php b/tests/Functional/FeedContextResultTest.php new file mode 100644 index 00000000..c3121b1e --- /dev/null +++ b/tests/Functional/FeedContextResultTest.php @@ -0,0 +1,96 @@ +persistFeed(); + $recorder = $this->recorder(); + + $recorder->record($feed, 'web_en_US_USD', new GenerationResult( + 'google/web_en_US_USD.xml', + 5, + 2, + 512, + [['item' => 'SKU-9', 'reason' => 'validation:setono_sylius_feed.google_shopping_item.title.not_blank']], + )); + + // a later run for the same context supersedes the first + $recorder->record($feed, 'web_en_US_USD', new GenerationResult('google/web_en_US_USD.xml', 7, 0, 700, [])); + + // a different context is stored independently + $recorder->record($feed, 'web_da_DK_DKK', new GenerationResult('google/web_da_DK_DKK.xml', 1, 0, 64, [])); + + $repository = $this->repository(); + + $latest = $repository->findLatestPublished($feed, 'web_en_US_USD'); + self::assertInstanceOf(FeedContextResultInterface::class, $latest); + self::assertSame(7, $latest->getItemCount()); + self::assertSame(0, $latest->getExcludedCount()); + self::assertSame(700, $latest->getBytes()); + self::assertSame([], $latest->getErrors()); + self::assertSame($feed->getId(), $latest->getFeed()?->getId()); + + $otherContext = $repository->findLatestPublished($feed, 'web_da_DK_DKK'); + self::assertInstanceOf(FeedContextResultInterface::class, $otherContext); + self::assertSame(1, $otherContext->getItemCount()); + + self::assertNull($repository->findLatestPublished($feed, 'does_not_exist')); + } + + private function persistFeed(): FeedInterface + { + $feed = new Feed(); + $feed->setCode('feed-context-result-' . bin2hex(random_bytes(4))); + + $manager = $this->entityManager(); + $manager->persist($feed); + $manager->flush(); + + return $feed; + } + + private function recorder(): FeedContextResultRecorderInterface + { + $recorder = self::getContainer()->get(FeedContextResultRecorderInterface::class); + self::assertInstanceOf(FeedContextResultRecorderInterface::class, $recorder); + + return $recorder; + } + + private function repository(): FeedContextResultRepositoryInterface + { + $repository = self::getContainer()->get('setono_sylius_feed.repository.feed_context_result'); + self::assertInstanceOf(FeedContextResultRepositoryInterface::class, $repository); + + return $repository; + } + + private function entityManager(): EntityManagerInterface + { + $manager = self::getContainer()->get('doctrine.orm.entity_manager'); + self::assertInstanceOf(EntityManagerInterface::class, $manager); + + return $manager; + } +} diff --git a/tests/Unit/Generator/FeedGeneratorFilterTest.php b/tests/Unit/Generator/FeedGeneratorFilterTest.php index 509a1089..246f6b4e 100644 --- a/tests/Unit/Generator/FeedGeneratorFilterTest.php +++ b/tests/Unit/Generator/FeedGeneratorFilterTest.php @@ -20,6 +20,7 @@ use Setono\SyliusFeedPlugin\Format\FormatRegistryInterface; use Setono\SyliusFeedPlugin\Generator\FeedGenerator; use Setono\SyliusFeedPlugin\Generator\FieldMappingEvaluator; +use Setono\SyliusFeedPlugin\Item\FeedItem; use Setono\SyliusFeedPlugin\Lookup\InMemoryLookup; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\FieldType; @@ -40,6 +41,7 @@ use Setono\SyliusFeedPlugin\Scripting\SandboxedTwigRenderer; use Setono\SyliusFeedPlugin\Transformation\TransformationChain; use Setono\SyliusFeedPlugin\Transformation\TransformationRegistry; +use Setono\SyliusFeedPlugin\Validator\FeedItemValidator; use Setono\SyliusFeedPlugin\Validator\RequiredFieldsValidator; use Setono\SyliusFeedPlugin\ValueResolver\ValueResolverInterface; use Setono\SyliusFeedPlugin\Writer\CsvWriter; @@ -47,6 +49,7 @@ use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RequestContext; +use Symfony\Component\Validator\Validation; /** * The §11 acceptance for the FilterEvaluator wired into the generator: a source with an @@ -82,6 +85,13 @@ public function it_excludes_the_out_of_stock_item(string $stage): void self::assertSame(1, $result->itemCount); self::assertSame(1, $result->excludedCount); + // the exclusion is recorded with the stage-qualified filter reason (§11); the item id is + // only resolvable once mapping has run, so it is present for a `post` filter but null for `pre` + self::assertCount(1, $result->errors); + self::assertSame(sprintf('filter:%s:availability', $stage), $result->errors[0]['reason']); + self::assertSame(FeedFilterInterface::STAGE_POST === $stage ? 'SKU-2' : null, $result->errors[0]['item']); + self::assertGreaterThan(0, $result->bytes, 'the produced file has a non-zero byte size'); + $rows = [...Reader::createFromString($this->filesystem->read($result->path))->getRecords()]; self::assertCount(2, $rows, 'header + the single kept row'); self::assertSame(['id', 'availability'], $rows[0]); @@ -128,7 +138,7 @@ private function createGenerator(): FeedGenerator ), new FilterEvaluator(new ReferenceResolver(), new OperatorRegistry([new Equals()])), new NullLookupReferenceResolver(), - new RequiredFieldsValidator(), + new FeedItemValidator(Validation::createValidator(), new RequiredFieldsValidator()), new EventDispatcher(), $urlGenerator->reveal(), $this->filesystem, @@ -185,6 +195,11 @@ public function getDataSource(): DataSourceInterface return $this->dataSource; } + public function createItem(object $entity, FeedContext $context): FeedItem + { + return new FeedItem($entity, $context); + } + public function getScopeDimensions(): array { return [ScopeDimension::CHANNEL, ScopeDimension::LOCALE, ScopeDimension::CURRENCY]; diff --git a/tests/Unit/Generator/FeedGeneratorMemoryTest.php b/tests/Unit/Generator/FeedGeneratorMemoryTest.php index 2567afe9..b92746ee 100644 --- a/tests/Unit/Generator/FeedGeneratorMemoryTest.php +++ b/tests/Unit/Generator/FeedGeneratorMemoryTest.php @@ -19,6 +19,7 @@ use Setono\SyliusFeedPlugin\Format\GoogleRssFormat; use Setono\SyliusFeedPlugin\Generator\FeedGenerator; use Setono\SyliusFeedPlugin\Generator\FieldMappingEvaluator; +use Setono\SyliusFeedPlugin\Item\FeedItem; use Setono\SyliusFeedPlugin\Lookup\InMemoryLookup; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\FieldType; @@ -38,12 +39,14 @@ use Setono\SyliusFeedPlugin\Transformation\TransformationChain; use Setono\SyliusFeedPlugin\Transformation\TransformationRegistry; use Setono\SyliusFeedPlugin\Transformation\Truncate; +use Setono\SyliusFeedPlugin\Validator\FeedItemValidator; use Setono\SyliusFeedPlugin\Validator\RequiredFieldsValidator; use Setono\SyliusFeedPlugin\Writer\FeedWriterRegistryInterface; use Setono\SyliusFeedPlugin\Writer\XmlWriter; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RequestContext; +use Symfony\Component\Validator\Validation; /** * Performance budget (§6.3, M1 acceptance): a 50,000-item feed must stream to storage without @@ -109,7 +112,7 @@ private function createGenerator(Filesystem $filesystem): FeedGenerator ), new FilterEvaluator(new ReferenceResolver(), new OperatorRegistry([])), new NullLookupReferenceResolver(), - new RequiredFieldsValidator(), + new FeedItemValidator(Validation::createValidator(), new RequiredFieldsValidator()), new EventDispatcher(), $urlGenerator->reveal(), $filesystem, @@ -172,6 +175,11 @@ public function getDataSource(): DataSourceInterface return $this->dataSource; } + public function createItem(object $entity, FeedContext $context): FeedItem + { + return new FeedItem($entity, $context); + } + public function getScopeDimensions(): array { return [ScopeDimension::CHANNEL, ScopeDimension::LOCALE, ScopeDimension::CURRENCY]; diff --git a/tests/Unit/Generator/FeedGeneratorTest.php b/tests/Unit/Generator/FeedGeneratorTest.php index 19163148..2b673eb5 100644 --- a/tests/Unit/Generator/FeedGeneratorTest.php +++ b/tests/Unit/Generator/FeedGeneratorTest.php @@ -22,6 +22,7 @@ use Setono\SyliusFeedPlugin\Format\PartnerAdsFormat; use Setono\SyliusFeedPlugin\Generator\FeedGenerator; use Setono\SyliusFeedPlugin\Generator\FieldMappingEvaluator; +use Setono\SyliusFeedPlugin\Item\FeedItem; use Setono\SyliusFeedPlugin\Lookup\InMemoryLookup; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\FieldType; @@ -47,6 +48,7 @@ use Setono\SyliusFeedPlugin\Transformation\TransformationRegistry; use Setono\SyliusFeedPlugin\Transformation\Truncate; use Setono\SyliusFeedPlugin\Transformation\ValueMap; +use Setono\SyliusFeedPlugin\Validator\FeedItemValidator; use Setono\SyliusFeedPlugin\Validator\RequiredFieldsValidator; use Setono\SyliusFeedPlugin\Writer\CsvWriter; use Setono\SyliusFeedPlugin\Writer\FeedWriterRegistryInterface; @@ -55,6 +57,7 @@ use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RequestContext; +use Symfony\Component\Validator\Validation; /** * Acceptance test for the M1 engine: generating a Google Shopping feed for a context produces a @@ -269,7 +272,7 @@ private function createGenerator(?array $presets = null): FeedGenerator ), new FilterEvaluator(new ReferenceResolver(), new OperatorRegistry([new Equals()])), new NullLookupReferenceResolver(), - new RequiredFieldsValidator(), + new FeedItemValidator(Validation::createValidator(), new RequiredFieldsValidator()), new EventDispatcher(), $urlGenerator->reveal(), $this->filesystem, @@ -336,6 +339,11 @@ public function getDataSource(): DataSourceInterface return $this->dataSource; } + public function createItem(object $entity, FeedContext $context): FeedItem + { + return new FeedItem($entity, $context); + } + public function getScopeDimensions(): array { return [ScopeDimension::CHANNEL, ScopeDimension::LOCALE, ScopeDimension::CURRENCY]; diff --git a/tests/Unit/Model/FeedContextResultTest.php b/tests/Unit/Model/FeedContextResultTest.php new file mode 100644 index 00000000..c7b58f5b --- /dev/null +++ b/tests/Unit/Model/FeedContextResultTest.php @@ -0,0 +1,65 @@ +prophesize(FeedInterface::class)->reveal(); + $createdAt = new \DateTimeImmutable('2026-07-02 10:00:00'); + + $result = new FeedContextResult(); + $result->setFeed($feed); + $result->setContextKey('web_en_US_USD'); + $result->setItemCount(42); + $result->setExcludedCount(3); + $result->setBytes(1024); + $result->setErrors([['item' => 'SKU-1', 'reason' => 'validation:missing title']]); + $result->setCreatedAt($createdAt); + + self::assertNull($result->getId()); + self::assertSame($feed, $result->getFeed()); + self::assertSame('web_en_US_USD', $result->getContextKey()); + self::assertSame(42, $result->getItemCount()); + self::assertSame(3, $result->getExcludedCount()); + self::assertSame(1024, $result->getBytes()); + self::assertSame([['item' => 'SKU-1', 'reason' => 'validation:missing title']], $result->getErrors()); + self::assertSame($createdAt, $result->getCreatedAt()); + } + + /** + * @test + */ + public function it_defaults_created_at_to_now(): void + { + self::assertEqualsWithDelta(time(), (new FeedContextResult())->getCreatedAt()->getTimestamp(), 5); + } + + /** + * @test + */ + public function it_appends_errors(): void + { + $result = new FeedContextResult(); + $result->addError('SKU-1', 'filter:pre:availability'); + $result->addError(null, 'skipped'); + + self::assertSame([ + ['item' => 'SKU-1', 'reason' => 'filter:pre:availability'], + ['item' => null, 'reason' => 'skipped'], + ], $result->getErrors()); + } +} diff --git a/tests/Unit/Validator/FeedItemValidatorTest.php b/tests/Unit/Validator/FeedItemValidatorTest.php new file mode 100644 index 00000000..170a8479 --- /dev/null +++ b/tests/Unit/Validator/FeedItemValidatorTest.php @@ -0,0 +1,146 @@ +symfonyValidator(), new RequiredFieldsValidator()); + } + + private function symfonyValidator(): ValidatorInterface + { + return Validation::createValidatorBuilder() + ->addXmlMapping(__DIR__ . '/../../../src/Resources/config/validation/GoogleShoppingItem.xml') + ->getValidator() + ; + } + + private function context(): FeedContext + { + return new FeedContext(null, 'en_US', 'USD'); + } + + private function validGoogleShoppingItem(): GoogleShoppingItem + { + $item = new GoogleShoppingItem(new \stdClass(), $this->context()); + $item->setId('SKU-1'); + $item->setTitle('Acme Shoe'); + $item->setDescription('Very nice'); + $item->setLink('https://example.com/products/acme-shoe'); + $item->setImageLink('https://example.com/img/main.jpg'); + $item->setAvailability(Availability::IN_STOCK); + $item->setPrice('9.99 USD'); + + return $item; + } + + /** + * @test + */ + public function it_reports_a_google_shopping_item_missing_its_title(): void + { + $item = $this->validGoogleShoppingItem(); + $item->remove(GoogleShoppingItem::TITLE); + + $messages = $this->feedItemValidator()->validate($item, []); + + self::assertContains('setono_sylius_feed.google_shopping_item.title.not_blank', $messages); + } + + /** + * @test + */ + public function it_reports_a_google_shopping_item_with_a_too_long_title(): void + { + $item = $this->validGoogleShoppingItem(); + $item->setTitle(str_repeat('a', 151)); + + $messages = $this->feedItemValidator()->validate($item, []); + + self::assertContains('setono_sylius_feed.google_shopping_item.title.max_length', $messages); + } + + /** + * @test + */ + public function it_considers_a_fully_populated_google_shopping_item_valid(): void + { + self::assertSame([], $this->feedItemValidator()->validate($this->validGoogleShoppingItem(), [])); + } + + /** + * @test + */ + public function it_reports_a_missing_required_field_on_a_plain_feed_item_via_the_fallback(): void + { + $item = new FeedItem(new \stdClass(), $this->context()); + $item->set('g:id', 'SKU-1'); + + $messages = $this->feedItemValidator()->validate($item, ['g:id', 'g:title']); + + self::assertSame(['setono_sylius_feed.validation.missing_field: g:title'], $messages); + } + + /** + * @test + */ + public function it_skips_the_typed_constraints_when_the_format_opts_out(): void + { + // an empty validation-group list (a non-Google format) must not run the g:* constraints, + // even though the item is a GoogleShoppingItem lacking every field + $item = new GoogleShoppingItem(new \stdClass(), $this->context()); + + self::assertSame([], $this->feedItemValidator()->validate($item, [], [])); + } + + /** + * @test + */ + public function it_still_runs_the_required_field_fallback_when_the_typed_constraints_are_skipped(): void + { + $item = new GoogleShoppingItem(new \stdClass(), $this->context()); + + self::assertSame( + ['setono_sylius_feed.validation.missing_field: g:id'], + $this->feedItemValidator()->validate($item, ['g:id'], []), + ); + } + + /** + * @test + */ + public function it_deduplicates_messages(): void + { + $item = $this->validGoogleShoppingItem(); + $item->remove(GoogleShoppingItem::TITLE); + + // both the typed NotBlank constraint and the required-field fallback flag g:title, but the + // fallback message key differs, so the two are distinct; the typed message appears once + $messages = $this->feedItemValidator()->validate($item, ['g:title']); + + self::assertSame( + $messages, + array_values(array_unique($messages)), + 'The returned messages must be deduplicated', + ); + } +} From e2ce9507a44f00c0fa95b09e88d03316c41b8925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Thu, 2 Jul 2026 16:49:03 +0200 Subject: [PATCH 3/4] M6: publish gate + guardrails (per-context gated promotion) - Feed.publishConfig (json guardrails); GuardrailRegistry + guardrails (min_items, max_drop_pct, non_empty, min_bytes, max_exclusion_pct, max_growth_pct) comparing candidate vs the last-good FeedContextResult baseline. - PublishGate returns a decision (blocked + reasons); block-severity trips block, warn only records; empty guardrails = no gate. - FeedContextResult gains publishState (pending/published/blocked) + publishCheck; findLatestPublished filters published so a candidate is never its own baseline. - Finalize is now per-context + gated: GenerateFeedContextHandler runs the gate after recording, stamps publishState, dispatches FeedPublishBlockedEvent when blocked/warned; MoveGeneratedFeedSubscriber no longer wipes canonical -- it promotes only published contexts and retains blocked candidates in staging. - Publish-anyway admin action promotes a retained blocked candidate. Acceptance: a 5k-vs-50k regenerate trips max_drop_pct -> blocked, live file retained, event fired. +44 tests (567 total); all gates (incl. PHP 8.1) green. --- .../Admin/PublishContextAnywayAction.php | 79 ++++++++ .../SetonoSyliusFeedExtension.php | 2 + src/Event/FeedPublishBlockedEvent.php | 27 +++ .../MoveGeneratedFeedSubscriber.php | 28 ++- .../GenerateFeedContextHandler.php | 50 ++++- src/Model/Feed.php | 13 ++ src/Model/FeedContextResult.php | 35 ++++ src/Model/FeedContextResultInterface.php | 39 ++++ src/Model/FeedInterface.php | 14 ++ src/Publish/AbstractGuardrail.php | 32 ++++ src/Publish/GuardrailInterface.php | 34 ++++ src/Publish/GuardrailRegistry.php | 26 +++ src/Publish/GuardrailRegistryInterface.php | 15 ++ src/Publish/MaxDropPctGuardrail.php | 44 +++++ src/Publish/MaxExclusionPctGuardrail.php | 51 ++++++ src/Publish/MaxGrowthPctGuardrail.php | 44 +++++ src/Publish/MinBytesGuardrail.php | 27 +++ src/Publish/MinItemsGuardrail.php | 27 +++ src/Publish/NonEmptyGuardrail.php | 27 +++ src/Publish/PublishDecision.php | 22 +++ src/Publish/PublishGate.php | 85 +++++++++ src/Publish/PublishGateInterface.php | 24 +++ .../FeedContextResultRepository.php | 18 ++ .../FeedContextResultRepositoryInterface.php | 13 +- .../config/doctrine/model/Feed.orm.xml | 1 + .../doctrine/model/FeedContextResult.orm.xml | 2 + src/Resources/config/routes/admin.yaml | 8 + src/Resources/config/services.xml | 1 + src/Resources/config/services/controller.xml | 10 + .../config/services/event_subscriber.xml | 1 + src/Resources/config/services/messenger.xml | 3 + src/Resources/config/services/publish.xml | 31 ++++ src/Resources/config/services/registry.xml | 5 + src/Resources/translations/flashes.en.yaml | 1 + tests/Functional/FeedContextResultTest.php | 74 +++++--- .../MessageHandler/PublishGateFlowTest.php | 172 ++++++++++++++++++ .../Admin/PublishContextAnywayActionTest.php | 128 +++++++++++++ .../MoveGeneratedFeedSubscriberTest.php | 57 ++++-- tests/Unit/Model/FeedContextResultTest.php | 34 ++++ tests/Unit/Model/FeedTest.php | 3 + tests/Unit/Publish/GuardrailRegistryTest.php | 41 +++++ .../Unit/Publish/MaxDropPctGuardrailTest.php | 73 ++++++++ .../Publish/MaxExclusionPctGuardrailTest.php | 77 ++++++++ .../Publish/MaxGrowthPctGuardrailTest.php | 73 ++++++++ tests/Unit/Publish/MinBytesGuardrailTest.php | 47 +++++ tests/Unit/Publish/MinItemsGuardrailTest.php | 55 ++++++ tests/Unit/Publish/NonEmptyGuardrailTest.php | 47 +++++ tests/Unit/Publish/PublishGateTest.php | 162 +++++++++++++++++ 48 files changed, 1826 insertions(+), 56 deletions(-) create mode 100644 src/Controller/Admin/PublishContextAnywayAction.php create mode 100644 src/Event/FeedPublishBlockedEvent.php create mode 100644 src/Publish/AbstractGuardrail.php create mode 100644 src/Publish/GuardrailInterface.php create mode 100644 src/Publish/GuardrailRegistry.php create mode 100644 src/Publish/GuardrailRegistryInterface.php create mode 100644 src/Publish/MaxDropPctGuardrail.php create mode 100644 src/Publish/MaxExclusionPctGuardrail.php create mode 100644 src/Publish/MaxGrowthPctGuardrail.php create mode 100644 src/Publish/MinBytesGuardrail.php create mode 100644 src/Publish/MinItemsGuardrail.php create mode 100644 src/Publish/NonEmptyGuardrail.php create mode 100644 src/Publish/PublishDecision.php create mode 100644 src/Publish/PublishGate.php create mode 100644 src/Publish/PublishGateInterface.php create mode 100644 src/Resources/config/services/publish.xml create mode 100644 tests/Functional/MessageHandler/PublishGateFlowTest.php create mode 100644 tests/Unit/Controller/Admin/PublishContextAnywayActionTest.php create mode 100644 tests/Unit/Publish/GuardrailRegistryTest.php create mode 100644 tests/Unit/Publish/MaxDropPctGuardrailTest.php create mode 100644 tests/Unit/Publish/MaxExclusionPctGuardrailTest.php create mode 100644 tests/Unit/Publish/MaxGrowthPctGuardrailTest.php create mode 100644 tests/Unit/Publish/MinBytesGuardrailTest.php create mode 100644 tests/Unit/Publish/MinItemsGuardrailTest.php create mode 100644 tests/Unit/Publish/NonEmptyGuardrailTest.php create mode 100644 tests/Unit/Publish/PublishGateTest.php diff --git a/src/Controller/Admin/PublishContextAnywayAction.php b/src/Controller/Admin/PublishContextAnywayAction.php new file mode 100644 index 00000000..d4a81519 --- /dev/null +++ b/src/Controller/Admin/PublishContextAnywayAction.php @@ -0,0 +1,79 @@ +managerRegistry = $managerRegistry; + } + + public function __invoke(Request $request, int|string $id, string $contextKey): Response + { + $feed = $this->feedRepository->find($id); + if (!$feed instanceof FeedInterface) { + throw new NotFoundHttpException(sprintf('There is no feed with id "%s"', $id)); + } + + $directory = (string) $feed->getCode(); + + foreach ($this->feedTmpFilesystem->listContents($directory, true) as $item) { + if (!$item->isFile()) { + continue; + } + + $path = $item->path(); + if (pathinfo($path, \PATHINFO_FILENAME) !== $contextKey) { + continue; + } + + $this->feedFilesystem->writeStream($path, $this->feedTmpFilesystem->readStream($path)); + $this->feedTmpFilesystem->delete($path); + } + + $result = $this->feedContextResultRepository->findLatestForContext($feed, $contextKey); + if (null !== $result) { + $result->setPublishState(FeedContextResultInterface::PUBLISH_STATE_PUBLISHED); + $this->getManager($result)->flush(); + } + + $session = $request->getSession(); + if ($session instanceof Session) { + $session->getFlashBag()->add('success', 'setono_sylius_feed.feed.context_published'); + } + + return new RedirectResponse( + $this->router->generate('setono_sylius_feed_admin_feed_results', ['id' => $feed->getId()]), + ); + } +} diff --git a/src/DependencyInjection/SetonoSyliusFeedExtension.php b/src/DependencyInjection/SetonoSyliusFeedExtension.php index 5bbbc56d..8c5abe50 100644 --- a/src/DependencyInjection/SetonoSyliusFeedExtension.php +++ b/src/DependencyInjection/SetonoSyliusFeedExtension.php @@ -10,6 +10,7 @@ use Setono\SyliusFeedPlugin\MappingPreset\MappingPresetInterface; use Setono\SyliusFeedPlugin\Model\FeedInterface; use Setono\SyliusFeedPlugin\Operator\OperatorInterface; +use Setono\SyliusFeedPlugin\Publish\GuardrailInterface; use Setono\SyliusFeedPlugin\Transformation\TransformationInterface; use Setono\SyliusFeedPlugin\ValueResolver\ValueResolverInterface; use Setono\SyliusFeedPlugin\Workflow\FeedGraph; @@ -37,6 +38,7 @@ final class SetonoSyliusFeedExtension extends AbstractResourceExtension implemen OperatorInterface::class => 'setono_sylius_feed.operator', FeedWriterInterface::class => 'setono_sylius_feed.writer', FormatInterface::class => 'setono_sylius_feed.format', + GuardrailInterface::class => 'setono_sylius_feed.guardrail', ]; public function load(array $configs, ContainerBuilder $container): void diff --git a/src/Event/FeedPublishBlockedEvent.php b/src/Event/FeedPublishBlockedEvent.php new file mode 100644 index 00000000..60a64f99 --- /dev/null +++ b/src/Event/FeedPublishBlockedEvent.php @@ -0,0 +1,27 @@ + $reasons + */ + public function __construct( + public readonly FeedInterface $feed, + public readonly string $contextKey, + public readonly array $reasons, + public readonly bool $blocked, + ) { + } +} diff --git a/src/EventSubscriber/MoveGeneratedFeedSubscriber.php b/src/EventSubscriber/MoveGeneratedFeedSubscriber.php index c4a820e1..1925b907 100644 --- a/src/EventSubscriber/MoveGeneratedFeedSubscriber.php +++ b/src/EventSubscriber/MoveGeneratedFeedSubscriber.php @@ -6,20 +6,25 @@ use League\Flysystem\FilesystemOperator; use Setono\SyliusFeedPlugin\Model\FeedInterface; +use Setono\SyliusFeedPlugin\Repository\FeedContextResultRepositoryInterface; use Setono\SyliusFeedPlugin\Workflow\FeedGraph; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Workflow\Event\CompletedEvent; /** - * On `complete`, swaps the feed's freshly generated files from temporary to canonical storage so - * the public feed is never served half-written (§6.3): the canonical directory is replaced with the - * temporary one, then the temporary copy is removed and the feed is stamped as generated. + * On `complete`, promotes each freshly generated context file from staging to canonical storage — + * but only per-context and only when the publish gate let it through (§6.6). A context whose latest + * result is `blocked` keeps its previously published canonical file live and its candidate is + * retained in staging for inspection (and a possible "publish anyway"); every other context is + * copied across (overwriting) and its staging copy removed. The canonical directory is never wiped + * wholesale, so a blocked context never loses its live file. */ final class MoveGeneratedFeedSubscriber implements EventSubscriberInterface { public function __construct( private readonly FilesystemOperator $feedTmpFilesystem, private readonly FilesystemOperator $feedFilesystem, + private readonly FeedContextResultRepositoryInterface $feedContextResultRepository, ) { } @@ -39,17 +44,24 @@ public function onCompleted(CompletedEvent $event): void $directory = (string) $feed->getCode(); - $this->feedFilesystem->deleteDirectory($directory); - foreach ($this->feedTmpFilesystem->listContents($directory, true) as $item) { if (!$item->isFile()) { continue; } - $this->feedFilesystem->writeStream($item->path(), $this->feedTmpFilesystem->readStream($item->path())); - } + $path = $item->path(); + $contextKey = pathinfo($path, \PATHINFO_FILENAME); + $result = $this->feedContextResultRepository->findLatestForContext($feed, $contextKey); + + if (null !== $result && $result->isBlocked()) { + // Keep the live canonical file; retain the blocked candidate in staging for inspection. + continue; + } - $this->feedTmpFilesystem->deleteDirectory($directory); + // Published (or no result / no gate): swap the candidate in and drop the staging copy. + $this->feedFilesystem->writeStream($path, $this->feedTmpFilesystem->readStream($path)); + $this->feedTmpFilesystem->delete($path); + } $feed->setLastGeneratedAt(new \DateTimeImmutable()); } diff --git a/src/MessageHandler/GenerateFeedContextHandler.php b/src/MessageHandler/GenerateFeedContextHandler.php index b34ad13a..15f69361 100644 --- a/src/MessageHandler/GenerateFeedContextHandler.php +++ b/src/MessageHandler/GenerateFeedContextHandler.php @@ -7,21 +7,28 @@ use Doctrine\Persistence\ManagerRegistry; use Setono\Doctrine\ORMTrait; use Setono\SyliusFeedPlugin\Context\FeedContext; +use Setono\SyliusFeedPlugin\Event\FeedPublishBlockedEvent; use Setono\SyliusFeedPlugin\Generator\FeedContextResultRecorderInterface; use Setono\SyliusFeedPlugin\Generator\FeedGeneratorInterface; use Setono\SyliusFeedPlugin\Message\Command\GenerateFeedContext; +use Setono\SyliusFeedPlugin\Model\FeedContextResultInterface; use Setono\SyliusFeedPlugin\Model\FeedInterface; +use Setono\SyliusFeedPlugin\Publish\PublishGateInterface; +use Setono\SyliusFeedPlugin\Repository\FeedContextResultRepositoryInterface; use Setono\SyliusFeedPlugin\Repository\FeedRepositoryInterface; use Setono\SyliusFeedPlugin\Workflow\FeedGraph; use Sylius\Component\Core\Model\ChannelInterface; use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\Workflow\Registry; +use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; use Webmozart\Assert\Assert; /** - * Generates one context's feed file to temporary storage, then atomically counts it as completed; - * the handler that finishes the last context completes the feed (§6.3). Because the generator - * clears the entity manager while streaming, the completion bookkeeping re-fetches the feed. + * Generates one context's feed file to temporary (staging) storage, records the candidate result, + * then runs it through the publish gate (§6.6) to decide whether it may be promoted to the live feed + * (the promotion itself happens per-context on `complete`). Finally it counts the context as + * completed; the handler that finishes the last context completes the feed (§6.3). Because the + * generator clears the entity manager while streaming, the bookkeeping re-fetches the feed. */ final class GenerateFeedContextHandler { @@ -33,6 +40,9 @@ public function __construct( private readonly RepositoryInterface $channelRepository, private readonly FeedGeneratorInterface $feedGenerator, private readonly FeedContextResultRecorderInterface $feedContextResultRecorder, + private readonly FeedContextResultRepositoryInterface $feedContextResultRepository, + private readonly PublishGateInterface $publishGate, + private readonly EventDispatcherInterface $eventDispatcher, private readonly Registry $workflowRegistry, ) { $this->managerRegistry = $managerRegistry; @@ -61,8 +71,11 @@ public function __invoke(GenerateFeedContext $message): void Assert::isInstanceOf($feed, FeedInterface::class); // Record the excluded-item report + size/count for this context now that generation - // finished, against the managed feed (§11). - $this->feedContextResultRecorder->record($feed, $context->key(), $result); + // finished, against the managed feed (§11). The candidate starts out `pending`. + $contextKey = $context->key(); + $candidate = $this->feedContextResultRecorder->record($feed, $contextKey, $result); + + $this->gate($feed, $contextKey, $candidate); $completed = $this->feedRepository->incrementCompletedContexts($feed); if (null !== $feed->getContextCount() && $completed >= $feed->getContextCount()) { @@ -70,6 +83,33 @@ public function __invoke(GenerateFeedContext $message): void } } + /** + * Runs the freshly recorded candidate through the publish gate against the last published + * baseline (§6.6), stamps the outcome onto the candidate, and notifies listeners when the gate + * blocked promotion or a `warn` guardrail flagged a concern. + */ + private function gate(FeedInterface $feed, string $contextKey, FeedContextResultInterface $candidate): void + { + $baseline = $this->feedContextResultRepository->findLatestPublished($feed, $contextKey); + + $guardrails = $feed->getPublishConfig()['guardrails'] ?? []; + $decision = $this->publishGate->evaluate($candidate, $baseline, is_array($guardrails) ? $guardrails : []); + + $candidate->setPublishState( + $decision->blocked + ? FeedContextResultInterface::PUBLISH_STATE_BLOCKED + : FeedContextResultInterface::PUBLISH_STATE_PUBLISHED, + ); + $candidate->setPublishCheck([] === $decision->reasons ? null : $decision->reasons); + $this->getManager($candidate)->flush(); + + if ($decision->blocked || [] !== $decision->reasons) { + $this->eventDispatcher->dispatch( + new FeedPublishBlockedEvent($feed, $contextKey, $decision->reasons, $decision->blocked), + ); + } + } + private function buildContext(GenerateFeedContext $message): FeedContext { $channel = null; diff --git a/src/Model/Feed.php b/src/Model/Feed.php index abb9e4cd..d9caf8f1 100644 --- a/src/Model/Feed.php +++ b/src/Model/Feed.php @@ -30,6 +30,9 @@ class Feed implements FeedInterface /** @var array */ protected array $formatConfig = []; + /** @var array */ + protected array $publishConfig = []; + /** @var Collection */ protected Collection $sources; @@ -143,6 +146,16 @@ public function setFormatConfig(array $formatConfig): void $this->formatConfig = $formatConfig; } + public function getPublishConfig(): array + { + return $this->publishConfig; + } + + public function setPublishConfig(array $publishConfig): void + { + $this->publishConfig = $publishConfig; + } + public function getSources(): Collection { return $this->sources; diff --git a/src/Model/FeedContextResult.php b/src/Model/FeedContextResult.php index 2f821fc0..f22091ae 100644 --- a/src/Model/FeedContextResult.php +++ b/src/Model/FeedContextResult.php @@ -21,6 +21,11 @@ class FeedContextResult implements FeedContextResultInterface /** @var list */ protected array $errors = []; + protected string $publishState = FeedContextResultInterface::PUBLISH_STATE_PENDING; + + /** @var list|null */ + protected ?array $publishCheck = null; + protected \DateTimeInterface $createdAt; public function __construct() @@ -98,6 +103,36 @@ public function addError(?string $item, string $reason): void $this->errors[] = ['item' => $item, 'reason' => $reason]; } + public function getPublishState(): string + { + return $this->publishState; + } + + public function setPublishState(string $publishState): void + { + $this->publishState = $publishState; + } + + public function getPublishCheck(): ?array + { + return $this->publishCheck; + } + + public function setPublishCheck(?array $publishCheck): void + { + $this->publishCheck = $publishCheck; + } + + public function isPublished(): bool + { + return FeedContextResultInterface::PUBLISH_STATE_PUBLISHED === $this->publishState; + } + + public function isBlocked(): bool + { + return FeedContextResultInterface::PUBLISH_STATE_BLOCKED === $this->publishState; + } + public function getCreatedAt(): \DateTimeInterface { return $this->createdAt; diff --git a/src/Model/FeedContextResultInterface.php b/src/Model/FeedContextResultInterface.php index 15cc0bb2..a3af6813 100644 --- a/src/Model/FeedContextResultInterface.php +++ b/src/Model/FeedContextResultInterface.php @@ -13,6 +13,22 @@ */ interface FeedContextResultInterface extends ResourceInterface { + /** + * The candidate has been recorded but not yet evaluated by the publish gate (§6.6). + */ + public const PUBLISH_STATE_PENDING = 'pending'; + + /** + * The candidate passed the publish gate and its file was promoted to canonical storage. + */ + public const PUBLISH_STATE_PUBLISHED = 'published'; + + /** + * The candidate tripped a `block`-severity guardrail; its file is retained in staging and the + * previously published canonical file is kept live. + */ + public const PUBLISH_STATE_BLOCKED = 'blocked'; + public function getId(): ?int; public function getFeed(): ?FeedInterface; @@ -55,6 +71,29 @@ public function setErrors(array $errors): void; public function addError(?string $item, string $reason): void; + /** + * The publish-gate outcome for this candidate (§6.6): one of the PUBLISH_STATE_* constants. + */ + public function getPublishState(): string; + + public function setPublishState(string $publishState): void; + + /** + * The reasons recorded by the publish gate when guardrails tripped, or null when none did. + * + * @return list|null + */ + public function getPublishCheck(): ?array; + + /** + * @param list|null $publishCheck + */ + public function setPublishCheck(?array $publishCheck): void; + + public function isPublished(): bool; + + public function isBlocked(): bool; + public function getCreatedAt(): \DateTimeInterface; public function setCreatedAt(\DateTimeInterface $createdAt): void; diff --git a/src/Model/FeedInterface.php b/src/Model/FeedInterface.php index c22b0821..4b9fc55b 100644 --- a/src/Model/FeedInterface.php +++ b/src/Model/FeedInterface.php @@ -52,6 +52,20 @@ public function getFormatConfig(): array; */ public function setFormatConfig(array $formatConfig): void; + /** + * The publish-gate configuration (§6.6): the guardrails evaluated before a freshly generated + * context is promoted from staging to canonical storage. Shape: + * {guardrails: list<{type: string, params: array, severity: 'block'|'warn'}>}. + * + * @return array + */ + public function getPublishConfig(): array; + + /** + * @param array $publishConfig + */ + public function setPublishConfig(array $publishConfig): void; + /** * @return Collection */ diff --git a/src/Publish/AbstractGuardrail.php b/src/Publish/AbstractGuardrail.php new file mode 100644 index 00000000..39e367f8 --- /dev/null +++ b/src/Publish/AbstractGuardrail.php @@ -0,0 +1,32 @@ + $params + */ + protected function intParam(array $params, string $key, int $default = 0): int + { + $value = $params[$key] ?? $default; + + return is_numeric($value) ? (int) $value : $default; + } + + /** + * @param array $params + */ + protected function floatParam(array $params, string $key, float $default = 0.0): float + { + $value = $params[$key] ?? $default; + + return is_numeric($value) ? (float) $value : $default; + } +} diff --git a/src/Publish/GuardrailInterface.php b/src/Publish/GuardrailInterface.php new file mode 100644 index 00000000..d2bf16cc --- /dev/null +++ b/src/Publish/GuardrailInterface.php @@ -0,0 +1,34 @@ + $params + * + * @return bool true when the guardrail is TRIPPED (the candidate looks wrong) + */ + public function evaluate( + FeedContextResultInterface $candidate, + ?FeedContextResultInterface $baseline, + array $params, + ): bool; +} diff --git a/src/Publish/GuardrailRegistry.php b/src/Publish/GuardrailRegistry.php new file mode 100644 index 00000000..36dfef8c --- /dev/null +++ b/src/Publish/GuardrailRegistry.php @@ -0,0 +1,26 @@ + + */ +final class GuardrailRegistry extends Registry implements GuardrailRegistryInterface +{ + public function get(string $type): GuardrailInterface + { + return $this->getByKey($type); + } + + /** + * @param GuardrailInterface $item + */ + protected function getKey(object $item): string + { + return $item->getType(); + } +} diff --git a/src/Publish/GuardrailRegistryInterface.php b/src/Publish/GuardrailRegistryInterface.php new file mode 100644 index 00000000..3fe2ce28 --- /dev/null +++ b/src/Publish/GuardrailRegistryInterface.php @@ -0,0 +1,15 @@ + + */ +interface GuardrailRegistryInterface extends RegistryInterface +{ + public function get(string $type): GuardrailInterface; +} diff --git a/src/Publish/MaxDropPctGuardrail.php b/src/Publish/MaxDropPctGuardrail.php new file mode 100644 index 00000000..a9f9eb0b --- /dev/null +++ b/src/Publish/MaxDropPctGuardrail.php @@ -0,0 +1,44 @@ +getItemCount(); + if ($baselineCount <= 0) { + return false; + } + + $candidateCount = $candidate->getItemCount(); + if ($candidateCount >= $baselineCount) { + return false; + } + + $dropPct = ($baselineCount - $candidateCount) / $baselineCount * 100; + + return $dropPct > $this->floatParam($params, 'pct'); + } +} diff --git a/src/Publish/MaxExclusionPctGuardrail.php b/src/Publish/MaxExclusionPctGuardrail.php new file mode 100644 index 00000000..8f501a33 --- /dev/null +++ b/src/Publish/MaxExclusionPctGuardrail.php @@ -0,0 +1,51 @@ +ratio($candidate); + if ($candidateRatio <= $this->floatParam($params, 'pct')) { + return false; + } + + if (null !== $baseline && $candidateRatio <= $this->ratio($baseline)) { + return false; + } + + return true; + } + + /** + * The excluded share of the run as a 0-100 percentage; 0 when nothing was processed. + */ + private function ratio(FeedContextResultInterface $result): float + { + $total = $result->getItemCount() + $result->getExcludedCount(); + if ($total <= 0) { + return 0.0; + } + + return $result->getExcludedCount() / $total * 100; + } +} diff --git a/src/Publish/MaxGrowthPctGuardrail.php b/src/Publish/MaxGrowthPctGuardrail.php new file mode 100644 index 00000000..fe46de4a --- /dev/null +++ b/src/Publish/MaxGrowthPctGuardrail.php @@ -0,0 +1,44 @@ +getItemCount(); + if ($baselineCount <= 0) { + return false; + } + + $candidateCount = $candidate->getItemCount(); + if ($candidateCount <= $baselineCount) { + return false; + } + + $growthPct = ($candidateCount - $baselineCount) / $baselineCount * 100; + + return $growthPct > $this->floatParam($params, 'pct'); + } +} diff --git a/src/Publish/MinBytesGuardrail.php b/src/Publish/MinBytesGuardrail.php new file mode 100644 index 00000000..b9d7e757 --- /dev/null +++ b/src/Publish/MinBytesGuardrail.php @@ -0,0 +1,27 @@ +getBytes() < $this->intParam($params, 'bytes'); + } +} diff --git a/src/Publish/MinItemsGuardrail.php b/src/Publish/MinItemsGuardrail.php new file mode 100644 index 00000000..c47e6309 --- /dev/null +++ b/src/Publish/MinItemsGuardrail.php @@ -0,0 +1,27 @@ +getItemCount() < $this->intParam($params, 'min'); + } +} diff --git a/src/Publish/NonEmptyGuardrail.php b/src/Publish/NonEmptyGuardrail.php new file mode 100644 index 00000000..5e77393d --- /dev/null +++ b/src/Publish/NonEmptyGuardrail.php @@ -0,0 +1,27 @@ +getItemCount(); + } +} diff --git a/src/Publish/PublishDecision.php b/src/Publish/PublishDecision.php new file mode 100644 index 00000000..62b3b46a --- /dev/null +++ b/src/Publish/PublishDecision.php @@ -0,0 +1,22 @@ + $reasons + */ + public function __construct( + public readonly bool $blocked, + public readonly array $reasons = [], + ) { + } +} diff --git a/src/Publish/PublishGate.php b/src/Publish/PublishGate.php new file mode 100644 index 00000000..ecab82b4 --- /dev/null +++ b/src/Publish/PublishGate.php @@ -0,0 +1,85 @@ +guardrailRegistry->has($type)) { + continue; + } + + $severity = $guardrail['severity'] ?? self::SEVERITY_BLOCK; + if (!is_string($severity)) { + $severity = self::SEVERITY_BLOCK; + } + + $tripped = $this->guardrailRegistry + ->get($type) + ->evaluate($candidate, $baseline, $this->toParams($guardrail['params'] ?? [])); + + if (!$tripped) { + continue; + } + + $reasons[] = sprintf('%s: guardrail tripped (severity: %s)', $type, $severity); + + if (self::SEVERITY_BLOCK === $severity) { + $blocked = true; + } + } + + return new PublishDecision($blocked, $reasons); + } + + /** + * Coerces an untrusted config value into a string-keyed params map for a guardrail. + * + * @return array + */ + private function toParams(mixed $value): array + { + if (!is_array($value)) { + return []; + } + + $params = []; + foreach ($value as $key => $item) { + $params[(string) $key] = $item; + } + + return $params; + } +} diff --git a/src/Publish/PublishGateInterface.php b/src/Publish/PublishGateInterface.php new file mode 100644 index 00000000..bcc5d488 --- /dev/null +++ b/src/Publish/PublishGateInterface.php @@ -0,0 +1,24 @@ + $guardrails + */ + public function evaluate( + FeedContextResultInterface $candidate, + ?FeedContextResultInterface $baseline, + array $guardrails, + ): PublishDecision; +} diff --git a/src/Repository/FeedContextResultRepository.php b/src/Repository/FeedContextResultRepository.php index 7ec487d9..e81ddec9 100644 --- a/src/Repository/FeedContextResultRepository.php +++ b/src/Repository/FeedContextResultRepository.php @@ -11,6 +11,24 @@ class FeedContextResultRepository extends EntityRepository implements FeedContextResultRepositoryInterface { public function findLatestPublished(FeedInterface $feed, string $contextKey): ?FeedContextResultInterface + { + $result = $this->createQueryBuilder('o') + ->andWhere('o.feed = :feed') + ->andWhere('o.contextKey = :contextKey') + ->andWhere('o.publishState = :publishState') + ->setParameter('feed', $feed) + ->setParameter('contextKey', $contextKey) + ->setParameter('publishState', FeedContextResultInterface::PUBLISH_STATE_PUBLISHED) + ->orderBy('o.createdAt', 'DESC') + ->addOrderBy('o.id', 'DESC') + ->setMaxResults(1) + ->getQuery() + ->getOneOrNullResult(); + + return $result instanceof FeedContextResultInterface ? $result : null; + } + + public function findLatestForContext(FeedInterface $feed, string $contextKey): ?FeedContextResultInterface { $result = $this->createQueryBuilder('o') ->andWhere('o.feed = :feed') diff --git a/src/Repository/FeedContextResultRepositoryInterface.php b/src/Repository/FeedContextResultRepositoryInterface.php index e90b25c8..10a75947 100644 --- a/src/Repository/FeedContextResultRepositoryInterface.php +++ b/src/Repository/FeedContextResultRepositoryInterface.php @@ -14,8 +14,17 @@ interface FeedContextResultRepositoryInterface extends RepositoryInterface { /** - * The most recent result recorded for the given feed and context key (§11) — the row the - * publish gate consults for the previously served output. Ordered by creation time, newest first. + * The most recent *published* result for the given feed and context key (§6.6) — the baseline + * the publish gate compares a new candidate against. Filters on + * {@see FeedContextResultInterface::PUBLISH_STATE_PUBLISHED} so a just-recorded pending candidate + * is never its own baseline. Ordered by creation time, newest first. */ public function findLatestPublished(FeedInterface $feed, string $contextKey): ?FeedContextResultInterface; + + /** + * The most recent result for the given feed and context key regardless of publish state (§6.6) — + * used by the promotion step to decide whether a staged file was published or blocked. Ordered + * by creation time, newest first. + */ + public function findLatestForContext(FeedInterface $feed, string $contextKey): ?FeedContextResultInterface; } diff --git a/src/Resources/config/doctrine/model/Feed.orm.xml b/src/Resources/config/doctrine/model/Feed.orm.xml index 2515ed1a..3019d8ad 100644 --- a/src/Resources/config/doctrine/model/Feed.orm.xml +++ b/src/Resources/config/doctrine/model/Feed.orm.xml @@ -15,6 +15,7 @@ + diff --git a/src/Resources/config/doctrine/model/FeedContextResult.orm.xml b/src/Resources/config/doctrine/model/FeedContextResult.orm.xml index 9a26c98c..daf1849f 100644 --- a/src/Resources/config/doctrine/model/FeedContextResult.orm.xml +++ b/src/Resources/config/doctrine/model/FeedContextResult.orm.xml @@ -19,6 +19,8 @@ + + diff --git a/src/Resources/config/routes/admin.yaml b/src/Resources/config/routes/admin.yaml index 342c09f0..1c4007e2 100644 --- a/src/Resources/config/routes/admin.yaml +++ b/src/Resources/config/routes/admin.yaml @@ -24,6 +24,14 @@ setono_sylius_feed_admin_feed_results: defaults: _controller: 'Setono\SyliusFeedPlugin\Controller\Admin\FeedResultsAction' +setono_sylius_feed_admin_feed_publish_context: + path: /feeds/{id}/publish/{contextKey} + methods: [GET] + defaults: + _controller: 'Setono\SyliusFeedPlugin\Controller\Admin\PublishContextAnywayAction' + requirements: + contextKey: '[^/]+' + setono_sylius_feed_admin_lookup_table: resource: | alias: setono_sylius_feed.lookup_table diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 65562545..015a9759 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -18,6 +18,7 @@ + diff --git a/src/Resources/config/services/controller.xml b/src/Resources/config/services/controller.xml index 3f5e636c..44dc1872 100644 --- a/src/Resources/config/services/controller.xml +++ b/src/Resources/config/services/controller.xml @@ -30,5 +30,15 @@ + + + + + + + + + + diff --git a/src/Resources/config/services/event_subscriber.xml b/src/Resources/config/services/event_subscriber.xml index c54bab41..6eac7647 100644 --- a/src/Resources/config/services/event_subscriber.xml +++ b/src/Resources/config/services/event_subscriber.xml @@ -7,6 +7,7 @@ + diff --git a/src/Resources/config/services/messenger.xml b/src/Resources/config/services/messenger.xml index 45be2613..ef6f6816 100644 --- a/src/Resources/config/services/messenger.xml +++ b/src/Resources/config/services/messenger.xml @@ -19,6 +19,9 @@ + + + diff --git a/src/Resources/config/services/publish.xml b/src/Resources/config/services/publish.xml new file mode 100644 index 00000000..777d738c --- /dev/null +++ b/src/Resources/config/services/publish.xml @@ -0,0 +1,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Resources/config/services/registry.xml b/src/Resources/config/services/registry.xml index ad052ca2..838d431a 100644 --- a/src/Resources/config/services/registry.xml +++ b/src/Resources/config/services/registry.xml @@ -43,5 +43,10 @@ + + + + + diff --git a/src/Resources/translations/flashes.en.yaml b/src/Resources/translations/flashes.en.yaml index 95b0400a..2d23d09d 100644 --- a/src/Resources/translations/flashes.en.yaml +++ b/src/Resources/translations/flashes.en.yaml @@ -1,5 +1,6 @@ setono_sylius_feed: feed: generation_dispatched: Feed generation has been dispatched + context_published: The blocked context has been published lookup_table: refreshed: Lookup table has been refreshed diff --git a/tests/Functional/FeedContextResultTest.php b/tests/Functional/FeedContextResultTest.php index c3121b1e..8f4cf45b 100644 --- a/tests/Functional/FeedContextResultTest.php +++ b/tests/Functional/FeedContextResultTest.php @@ -13,49 +13,67 @@ use Setono\SyliusFeedPlugin\Repository\FeedContextResultRepositoryInterface; /** - * Proves the FeedContextResult resource is mapped and persistable end to end (§11): the recorder - * writes a result against a real feed and the repository reads back the most recent one per - * (feed, contextKey) via findLatestPublished. Uses the database (rolled back by dama). + * Proves the FeedContextResult resource is mapped and persistable end to end (§11) and that the + * publish-gate lookups behave (§6.6): findLatestPublished only sees `published` rows (so a pending + * candidate is never its own baseline), while findLatestForContext returns the most recent row + * regardless of state. Uses the database (rolled back by dama). */ final class FeedContextResultTest extends FunctionalTestCase { /** * @test */ - public function it_records_a_result_and_reads_back_the_latest_per_context(): void + public function it_does_not_treat_a_pending_candidate_as_its_own_baseline(): void { $feed = $this->persistFeed(); - $recorder = $this->recorder(); - - $recorder->record($feed, 'web_en_US_USD', new GenerationResult( - 'google/web_en_US_USD.xml', - 5, - 2, - 512, - [['item' => 'SKU-9', 'reason' => 'validation:setono_sylius_feed.google_shopping_item.title.not_blank']], - )); + $repository = $this->repository(); - // a later run for the same context supersedes the first - $recorder->record($feed, 'web_en_US_USD', new GenerationResult('google/web_en_US_USD.xml', 7, 0, 700, [])); + // A freshly recorded candidate is `pending`... + $candidate = $this->recorder()->record($feed, 'web_en_US_USD', new GenerationResult('google/web_en_US_USD.xml', 5, 2, 512, [])); - // a different context is stored independently - $recorder->record($feed, 'web_da_DK_DKK', new GenerationResult('google/web_da_DK_DKK.xml', 1, 0, 64, [])); + // ...so it is not yet a published baseline, but it is the latest result for the context. + self::assertNull($repository->findLatestPublished($feed, 'web_en_US_USD')); + self::assertSame($candidate->getId(), $repository->findLatestForContext($feed, 'web_en_US_USD')?->getId()); + self::assertNull($repository->findLatestForContext($feed, 'does_not_exist')); + } + /** + * @test + */ + public function it_returns_the_latest_published_row_as_the_baseline(): void + { + $feed = $this->persistFeed(); + $recorder = $this->recorder(); $repository = $this->repository(); + $manager = $this->entityManager(); + + // Publish a first run for the context. + $first = $recorder->record($feed, 'web_en_US_USD', new GenerationResult('google/web_en_US_USD.xml', 5, 2, 512, [])); + $first->setPublishState(FeedContextResultInterface::PUBLISH_STATE_PUBLISHED); + $manager->flush(); + + $baseline = $repository->findLatestPublished($feed, 'web_en_US_USD'); + self::assertInstanceOf(FeedContextResultInterface::class, $baseline); + self::assertSame($first->getId(), $baseline->getId()); - $latest = $repository->findLatestPublished($feed, 'web_en_US_USD'); - self::assertInstanceOf(FeedContextResultInterface::class, $latest); - self::assertSame(7, $latest->getItemCount()); - self::assertSame(0, $latest->getExcludedCount()); - self::assertSame(700, $latest->getBytes()); - self::assertSame([], $latest->getErrors()); - self::assertSame($feed->getId(), $latest->getFeed()?->getId()); + // A later run supersedes the first as the latest candidate, but while it is still pending the + // baseline remains the previously published row. + $second = $recorder->record($feed, 'web_en_US_USD', new GenerationResult('google/web_en_US_USD.xml', 7, 0, 700, [])); + self::assertSame($second->getId(), $repository->findLatestForContext($feed, 'web_en_US_USD')?->getId()); + self::assertSame($first->getId(), $repository->findLatestPublished($feed, 'web_en_US_USD')?->getId()); - $otherContext = $repository->findLatestPublished($feed, 'web_da_DK_DKK'); - self::assertInstanceOf(FeedContextResultInterface::class, $otherContext); - self::assertSame(1, $otherContext->getItemCount()); + // Publishing the second run makes it the new baseline. + $second->setPublishState(FeedContextResultInterface::PUBLISH_STATE_PUBLISHED); + $manager->flush(); + $newBaseline = $repository->findLatestPublished($feed, 'web_en_US_USD'); + self::assertInstanceOf(FeedContextResultInterface::class, $newBaseline); + self::assertSame(7, $newBaseline->getItemCount()); + self::assertSame(700, $newBaseline->getBytes()); + self::assertSame($feed->getId(), $newBaseline->getFeed()?->getId()); - self::assertNull($repository->findLatestPublished($feed, 'does_not_exist')); + // A different context is stored independently. + $recorder->record($feed, 'web_da_DK_DKK', new GenerationResult('google/web_da_DK_DKK.xml', 1, 0, 64, [])); + self::assertSame(1, $repository->findLatestForContext($feed, 'web_da_DK_DKK')?->getItemCount()); } private function persistFeed(): FeedInterface diff --git a/tests/Functional/MessageHandler/PublishGateFlowTest.php b/tests/Functional/MessageHandler/PublishGateFlowTest.php new file mode 100644 index 00000000..5dbc50c8 --- /dev/null +++ b/tests/Functional/MessageHandler/PublishGateFlowTest.php @@ -0,0 +1,172 @@ +OLD-LIVE-FEED'; + + protected function tearDown(): void + { + foreach (['setono_sylius_feed.storage.feed', 'setono_sylius_feed.storage.feed_tmp'] as $id) { + $filesystem = self::getContainer()->get($id); + if ($filesystem instanceof FilesystemOperator) { + $filesystem->deleteDirectory('google'); + } + } + + parent::tearDown(); + } + + /** + * @test + */ + public function it_blocks_promotion_and_keeps_the_live_file_when_a_guardrail_trips(): void + { + // the single generated item cannot satisfy a min_items guardrail of 100 + $feedId = (int) $this->persistFixtures([ + 'guardrails' => [ + ['type' => 'min_items', 'params' => ['min' => 100], 'severity' => 'block'], + ], + ])->getId(); + + // seed a previously published canonical file so we can prove it is kept live + $canonical = self::getContainer()->get('setono_sylius_feed.storage.feed'); + self::assertInstanceOf(FilesystemOperator::class, $canonical); + $canonical->write('google/' . self::CONTEXT . '.xml', self::LIVE_CONTENT); + + $commandBus = self::getContainer()->get('setono_sylius_feed.command_bus'); + self::assertInstanceOf(MessageBusInterface::class, $commandBus); + $commandBus->dispatch(new ProcessFeed($feedId)); + + $this->entityManager()->clear(); + + $repository = self::getContainer()->get('setono_sylius_feed.repository.feed'); + self::assertInstanceOf(RepositoryInterface::class, $repository); + $feed = $repository->find($feedId); + self::assertInstanceOf(FeedInterface::class, $feed); + self::assertSame(FeedGraph::STATE_COMPLETED, $feed->getState()); + + // the candidate was gated: recorded as blocked with a reason + $resultRepository = self::getContainer()->get('setono_sylius_feed.repository.feed_context_result'); + self::assertInstanceOf(FeedContextResultRepositoryInterface::class, $resultRepository); + $result = $resultRepository->findLatestForContext($feed, self::CONTEXT); + self::assertNotNull($result); + self::assertTrue($result->isBlocked()); + self::assertNotNull($result->getPublishCheck()); + + // the live canonical file was NOT overwritten + self::assertSame(self::LIVE_CONTENT, $canonical->read('google/' . self::CONTEXT . '.xml')); + + // the blocked candidate is retained in staging for inspection + $temporary = self::getContainer()->get('setono_sylius_feed.storage.feed_tmp'); + self::assertInstanceOf(FilesystemOperator::class, $temporary); + self::assertTrue($temporary->fileExists('google/' . self::CONTEXT . '.xml')); + } + + private function entityManager(): EntityManagerInterface + { + $manager = self::getContainer()->get('doctrine.orm.entity_manager'); + self::assertInstanceOf(EntityManagerInterface::class, $manager); + + return $manager; + } + + /** + * @param array $publishConfig + */ + private function persistFixtures(array $publishConfig): FeedInterface + { + $currency = new Currency(); + $currency->setCode('USD'); + + $locale = new Locale(); + $locale->setCode('en_US'); + + $channel = new Channel(); + $channel->setCode('web'); + $channel->setName('Web'); + $channel->setHostname('localhost'); + $channel->setBaseCurrency($currency); + $channel->setDefaultLocale($locale); + $channel->addCurrency($currency); + $channel->addLocale($locale); + $channel->setTaxCalculationStrategy('order_items_based'); + $channel->setEnabled(true); + + $product = new Product(); + $product->setCode('PROD-1'); + $product->setEnabled(true); + $product->setCurrentLocale('en_US'); + $product->setFallbackLocale('en_US'); + $product->setName('Acme Shoe'); + $product->setSlug('acme-shoe'); + $product->setDescription('A very nice shoe'); + $product->addChannel($channel); + + $image = new ProductImage(); + $image->setPath('acme.jpg'); + $product->addImage($image); + + $variant = new ProductVariant(); + $variant->setCode('VARIANT-1'); + + $channelPricing = new ChannelPricing(); + $channelPricing->setChannelCode('web'); + $channelPricing->setPrice(999); + $variant->addChannelPricing($channelPricing); + + $product->addVariant($variant); + + $feed = new Feed(); + $feed->setCode('google'); + $feed->setFormat('google_rss'); + $feed->setEnabled(true); + $feed->setCurrentLocale('en_US'); + $feed->setName('Google feed'); + $feed->setSlug('google-feed'); + $feed->addChannel($channel); + $feed->setPublishConfig($publishConfig); + + $source = new FeedSource(); + $source->setFeedType('product_variant'); + $source->setPosition(0); + $feed->addSource($source); + + $manager = $this->entityManager(); + foreach ([$currency, $locale, $channel, $product, $variant, $feed] as $entity) { + $manager->persist($entity); + } + $manager->flush(); + + return $feed; + } +} diff --git a/tests/Unit/Controller/Admin/PublishContextAnywayActionTest.php b/tests/Unit/Controller/Admin/PublishContextAnywayActionTest.php new file mode 100644 index 00000000..e212aa48 --- /dev/null +++ b/tests/Unit/Controller/Admin/PublishContextAnywayActionTest.php @@ -0,0 +1,128 @@ +setCode('google'); + + $repository = $this->prophesize(FeedRepositoryInterface::class); + $repository->find(1)->willReturn($feed); + + $listing = (static function (): \Generator { + yield new DirectoryAttributes('google/nested'); // not a file -> skipped + yield new FileAttributes('google/web_en_us_usd.xml'); // matches the requested context + yield new FileAttributes('google/web_da_dk_dkk.xml'); // a different context -> untouched + })(); + + $temporary = $this->prophesize(FilesystemOperator::class); + $temporary->listContents('google', true)->willReturn(new DirectoryListing($listing)); + $temporary->readStream('google/web_en_us_usd.xml')->willReturn($stream); + $temporary->delete('google/web_en_us_usd.xml')->shouldBeCalledOnce(); + $temporary->delete('google/web_da_dk_dkk.xml')->shouldNotBeCalled(); + + $canonical = $this->prophesize(FilesystemOperator::class); + $canonical->writeStream('google/web_en_us_usd.xml', $stream)->shouldBeCalledOnce(); + $canonical->writeStream('google/web_da_dk_dkk.xml', Argument::any())->shouldNotBeCalled(); + + $result = new FeedContextResult(); + $result->setPublishState(FeedContextResultInterface::PUBLISH_STATE_BLOCKED); + + $resultRepository = $this->prophesize(FeedContextResultRepositoryInterface::class); + $resultRepository->findLatestForContext($feed, 'web_en_us_usd')->willReturn($result); + + $manager = $this->prophesize(EntityManagerInterface::class); + $manager->flush()->shouldBeCalledOnce(); + + $managerRegistry = $this->prophesize(ManagerRegistry::class); + $managerRegistry->getManagerForClass(FeedContextResult::class)->willReturn($manager->reveal()); + + $router = $this->prophesize(RouterInterface::class); + $router->generate('setono_sylius_feed_admin_feed_results', Argument::any())->willReturn('/admin/feeds/1/results'); + + $session = new Session(new MockArraySessionStorage()); + $request = new Request(); + $request->setSession($session); + + $action = new PublishContextAnywayAction( + $managerRegistry->reveal(), + $repository->reveal(), + $resultRepository->reveal(), + $temporary->reveal(), + $canonical->reveal(), + $router->reveal(), + ); + + $response = $action($request, 1, 'web_en_us_usd'); + + self::assertInstanceOf(RedirectResponse::class, $response); + self::assertSame('/admin/feeds/1/results', $response->getTargetUrl()); + self::assertTrue($result->isPublished()); + + /** @var array> $flashes */ + $flashes = $session->getFlashBag()->peekAll(); + self::assertContains('setono_sylius_feed.feed.context_published', $flashes['success'] ?? []); + + fclose($stream); + } + + /** + * @test + */ + public function it_404s_for_a_missing_feed(): void + { + $repository = $this->prophesize(FeedRepositoryInterface::class); + $repository->find(999)->willReturn(null); + + $temporary = $this->prophesize(FilesystemOperator::class); + $temporary->listContents(Argument::cetera())->shouldNotBeCalled(); + + $action = new PublishContextAnywayAction( + $this->prophesize(ManagerRegistry::class)->reveal(), + $repository->reveal(), + $this->prophesize(FeedContextResultRepositoryInterface::class)->reveal(), + $temporary->reveal(), + $this->prophesize(FilesystemOperator::class)->reveal(), + $this->prophesize(RouterInterface::class)->reveal(), + ); + + $this->expectException(NotFoundHttpException::class); + + $action(new Request(), 999, 'web_en_us_usd'); + } +} diff --git a/tests/Unit/EventSubscriber/MoveGeneratedFeedSubscriberTest.php b/tests/Unit/EventSubscriber/MoveGeneratedFeedSubscriberTest.php index 4e564f9c..77535942 100644 --- a/tests/Unit/EventSubscriber/MoveGeneratedFeedSubscriberTest.php +++ b/tests/Unit/EventSubscriber/MoveGeneratedFeedSubscriberTest.php @@ -13,6 +13,9 @@ use Prophecy\PhpUnit\ProphecyTrait; use Setono\SyliusFeedPlugin\EventSubscriber\MoveGeneratedFeedSubscriber; use Setono\SyliusFeedPlugin\Model\Feed; +use Setono\SyliusFeedPlugin\Model\FeedContextResult; +use Setono\SyliusFeedPlugin\Model\FeedContextResultInterface; +use Setono\SyliusFeedPlugin\Repository\FeedContextResultRepositoryInterface; use Symfony\Component\Workflow\Event\CompletedEvent; use Symfony\Component\Workflow\Marking; use Symfony\Component\Workflow\Transition; @@ -36,33 +39,50 @@ public function it_subscribes_to_the_complete_transition(): void /** * @test */ - public function it_swaps_temporary_files_to_canonical_storage_and_stamps_the_feed(): void + public function it_promotes_published_contexts_and_retains_blocked_ones(): void { - $stream = fopen('php://temp', 'r'); - self::assertIsResource($stream); + $publishedStream = fopen('php://temp', 'r'); + self::assertIsResource($publishedStream); + $ungatedStream = fopen('php://temp', 'r'); + self::assertIsResource($ungatedStream); $listing = (static function (): \Generator { yield new DirectoryAttributes('google/nested'); // not a file -> skipped - yield new FileAttributes('google/web_en_us_usd.xml'); + yield new FileAttributes('google/web_en_us_usd.xml'); // published -> promoted + yield new FileAttributes('google/web_da_dk_dkk.xml'); // blocked -> retained + yield new FileAttributes('google/web_sv_se_sek.xml'); // no result -> promoted (no gate) })(); $temporary = $this->prophesize(FilesystemOperator::class); $temporary->listContents('google', true)->willReturn(new DirectoryListing($listing)); - $temporary->readStream('google/web_en_us_usd.xml')->willReturn($stream); - $temporary->deleteDirectory('google')->shouldBeCalled(); + $temporary->readStream('google/web_en_us_usd.xml')->willReturn($publishedStream); + $temporary->readStream('google/web_sv_se_sek.xml')->willReturn($ungatedStream); + $temporary->readStream('google/web_da_dk_dkk.xml')->shouldNotBeCalled(); + $temporary->delete('google/web_en_us_usd.xml')->shouldBeCalledOnce(); + $temporary->delete('google/web_sv_se_sek.xml')->shouldBeCalledOnce(); + $temporary->delete('google/web_da_dk_dkk.xml')->shouldNotBeCalled(); $canonical = $this->prophesize(FilesystemOperator::class); - $canonical->deleteDirectory('google')->shouldBeCalled(); - $canonical->writeStream('google/web_en_us_usd.xml', $stream)->shouldBeCalled(); + $canonical->writeStream('google/web_en_us_usd.xml', $publishedStream)->shouldBeCalledOnce(); + $canonical->writeStream('google/web_sv_se_sek.xml', $ungatedStream)->shouldBeCalledOnce(); + $canonical->writeStream('google/web_da_dk_dkk.xml', Argument::any())->shouldNotBeCalled(); + $canonical->deleteDirectory(Argument::any())->shouldNotBeCalled(); $feed = new Feed(); $feed->setCode('google'); - (new MoveGeneratedFeedSubscriber($temporary->reveal(), $canonical->reveal()))->onCompleted($this->event($feed)); + $repository = $this->prophesize(FeedContextResultRepositoryInterface::class); + $repository->findLatestForContext($feed, 'web_en_us_usd')->willReturn($this->result(FeedContextResultInterface::PUBLISH_STATE_PUBLISHED)); + $repository->findLatestForContext($feed, 'web_da_dk_dkk')->willReturn($this->result(FeedContextResultInterface::PUBLISH_STATE_BLOCKED)); + $repository->findLatestForContext($feed, 'web_sv_se_sek')->willReturn(null); + + (new MoveGeneratedFeedSubscriber($temporary->reveal(), $canonical->reveal(), $repository->reveal())) + ->onCompleted($this->event($feed)); self::assertNotNull($feed->getLastGeneratedAt()); - fclose($stream); + fclose($publishedStream); + fclose($ungatedStream); } /** @@ -71,14 +91,25 @@ public function it_swaps_temporary_files_to_canonical_storage_and_stamps_the_fee public function it_ignores_a_non_feed_subject(): void { $temporary = $this->prophesize(FilesystemOperator::class); - $temporary->deleteDirectory(Argument::any())->shouldNotBeCalled(); + $temporary->listContents(Argument::cetera())->shouldNotBeCalled(); - (new MoveGeneratedFeedSubscriber($temporary->reveal(), $this->prophesize(FilesystemOperator::class)->reveal())) - ->onCompleted($this->event(new \stdClass())); + (new MoveGeneratedFeedSubscriber( + $temporary->reveal(), + $this->prophesize(FilesystemOperator::class)->reveal(), + $this->prophesize(FeedContextResultRepositoryInterface::class)->reveal(), + ))->onCompleted($this->event(new \stdClass())); self::addToAssertionCount(1); } + private function result(string $publishState): FeedContextResult + { + $result = new FeedContextResult(); + $result->setPublishState($publishState); + + return $result; + } + private function event(object $subject): CompletedEvent { return new CompletedEvent( diff --git a/tests/Unit/Model/FeedContextResultTest.php b/tests/Unit/Model/FeedContextResultTest.php index c7b58f5b..bf7941a7 100644 --- a/tests/Unit/Model/FeedContextResultTest.php +++ b/tests/Unit/Model/FeedContextResultTest.php @@ -7,6 +7,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; use Setono\SyliusFeedPlugin\Model\FeedContextResult; +use Setono\SyliusFeedPlugin\Model\FeedContextResultInterface; use Setono\SyliusFeedPlugin\Model\FeedInterface; final class FeedContextResultTest extends TestCase @@ -28,6 +29,8 @@ public function it_exposes_its_accessors(): void $result->setExcludedCount(3); $result->setBytes(1024); $result->setErrors([['item' => 'SKU-1', 'reason' => 'validation:missing title']]); + $result->setPublishState(FeedContextResultInterface::PUBLISH_STATE_BLOCKED); + $result->setPublishCheck(['max_drop_pct: guardrail tripped (severity: block)']); $result->setCreatedAt($createdAt); self::assertNull($result->getId()); @@ -37,9 +40,40 @@ public function it_exposes_its_accessors(): void self::assertSame(3, $result->getExcludedCount()); self::assertSame(1024, $result->getBytes()); self::assertSame([['item' => 'SKU-1', 'reason' => 'validation:missing title']], $result->getErrors()); + self::assertSame(FeedContextResultInterface::PUBLISH_STATE_BLOCKED, $result->getPublishState()); + self::assertSame(['max_drop_pct: guardrail tripped (severity: block)'], $result->getPublishCheck()); self::assertSame($createdAt, $result->getCreatedAt()); } + /** + * @test + */ + public function it_starts_pending_with_no_publish_check(): void + { + $result = new FeedContextResult(); + + self::assertSame(FeedContextResultInterface::PUBLISH_STATE_PENDING, $result->getPublishState()); + self::assertNull($result->getPublishCheck()); + self::assertFalse($result->isPublished()); + self::assertFalse($result->isBlocked()); + } + + /** + * @test + */ + public function it_reflects_its_publish_state_through_predicates(): void + { + $result = new FeedContextResult(); + + $result->setPublishState(FeedContextResultInterface::PUBLISH_STATE_PUBLISHED); + self::assertTrue($result->isPublished()); + self::assertFalse($result->isBlocked()); + + $result->setPublishState(FeedContextResultInterface::PUBLISH_STATE_BLOCKED); + self::assertFalse($result->isPublished()); + self::assertTrue($result->isBlocked()); + } + /** * @test */ diff --git a/tests/Unit/Model/FeedTest.php b/tests/Unit/Model/FeedTest.php index 6fd6733f..55357a0f 100644 --- a/tests/Unit/Model/FeedTest.php +++ b/tests/Unit/Model/FeedTest.php @@ -26,6 +26,7 @@ public function it_starts_ready_enabled_and_empty(): void self::assertTrue($feed->isEnabled()); self::assertSame(FeedGraph::STATE_READY, $feed->getState()); self::assertSame([], $feed->getFormatConfig()); + self::assertSame([], $feed->getPublishConfig()); self::assertNull($feed->getLastGeneratedAt()); self::assertCount(0, $feed->getChannels()); self::assertCount(0, $feed->getSources()); @@ -44,6 +45,7 @@ public function it_holds_its_scalar_configuration(): void $feed->setFormat('google_rss'); $feed->setState(FeedGraph::STATE_PROCESSING); $feed->setFormatConfig(['gzip' => true]); + $feed->setPublishConfig(['guardrails' => [['type' => 'non_empty', 'severity' => 'block']]]); $feed->setLastGeneratedAt($generatedAt); self::assertSame('google', $feed->getCode()); @@ -51,6 +53,7 @@ public function it_holds_its_scalar_configuration(): void self::assertSame('google_rss', $feed->getFormat()); self::assertSame(FeedGraph::STATE_PROCESSING, $feed->getState()); self::assertSame(['gzip' => true], $feed->getFormatConfig()); + self::assertSame(['guardrails' => [['type' => 'non_empty', 'severity' => 'block']]], $feed->getPublishConfig()); self::assertSame($generatedAt, $feed->getLastGeneratedAt()); } diff --git a/tests/Unit/Publish/GuardrailRegistryTest.php b/tests/Unit/Publish/GuardrailRegistryTest.php new file mode 100644 index 00000000..f64f2ef1 --- /dev/null +++ b/tests/Unit/Publish/GuardrailRegistryTest.php @@ -0,0 +1,41 @@ +has('min_items')); + self::assertTrue($registry->has('non_empty')); + self::assertFalse($registry->has('does_not_exist')); + self::assertSame($minItems, $registry->get('min_items')); + self::assertSame($nonEmpty, $registry->get('non_empty')); + self::assertCount(2, $registry); + } + + /** + * @test + */ + public function it_rejects_duplicate_types(): void + { + $this->expectException(\InvalidArgumentException::class); + + new GuardrailRegistry([new NonEmptyGuardrail(), new NonEmptyGuardrail()]); + } +} diff --git a/tests/Unit/Publish/MaxDropPctGuardrailTest.php b/tests/Unit/Publish/MaxDropPctGuardrailTest.php new file mode 100644 index 00000000..c310a250 --- /dev/null +++ b/tests/Unit/Publish/MaxDropPctGuardrailTest.php @@ -0,0 +1,73 @@ +getType()); + } + + /** + * @test + */ + public function it_does_not_trip_without_a_baseline(): void + { + self::assertFalse((new MaxDropPctGuardrail())->evaluate($this->result(0), null, ['pct' => 40])); + } + + /** + * @test + */ + public function it_trips_when_the_drop_exceeds_the_threshold(): void + { + // 50_000 -> 5_000 is a 90% drop, well beyond the 40% threshold + self::assertTrue((new MaxDropPctGuardrail())->evaluate($this->result(5000), $this->result(50000), ['pct' => 40])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_drop_is_within_the_threshold(): void + { + // 100 -> 70 is a 30% drop, within a 40% threshold + self::assertFalse((new MaxDropPctGuardrail())->evaluate($this->result(70), $this->result(100), ['pct' => 40])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_candidate_grew_or_held_steady(): void + { + $guardrail = new MaxDropPctGuardrail(); + + self::assertFalse($guardrail->evaluate($this->result(120), $this->result(100), ['pct' => 40])); + self::assertFalse($guardrail->evaluate($this->result(100), $this->result(100), ['pct' => 40])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_baseline_was_empty(): void + { + self::assertFalse((new MaxDropPctGuardrail())->evaluate($this->result(0), $this->result(0), ['pct' => 40])); + } + + private function result(int $itemCount): FeedContextResult + { + $result = new FeedContextResult(); + $result->setItemCount($itemCount); + + return $result; + } +} diff --git a/tests/Unit/Publish/MaxExclusionPctGuardrailTest.php b/tests/Unit/Publish/MaxExclusionPctGuardrailTest.php new file mode 100644 index 00000000..cd1533b2 --- /dev/null +++ b/tests/Unit/Publish/MaxExclusionPctGuardrailTest.php @@ -0,0 +1,77 @@ +getType()); + } + + /** + * @test + */ + public function it_trips_on_the_absolute_threshold_without_a_baseline(): void + { + // 60 excluded of 100 processed = 60% exclusion, beyond the 50% threshold + self::assertTrue((new MaxExclusionPctGuardrail())->evaluate($this->result(40, 60), null, ['pct' => 50])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_ratio_is_within_the_threshold(): void + { + // 40 excluded of 100 = 40% exclusion, within the 50% threshold + self::assertFalse((new MaxExclusionPctGuardrail())->evaluate($this->result(60, 40), null, ['pct' => 50])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_ratio_did_not_jump_beyond_the_baseline(): void + { + // candidate 60% and baseline already 70% => a stable/lower exclusion share, not a regression + self::assertFalse( + (new MaxExclusionPctGuardrail())->evaluate($this->result(40, 60), $this->result(30, 70), ['pct' => 50]), + ); + } + + /** + * @test + */ + public function it_trips_when_the_ratio_exceeds_both_the_threshold_and_the_baseline(): void + { + // candidate 60% exclusion vs a baseline 20% exclusion => a real jump past the 50% threshold + self::assertTrue( + (new MaxExclusionPctGuardrail())->evaluate($this->result(40, 60), $this->result(80, 20), ['pct' => 50]), + ); + } + + /** + * @test + */ + public function it_does_not_trip_when_nothing_was_processed(): void + { + self::assertFalse((new MaxExclusionPctGuardrail())->evaluate($this->result(0, 0), null, ['pct' => 50])); + } + + private function result(int $itemCount, int $excludedCount): FeedContextResult + { + $result = new FeedContextResult(); + $result->setItemCount($itemCount); + $result->setExcludedCount($excludedCount); + + return $result; + } +} diff --git a/tests/Unit/Publish/MaxGrowthPctGuardrailTest.php b/tests/Unit/Publish/MaxGrowthPctGuardrailTest.php new file mode 100644 index 00000000..2cbe2c4f --- /dev/null +++ b/tests/Unit/Publish/MaxGrowthPctGuardrailTest.php @@ -0,0 +1,73 @@ +getType()); + } + + /** + * @test + */ + public function it_does_not_trip_without_a_baseline(): void + { + self::assertFalse((new MaxGrowthPctGuardrail())->evaluate($this->result(1_000_000), null, ['pct' => 50])); + } + + /** + * @test + */ + public function it_trips_when_the_growth_exceeds_the_threshold(): void + { + // 100 -> 250 is 150% growth, beyond the 50% threshold + self::assertTrue((new MaxGrowthPctGuardrail())->evaluate($this->result(250), $this->result(100), ['pct' => 50])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_growth_is_within_the_threshold(): void + { + // 100 -> 140 is 40% growth, within a 50% threshold + self::assertFalse((new MaxGrowthPctGuardrail())->evaluate($this->result(140), $this->result(100), ['pct' => 50])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_candidate_shrank_or_held_steady(): void + { + $guardrail = new MaxGrowthPctGuardrail(); + + self::assertFalse($guardrail->evaluate($this->result(80), $this->result(100), ['pct' => 50])); + self::assertFalse($guardrail->evaluate($this->result(100), $this->result(100), ['pct' => 50])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_baseline_was_empty(): void + { + self::assertFalse((new MaxGrowthPctGuardrail())->evaluate($this->result(500), $this->result(0), ['pct' => 50])); + } + + private function result(int $itemCount): FeedContextResult + { + $result = new FeedContextResult(); + $result->setItemCount($itemCount); + + return $result; + } +} diff --git a/tests/Unit/Publish/MinBytesGuardrailTest.php b/tests/Unit/Publish/MinBytesGuardrailTest.php new file mode 100644 index 00000000..8667e380 --- /dev/null +++ b/tests/Unit/Publish/MinBytesGuardrailTest.php @@ -0,0 +1,47 @@ +getType()); + } + + /** + * @test + */ + public function it_trips_when_the_file_is_smaller_than_the_minimum(): void + { + self::assertTrue((new MinBytesGuardrail())->evaluate($this->result(511), null, ['bytes' => 512])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_file_meets_the_minimum(): void + { + $guardrail = new MinBytesGuardrail(); + + self::assertFalse($guardrail->evaluate($this->result(512), null, ['bytes' => 512])); + self::assertFalse($guardrail->evaluate($this->result(1024), null, ['bytes' => 512])); + } + + private function result(int $bytes): FeedContextResult + { + $result = new FeedContextResult(); + $result->setBytes($bytes); + + return $result; + } +} diff --git a/tests/Unit/Publish/MinItemsGuardrailTest.php b/tests/Unit/Publish/MinItemsGuardrailTest.php new file mode 100644 index 00000000..5edc899a --- /dev/null +++ b/tests/Unit/Publish/MinItemsGuardrailTest.php @@ -0,0 +1,55 @@ +getType()); + } + + /** + * @test + */ + public function it_trips_when_the_item_count_is_below_the_minimum(): void + { + self::assertTrue((new MinItemsGuardrail())->evaluate($this->result(9), null, ['min' => 10])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_item_count_meets_the_minimum(): void + { + $guardrail = new MinItemsGuardrail(); + + self::assertFalse($guardrail->evaluate($this->result(10), null, ['min' => 10])); + self::assertFalse($guardrail->evaluate($this->result(11), null, ['min' => 10])); + } + + /** + * @test + */ + public function it_ignores_the_baseline(): void + { + self::assertTrue((new MinItemsGuardrail())->evaluate($this->result(1), $this->result(1000), ['min' => 10])); + } + + private function result(int $itemCount): FeedContextResult + { + $result = new FeedContextResult(); + $result->setItemCount($itemCount); + + return $result; + } +} diff --git a/tests/Unit/Publish/NonEmptyGuardrailTest.php b/tests/Unit/Publish/NonEmptyGuardrailTest.php new file mode 100644 index 00000000..96691cbc --- /dev/null +++ b/tests/Unit/Publish/NonEmptyGuardrailTest.php @@ -0,0 +1,47 @@ +getType()); + } + + /** + * @test + */ + public function it_trips_when_the_candidate_is_empty(): void + { + self::assertTrue((new NonEmptyGuardrail())->evaluate($this->result(0), null, [])); + } + + /** + * @test + */ + public function it_does_not_trip_when_the_candidate_has_items(): void + { + $guardrail = new NonEmptyGuardrail(); + + self::assertFalse($guardrail->evaluate($this->result(1), null, [])); + self::assertFalse($guardrail->evaluate($this->result(1), $this->result(0), [])); + } + + private function result(int $itemCount): FeedContextResult + { + $result = new FeedContextResult(); + $result->setItemCount($itemCount); + + return $result; + } +} diff --git a/tests/Unit/Publish/PublishGateTest.php b/tests/Unit/Publish/PublishGateTest.php new file mode 100644 index 00000000..c3ad34bf --- /dev/null +++ b/tests/Unit/Publish/PublishGateTest.php @@ -0,0 +1,162 @@ +gate()->evaluate($this->result(0), null, []); + + self::assertFalse($decision->blocked); + self::assertSame([], $decision->reasons); + } + + /** + * @test + */ + public function it_blocks_when_a_block_severity_guardrail_trips(): void + { + $decision = $this->gate()->evaluate( + $this->result(0), + null, + [['type' => 'non_empty', 'severity' => 'block']], + ); + + self::assertTrue($decision->blocked); + self::assertCount(1, $decision->reasons); + self::assertStringContainsString('non_empty', $decision->reasons[0]); + } + + /** + * @test + */ + public function it_records_a_reason_but_does_not_block_for_a_warn_guardrail(): void + { + $decision = $this->gate()->evaluate( + $this->result(0), + null, + [['type' => 'non_empty', 'severity' => 'warn']], + ); + + self::assertFalse($decision->blocked); + self::assertCount(1, $decision->reasons); + self::assertStringContainsString('warn', $decision->reasons[0]); + } + + /** + * @test + */ + public function it_skips_unknown_guardrail_types(): void + { + $decision = $this->gate()->evaluate( + $this->result(0), + null, + [['type' => 'does_not_exist', 'severity' => 'block']], + ); + + self::assertFalse($decision->blocked); + self::assertSame([], $decision->reasons); + } + + /** + * @test + */ + public function it_does_not_record_a_reason_for_a_guardrail_that_does_not_trip(): void + { + $decision = $this->gate()->evaluate( + $this->result(100), + null, + [['type' => 'non_empty', 'severity' => 'block']], + ); + + self::assertFalse($decision->blocked); + self::assertSame([], $decision->reasons); + } + + /** + * @test + */ + public function it_blocks_when_any_block_guardrail_trips_among_several(): void + { + $decision = $this->gate()->evaluate( + $this->result(0), + null, + [ + ['type' => 'min_bytes', 'params' => ['bytes' => 10], 'severity' => 'warn'], + ['type' => 'non_empty', 'severity' => 'block'], + ], + ); + + self::assertTrue($decision->blocked); + self::assertCount(2, $decision->reasons); + } + + /** + * @test + */ + public function it_defaults_missing_severity_to_block(): void + { + $decision = $this->gate()->evaluate( + $this->result(0), + null, + [['type' => 'non_empty']], + ); + + self::assertTrue($decision->blocked); + } + + /** + * @test + */ + public function it_blocks_the_50k_to_5k_collapse(): void + { + // Acceptance (§6.6): a candidate of 5000 items vs a baseline of 50000, with a + // max_drop_pct guardrail set to 40% and block severity, must block promotion. + $decision = $this->gate()->evaluate( + $this->result(5000), + $this->result(50000), + [['type' => 'max_drop_pct', 'params' => ['pct' => 40], 'severity' => 'block']], + ); + + self::assertTrue($decision->blocked); + self::assertCount(1, $decision->reasons); + self::assertStringContainsString('max_drop_pct', $decision->reasons[0]); + } + + private function gate(): PublishGate + { + return new PublishGate(new GuardrailRegistry([ + new MinItemsGuardrail(), + new MaxDropPctGuardrail(), + new NonEmptyGuardrail(), + new MinBytesGuardrail(), + new MaxExclusionPctGuardrail(), + new MaxGrowthPctGuardrail(), + ])); + } + + private function result(int $itemCount): FeedContextResult + { + $result = new FeedContextResult(); + $result->setItemCount($itemCount); + + return $result; + } +} From e9326c3db470ab09277766b48c58c7c37114fb00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joachim=20L=C3=B8vgaard?= Date: Thu, 2 Jul 2026 17:16:32 +0200 Subject: [PATCH 4/4] M6: dry-run preview + feed audit - Extract MappingResolver (FeedField-first, preset fallback) shared by the generator + preview so the pipeline is not forked. - PreviewService runs the exact per-item pipeline (bind -> pre-filter -> map -> post-filter -> validate) over <=N sampled items, writing nothing, and returns a funnel (source -> after pre -> after mapping/validation -> after post -> included), included sample bags, excluded samples with the dropping reason, and a single-item tester (previewItem by id). - FeedAuditService computes per-field fill rates, soft warnings (title>150, HTML-in-description, price=0) and value distributions over the included sample -- advisory, excludes nothing. - Admin Preview screen (/feeds/{id}/preview + grid action) and console --preview[=N] / --audit options. +14 tests (581 total); all gates (incl. PHP 8.1 + twig lint) green. --- src/Audit/AuditReport.php | 25 ++ src/Audit/FeedAuditService.php | 228 +++++++++++ src/Audit/FeedAuditServiceInterface.php | 16 + src/Command/ProcessFeedCommand.php | 128 +++++- src/Controller/Admin/PreviewFeedAction.php | 63 +++ src/Generator/FeedGenerator.php | 37 +- src/Mapping/MappingResolver.php | 41 ++ src/Mapping/MappingResolverInterface.php | 22 ++ src/Preview/PreviewFunnel.php | 23 ++ src/Preview/PreviewResult.php | 24 ++ src/Preview/PreviewService.php | 235 +++++++++++ src/Preview/PreviewServiceInterface.php | 31 ++ src/Resources/config/app/config.yaml | 9 + src/Resources/config/routes/admin.yaml | 6 + src/Resources/config/services.xml | 1 + src/Resources/config/services/command.xml | 3 + src/Resources/config/services/controller.xml | 9 + src/Resources/config/services/generator.xml | 7 +- src/Resources/config/services/preview.xml | 22 ++ src/Resources/translations/messages.en.yaml | 26 ++ .../views/admin/feed/preview.html.twig | 154 ++++++++ tests/Unit/Audit/FeedAuditServiceTest.php | 63 +++ tests/Unit/Command/ProcessFeedCommandTest.php | 114 +++++- .../Admin/PreviewFeedActionTest.php | 92 +++++ .../Generator/FeedGeneratorFilterTest.php | 3 +- .../Generator/FeedGeneratorMemoryTest.php | 3 +- tests/Unit/Generator/FeedGeneratorTest.php | 3 +- tests/Unit/Mapping/MappingResolverTest.php | 110 ++++++ tests/Unit/Preview/PreviewServiceTest.php | 372 ++++++++++++++++++ 29 files changed, 1828 insertions(+), 42 deletions(-) create mode 100644 src/Audit/AuditReport.php create mode 100644 src/Audit/FeedAuditService.php create mode 100644 src/Audit/FeedAuditServiceInterface.php create mode 100644 src/Controller/Admin/PreviewFeedAction.php create mode 100644 src/Mapping/MappingResolver.php create mode 100644 src/Mapping/MappingResolverInterface.php create mode 100644 src/Preview/PreviewFunnel.php create mode 100644 src/Preview/PreviewResult.php create mode 100644 src/Preview/PreviewService.php create mode 100644 src/Preview/PreviewServiceInterface.php create mode 100644 src/Resources/config/services/preview.xml create mode 100644 src/Resources/views/admin/feed/preview.html.twig create mode 100644 tests/Unit/Audit/FeedAuditServiceTest.php create mode 100644 tests/Unit/Controller/Admin/PreviewFeedActionTest.php create mode 100644 tests/Unit/Mapping/MappingResolverTest.php create mode 100644 tests/Unit/Preview/PreviewServiceTest.php diff --git a/src/Audit/AuditReport.php b/src/Audit/AuditReport.php new file mode 100644 index 00000000..acba5262 --- /dev/null +++ b/src/Audit/AuditReport.php @@ -0,0 +1,25 @@ + $fillRates output field => fraction (0..1) of included items where the field is non-empty + * @param list $warnings advisory soft warnings, each with the number of affected items + * @param array> $distributions output field => value => count + */ + public function __construct( + public readonly array $fillRates, + public readonly array $warnings, + public readonly array $distributions, + ) { + } +} diff --git a/src/Audit/FeedAuditService.php b/src/Audit/FeedAuditService.php new file mode 100644 index 00000000..0e0358d6 --- /dev/null +++ b/src/Audit/FeedAuditService.php @@ -0,0 +1,228 @@ + */ + private const TITLE_FIELDS = ['g:title', 'title']; + + /** @var list */ + private const DESCRIPTION_FIELDS = ['g:description', 'description']; + + /** @var list */ + private const PRICE_FIELDS = ['g:price', 'price']; + + /** @var list */ + private const AVAILABILITY_FIELDS = ['g:availability', 'availability']; + + public function audit(PreviewResult $result): AuditReport + { + $bags = $result->included; + + return new AuditReport( + $this->fillRates($bags), + $this->warnings($bags), + $this->distributions($bags), + ); + } + + /** + * @param list> $bags + * + * @return array + */ + private function fillRates(array $bags): array + { + $total = count($bags); + if (0 === $total) { + return []; + } + + $fields = []; + foreach ($bags as $bag) { + foreach (array_keys($bag) as $field) { + if (!in_array($field, $fields, true)) { + $fields[] = $field; + } + } + } + + $fillRates = []; + foreach ($fields as $field) { + $filled = 0; + foreach ($bags as $bag) { + if (array_key_exists($field, $bag) && !$this->isEmpty($bag[$field])) { + ++$filled; + } + } + + $fillRates[$field] = (float) $filled / $total; + } + + return $fillRates; + } + + /** + * @param list> $bags + * + * @return list + */ + private function warnings(array $bags): array + { + $warnings = []; + + $titleField = $this->firstPresentField($bags, self::TITLE_FIELDS); + if (null !== $titleField) { + $count = $this->countMatching($bags, $titleField, static fn (mixed $value): bool => is_string($value) && mb_strlen($value) > self::MAX_TITLE_LENGTH); + if ($count > 0) { + $warnings[] = ['type' => 'title_too_long', 'field' => $titleField, 'count' => $count]; + } + } + + $descriptionField = $this->firstPresentField($bags, self::DESCRIPTION_FIELDS); + if (null !== $descriptionField) { + $count = $this->countMatching($bags, $descriptionField, static fn (mixed $value): bool => is_string($value) && 1 === preg_match('/<[^>]+>/', $value)); + if ($count > 0) { + $warnings[] = ['type' => 'description_contains_html', 'field' => $descriptionField, 'count' => $count]; + } + } + + $priceField = $this->firstPresentField($bags, self::PRICE_FIELDS); + if (null !== $priceField) { + $count = $this->countMatching($bags, $priceField, fn (mixed $value): bool => $this->isPriceEmpty($value)); + if ($count > 0) { + $warnings[] = ['type' => 'price_empty', 'field' => $priceField, 'count' => $count]; + } + } + + return $warnings; + } + + /** + * @param list> $bags + * + * @return array> + */ + private function distributions(array $bags): array + { + $distributions = []; + + $availabilityField = $this->firstPresentField($bags, self::AVAILABILITY_FIELDS); + if (null !== $availabilityField) { + $counts = []; + foreach ($bags as $bag) { + $value = $bag[$availabilityField] ?? null; + if (is_scalar($value)) { + $key = (string) $value; + $counts[$key] = ($counts[$key] ?? 0) + 1; + } + } + + if ([] !== $counts) { + $distributions[$availabilityField] = $counts; + } + } + + $priceField = $this->firstPresentField($bags, self::PRICE_FIELDS); + if (null !== $priceField) { + $zero = 0; + $nonZero = 0; + foreach ($bags as $bag) { + if (!array_key_exists($priceField, $bag)) { + continue; + } + + if ($this->isPriceEmpty($bag[$priceField])) { + ++$zero; + } else { + ++$nonZero; + } + } + + if ($zero + $nonZero > 0) { + $distributions[$priceField] = ['zero' => $zero, 'non_zero' => $nonZero]; + } + } + + return $distributions; + } + + /** + * @param list> $bags + * @param callable(mixed): bool $matches + */ + private function countMatching(array $bags, string $field, callable $matches): int + { + $count = 0; + foreach ($bags as $bag) { + if ($matches($bag[$field] ?? null)) { + ++$count; + } + } + + return $count; + } + + /** + * The first of the candidate keys that appears in any bag — resolves the logical concept (title, + * price, …) to the concrete output field the feed actually uses (`g:title` vs `title`, …). + * + * @param list> $bags + * @param list $candidates + */ + private function firstPresentField(array $bags, array $candidates): ?string + { + foreach ($candidates as $candidate) { + foreach ($bags as $bag) { + if (array_key_exists($candidate, $bag)) { + return $candidate; + } + } + } + + return null; + } + + private function isEmpty(mixed $value): bool + { + return null === $value || '' === $value || [] === $value; + } + + /** + * Advisory: a price is "empty" when it is missing, empty, or resolves to a zero amount — a + * leading numeric of 0 in a formatted string like "0.00 USD" counts as zero. + */ + private function isPriceEmpty(mixed $value): bool + { + if ($this->isEmpty($value)) { + return true; + } + + if (is_int($value) || is_float($value)) { + return 0.0 === (float) $value; + } + + if (is_string($value)) { + if (1 === preg_match('/-?\d+(?:[.,]\d+)?/', $value, $matches)) { + return 0.0 === (float) str_replace(',', '.', $matches[0]); + } + + return true; + } + + return false; + } +} diff --git a/src/Audit/FeedAuditServiceInterface.php b/src/Audit/FeedAuditServiceInterface.php new file mode 100644 index 00000000..9a90f41b --- /dev/null +++ b/src/Audit/FeedAuditServiceInterface.php @@ -0,0 +1,16 @@ +addOption('feed', null, InputOption::VALUE_REQUIRED, 'Process only the feed with this code'); + $this + ->addOption('feed', null, InputOption::VALUE_REQUIRED, 'Process only the feed with this code') + ->addOption('preview', null, InputOption::VALUE_OPTIONAL, 'Dry-run: print the funnel + a few sample rows instead of generating (optionally set the sample size, default ' . self::DEFAULT_PREVIEW_LIMIT . ')', false) + ->addOption('audit', null, InputOption::VALUE_NONE, 'Dry-run: also print fill rates and soft warnings (implies --preview)') + ; } protected function execute(InputInterface $input, OutputInterface $output): int @@ -47,6 +65,21 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Command::SUCCESS; } + $audit = true === $input->getOption('audit'); + $previewOption = $input->getOption('preview'); + + if ($audit || false !== $previewOption) { + $limit = is_numeric($previewOption) ? max(1, (int) $previewOption) : self::DEFAULT_PREVIEW_LIMIT; + + foreach ($feeds as $feed) { + if ($feed instanceof FeedInterface) { + $this->previewFeed($io, $feed, $limit, $audit); + } + } + + return Command::SUCCESS; + } + foreach ($feeds as $feed) { if (!$feed instanceof FeedInterface) { continue; @@ -59,4 +92,97 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Command::SUCCESS; } + + private function previewFeed(SymfonyStyle $io, FeedInterface $feed, int $limit, bool $audit): void + { + $contexts = $this->contextFactory->create($feed); + $context = $contexts[0] ?? new FeedContext(); + + $result = $this->previewService->preview($feed, $context, $limit); + $funnel = $result->funnel; + + $io->section(sprintf('%s (%s)', (string) $feed->getCode(), $context->key())); + $io->table( + ['Stage', 'Count'], + [ + ['Sampled', (string) $funnel->source], + ['After pre-filters', (string) $funnel->afterPreFilters], + ['After mapping & validation', (string) $funnel->afterMappingValidation], + ['After post-filters', (string) $funnel->afterPostFilters], + ['Included', (string) $funnel->included], + ], + ); + + $includedSample = array_slice($result->included, 0, self::SAMPLE_ROWS); + if ([] !== $includedSample) { + $io->writeln('Included sample:'); + foreach ($includedSample as $bag) { + $io->writeln(' - ' . $this->summarizeBag($bag)); + } + $io->newLine(); + } + + $excludedSample = array_slice($result->excluded, 0, self::SAMPLE_ROWS); + if ([] !== $excludedSample) { + $io->writeln('Excluded sample:'); + foreach ($excludedSample as $row) { + $io->writeln(sprintf(' - %s: %s', $row['item'] ?? '(no id)', $row['reason'])); + } + $io->newLine(); + } + + if ($audit) { + $this->printAudit($io, $this->auditService->audit($result)); + } + } + + private function printAudit(SymfonyStyle $io, AuditReport $report): void + { + if ([] !== $report->fillRates) { + $rows = []; + foreach ($report->fillRates as $field => $rate) { + $rows[] = [$field, sprintf('%.1f%%', $rate * 100)]; + } + $io->writeln('Fill rates:'); + $io->table(['Field', 'Fill rate'], $rows); + } + + if ([] !== $report->warnings) { + $rows = []; + foreach ($report->warnings as $warning) { + $rows[] = [$warning['type'], $warning['field'], (string) $warning['count']]; + } + $io->writeln('Soft warnings:'); + $io->table(['Type', 'Field', 'Count'], $rows); + } + } + + /** + * @param array $bag + */ + private function summarizeBag(array $bag): string + { + $parts = []; + foreach ($bag as $key => $value) { + $parts[] = sprintf('%s=%s', $key, $this->stringifyValue($value)); + } + + return implode(', ', array_slice($parts, 0, self::SAMPLE_FIELDS)); + } + + private function stringifyValue(mixed $value): string + { + if (is_scalar($value)) { + return (string) $value; + } + + if (is_array($value)) { + return implode('|', array_map( + static fn (mixed $item): string => is_scalar($item) ? (string) $item : get_debug_type($item), + $value, + )); + } + + return get_debug_type($value); + } } diff --git a/src/Controller/Admin/PreviewFeedAction.php b/src/Controller/Admin/PreviewFeedAction.php new file mode 100644 index 00000000..90e155c7 --- /dev/null +++ b/src/Controller/Admin/PreviewFeedAction.php @@ -0,0 +1,63 @@ +feedRepository->find($id); + if (!$feed instanceof FeedInterface) { + throw new NotFoundHttpException(sprintf('There is no feed with id "%s"', $id)); + } + + $contexts = $this->contextFactory->create($feed); + $context = $contexts[0] ?? new FeedContext(); + + $preview = $this->previewService->preview($feed, $context); + $audit = $this->auditService->audit($preview); + + $columns = []; + foreach ($preview->included as $bag) { + foreach (array_keys($bag) as $key) { + if (!in_array($key, $columns, true)) { + $columns[] = $key; + } + } + } + + return new Response($this->twig->render('@SetonoSyliusFeedPlugin/admin/feed/preview.html.twig', [ + 'feed' => $feed, + 'context' => $context, + 'preview' => $preview, + 'audit' => $audit, + 'columns' => $columns, + ])); + } +} diff --git a/src/Generator/FeedGenerator.php b/src/Generator/FeedGenerator.php index 776caed2..eb821834 100644 --- a/src/Generator/FeedGenerator.php +++ b/src/Generator/FeedGenerator.php @@ -13,9 +13,7 @@ use Setono\SyliusFeedPlugin\Format\FormatRegistryInterface; use Setono\SyliusFeedPlugin\Item\FeedItem; use Setono\SyliusFeedPlugin\Lookup\LookupReferenceResolverInterface; -use Setono\SyliusFeedPlugin\Mapping\FieldMapping; -use Setono\SyliusFeedPlugin\MappingPreset\MappingPresetRegistryInterface; -use Setono\SyliusFeedPlugin\Model\FeedFieldInterface; +use Setono\SyliusFeedPlugin\Mapping\MappingResolverInterface; use Setono\SyliusFeedPlugin\Model\FeedInterface; use Setono\SyliusFeedPlugin\Model\FeedSourceInterface; use Setono\SyliusFeedPlugin\Validator\FeedItemValidatorInterface; @@ -33,7 +31,7 @@ final class FeedGenerator implements FeedGeneratorInterface { public function __construct( private readonly FeedTypeRegistryInterface $feedTypeRegistry, - private readonly MappingPresetRegistryInterface $mappingPresetRegistry, + private readonly MappingResolverInterface $mappingResolver, private readonly FormatRegistryInterface $formatRegistry, private readonly FeedWriterRegistryInterface $writerRegistry, private readonly FieldMappingEvaluatorInterface $fieldMappingEvaluator, @@ -72,7 +70,7 @@ public function generate(FeedInterface $feed, FeedContext $context): GenerationR foreach ($this->sortedSources($feed) as $source) { $feedType = $this->feedTypeRegistry->get((string) $source->getFeedType()); $availableFields = $feedType->getAvailableFields(); - $mappings = $this->resolveMappings($feed, $source); + $mappings = $this->mappingResolver->resolve($feed, $source); $filterSet = new FilterSet($source->getFilters()); foreach ($feedType->getDataSource()->getItems($context, $filterSet) as $entity) { @@ -149,33 +147,6 @@ private function resolveItemIdentifier(FeedItem $item): ?string return null; } - /** - * The admin-editable FeedField rows are the source of truth once a source has any; the matching - * MappingPreset is only the fallback for a source that was never seeded/edited (§10). - * - * @return list - */ - private function resolveMappings(FeedInterface $feed, FeedSourceInterface $source): array - { - $fields = $source->getFields()->toArray(); - if ([] !== $fields) { - usort( - $fields, - static fn (FeedFieldInterface $a, FeedFieldInterface $b): int => ($a->getPosition() ?? 0) <=> ($b->getPosition() ?? 0), - ); - - return array_map(FieldMapping::fromFeedField(...), $fields); - } - - foreach ($this->mappingPresetRegistry->forFeedType((string) $source->getFeedType()) as $preset) { - if ($preset->getFormat() === $feed->getFormat()) { - return $preset->getMapping(); - } - } - - return []; - } - /** * The union of every source's output fields, in first-seen order — the CSV header. * @@ -185,7 +156,7 @@ private function unionHeader(FeedInterface $feed): array { $header = []; foreach ($this->sortedSources($feed) as $source) { - foreach ($this->resolveMappings($feed, $source) as $mapping) { + foreach ($this->mappingResolver->resolve($feed, $source) as $mapping) { $outputField = $mapping->getOutputField(); if (!in_array($outputField, $header, true)) { $header[] = $outputField; diff --git a/src/Mapping/MappingResolver.php b/src/Mapping/MappingResolver.php new file mode 100644 index 00000000..8ca75b7e --- /dev/null +++ b/src/Mapping/MappingResolver.php @@ -0,0 +1,41 @@ +getFields()->toArray(); + if ([] !== $fields) { + usort( + $fields, + static fn (FeedFieldInterface $a, FeedFieldInterface $b): int => ($a->getPosition() ?? 0) <=> ($b->getPosition() ?? 0), + ); + + return array_map(FieldMapping::fromFeedField(...), $fields); + } + + foreach ($this->mappingPresetRegistry->forFeedType((string) $source->getFeedType()) as $preset) { + if ($preset->getFormat() === $feed->getFormat()) { + return $preset->getMapping(); + } + } + + return []; + } +} diff --git a/src/Mapping/MappingResolverInterface.php b/src/Mapping/MappingResolverInterface.php new file mode 100644 index 00000000..2c7e1f92 --- /dev/null +++ b/src/Mapping/MappingResolverInterface.php @@ -0,0 +1,22 @@ + + */ + public function resolve(FeedInterface $feed, FeedSourceInterface $source): array; +} diff --git a/src/Preview/PreviewFunnel.php b/src/Preview/PreviewFunnel.php new file mode 100644 index 00000000..d42ea3e1 --- /dev/null +++ b/src/Preview/PreviewFunnel.php @@ -0,0 +1,23 @@ +> $included the mapped output bags of included sample items + * @param list $excluded excluded sample items + why + */ + public function __construct( + public readonly PreviewFunnel $funnel, + public readonly array $included, + public readonly array $excluded, + ) { + } +} diff --git a/src/Preview/PreviewService.php b/src/Preview/PreviewService.php new file mode 100644 index 00000000..fb0325bb --- /dev/null +++ b/src/Preview/PreviewService.php @@ -0,0 +1,235 @@ +formatRegistry->get((string) $feed->getFormat()); + $requiredFields = $format->getRequiredFields(); + $itemValidationGroups = $format->getItemValidationGroups(); + + $sampled = 0; + $preExcluded = 0; + $postExcluded = 0; + // The item-built event (skip) and validation both drop after mapping — grouped as one stage. + $mappingValidationExcluded = 0; + $includedCount = 0; + + $included = []; + $excluded = []; + + foreach ($this->sortedSources($feed) as $feedSource) { + $feedType = $this->feedTypeRegistry->get((string) $feedSource->getFeedType()); + $availableFields = $feedType->getAvailableFields(); + $mappings = $this->mappingResolver->resolve($feed, $feedSource); + $filterSet = new FilterSet($feedSource->getFilters()); + + $sampledFromSource = 0; + foreach ($feedType->getDataSource()->getItems($context, $filterSet) as $entity) { + if ($sampledFromSource >= $limit) { + break; + } + ++$sampledFromSource; + ++$sampled; + + $item = $feedType->createItem($entity, $context); + SourceResolverBinder::bind($item, $availableFields, $this->lookupReferenceResolver); + + $excludingPreFilter = $this->filterEvaluator->excludedBy($item, $filterSet->getPreFilters()); + if (null !== $excludingPreFilter) { + ++$preExcluded; + $excluded[] = ['item' => $this->resolveItemIdentifier($item), 'reason' => sprintf('filter:pre:%s', (string) $excludingPreFilter->getField())]; + + continue; + } + + $this->fieldMappingEvaluator->apply($item, $mappings, $availableFields); + + $excludingPostFilter = $this->filterEvaluator->excludedBy($item, $filterSet->getPostFilters()); + if (null !== $excludingPostFilter) { + ++$postExcluded; + $excluded[] = ['item' => $this->resolveItemIdentifier($item), 'reason' => sprintf('filter:post:%s', (string) $excludingPostFilter->getField())]; + + continue; + } + + $this->eventDispatcher->dispatch(new FeedItemBuiltEvent($item)); + + if ($item->isSkipped()) { + ++$mappingValidationExcluded; + $excluded[] = ['item' => $this->resolveItemIdentifier($item), 'reason' => 'skipped']; + + continue; + } + + $violations = $this->feedItemValidator->validate($item, $requiredFields, $itemValidationGroups); + if ([] !== $violations) { + ++$mappingValidationExcluded; + $excluded[] = ['item' => $this->resolveItemIdentifier($item), 'reason' => 'validation:' . implode('; ', $violations)]; + + continue; + } + + ++$includedCount; + $included[] = $item->all(); + } + } + + $afterPreFilters = $sampled - $preExcluded; + $afterMappingValidation = $afterPreFilters - $mappingValidationExcluded; + $afterPostFilters = $afterMappingValidation - $postExcluded; + + $funnel = new PreviewFunnel($sampled, $afterPreFilters, $afterMappingValidation, $afterPostFilters, $includedCount); + + return new PreviewResult( + $funnel, + array_slice($included, 0, $limit), + array_slice($excluded, 0, $limit), + ); + } + + public function previewItem(FeedInterface $feed, FeedContext $context, string $id): array + { + $format = $this->formatRegistry->get((string) $feed->getFormat()); + $requiredFields = $format->getRequiredFields(); + $itemValidationGroups = $format->getItemValidationGroups(); + + foreach ($this->sortedSources($feed) as $feedSource) { + $feedType = $this->feedTypeRegistry->get((string) $feedSource->getFeedType()); + $availableFields = $feedType->getAvailableFields(); + $mappings = $this->mappingResolver->resolve($feed, $feedSource); + $filterSet = new FilterSet($feedSource->getFilters()); + + $scanned = 0; + foreach ($feedType->getDataSource()->getItems($context, $filterSet) as $entity) { + if ($scanned >= self::PREVIEW_ITEM_SCAN_CAP) { + break; + } + ++$scanned; + + $item = $feedType->createItem($entity, $context); + SourceResolverBinder::bind($item, $availableFields, $this->lookupReferenceResolver); + + // The pre-filter is evaluated before mapping in the real pipeline, but we always map + // so the output id resolves and we can match even a pre-filtered item. + $excludingPreFilter = $this->filterEvaluator->excludedBy($item, $filterSet->getPreFilters()); + $this->fieldMappingEvaluator->apply($item, $mappings, $availableFields); + + if ($this->resolveItemIdentifier($item) !== $id) { + continue; + } + + if (null !== $excludingPreFilter) { + return $this->itemVerdict(false, $item->all(), sprintf('filter:pre:%s', (string) $excludingPreFilter->getField())); + } + + $excludingPostFilter = $this->filterEvaluator->excludedBy($item, $filterSet->getPostFilters()); + if (null !== $excludingPostFilter) { + return $this->itemVerdict(false, $item->all(), sprintf('filter:post:%s', (string) $excludingPostFilter->getField())); + } + + $this->eventDispatcher->dispatch(new FeedItemBuiltEvent($item)); + + if ($item->isSkipped()) { + return $this->itemVerdict(false, $item->all(), 'skipped'); + } + + $violations = $this->feedItemValidator->validate($item, $requiredFields, $itemValidationGroups); + if ([] !== $violations) { + return $this->itemVerdict(false, $item->all(), 'validation:' . implode('; ', $violations)); + } + + return $this->itemVerdict(true, $item->all(), null); + } + } + + return $this->itemVerdict(false, [], 'not_found'); + } + + /** + * @param array $output + * + * @return array{included: bool, output: array, reason: ?string} + */ + private function itemVerdict(bool $included, array $output, ?string $reason): array + { + return ['included' => $included, 'output' => $output, 'reason' => $reason]; + } + + /** + * The item's identity for reporting: its output id if the mapping has produced one, else null + * (mirrors the generator, §11). + */ + private function resolveItemIdentifier(FeedItem $item): ?string + { + foreach (['g:id', 'id'] as $key) { + $value = $item->get($key); + if (is_scalar($value) && '' !== (string) $value) { + return (string) $value; + } + } + + return null; + } + + /** + * @return list + */ + private function sortedSources(FeedInterface $feed): array + { + $sources = array_values($feed->getSources()->toArray()); + usort( + $sources, + static fn (FeedSourceInterface $a, FeedSourceInterface $b): int => ($a->getPosition() ?? 0) <=> ($b->getPosition() ?? 0), + ); + + return $sources; + } +} diff --git a/src/Preview/PreviewServiceInterface.php b/src/Preview/PreviewServiceInterface.php new file mode 100644 index 00000000..82a6eb28 --- /dev/null +++ b/src/Preview/PreviewServiceInterface.php @@ -0,0 +1,31 @@ +, reason: ?string} + */ + public function previewItem(FeedInterface $feed, FeedContext $context, string $id): array; +} diff --git a/src/Resources/config/app/config.yaml b/src/Resources/config/app/config.yaml index ca683e1b..535df5d9 100644 --- a/src/Resources/config/app/config.yaml +++ b/src/Resources/config/app/config.yaml @@ -69,6 +69,15 @@ sylius_grid: route: setono_sylius_feed_admin_feed_results parameters: id: resource.id + preview: + type: default + label: setono_sylius_feed.ui.preview + icon: search + options: + link: + route: setono_sylius_feed_admin_feed_preview + parameters: + id: resource.id update: type: update delete: diff --git a/src/Resources/config/routes/admin.yaml b/src/Resources/config/routes/admin.yaml index 1c4007e2..726f95e5 100644 --- a/src/Resources/config/routes/admin.yaml +++ b/src/Resources/config/routes/admin.yaml @@ -24,6 +24,12 @@ setono_sylius_feed_admin_feed_results: defaults: _controller: 'Setono\SyliusFeedPlugin\Controller\Admin\FeedResultsAction' +setono_sylius_feed_admin_feed_preview: + path: /feeds/{id}/preview + methods: [GET] + defaults: + _controller: 'Setono\SyliusFeedPlugin\Controller\Admin\PreviewFeedAction' + setono_sylius_feed_admin_feed_publish_context: path: /feeds/{id}/publish/{contextKey} methods: [GET] diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 015a9759..9e319c3d 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -20,6 +20,7 @@ + diff --git a/src/Resources/config/services/command.xml b/src/Resources/config/services/command.xml index 505b2cc3..20ed66ae 100644 --- a/src/Resources/config/services/command.xml +++ b/src/Resources/config/services/command.xml @@ -7,6 +7,9 @@ + + + diff --git a/src/Resources/config/services/controller.xml b/src/Resources/config/services/controller.xml index 44dc1872..e60f2b00 100644 --- a/src/Resources/config/services/controller.xml +++ b/src/Resources/config/services/controller.xml @@ -31,6 +31,15 @@ + + + + + + + + + diff --git a/src/Resources/config/services/generator.xml b/src/Resources/config/services/generator.xml index 5e4e9578..b5148e0a 100644 --- a/src/Resources/config/services/generator.xml +++ b/src/Resources/config/services/generator.xml @@ -4,6 +4,11 @@ xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> + + + + + @@ -25,7 +30,7 @@ - + diff --git a/src/Resources/config/services/preview.xml b/src/Resources/config/services/preview.xml new file mode 100644 index 00000000..e928e11d --- /dev/null +++ b/src/Resources/config/services/preview.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/src/Resources/translations/messages.en.yaml b/src/Resources/translations/messages.en.yaml index 6d02e75e..9822f732 100644 --- a/src/Resources/translations/messages.en.yaml +++ b/src/Resources/translations/messages.en.yaml @@ -18,6 +18,32 @@ setono_sylius_feed: file: File size: Size no_results: This feed has not been generated yet + preview: Preview + preview_context: Previewing context + funnel: Include funnel + funnel_source: Sampled + funnel_after_pre_filters: After pre-filters + funnel_after_mapping_validation: After mapping & validation + funnel_after_post_filters: After post-filters + funnel_included: Included + audit: Audit + fill_rates: Fill rate + soft_warnings: Soft warnings + distributions: Distributions + included_sample: Included sample + excluded_sample: Excluded sample + no_included_items: No items would be included in this preview + no_excluded_items: No items were excluded in this preview + item: Item + field: Field + value: Value + count: Count + type: Type + reason: Reason + warning: + title_too_long: Title longer than 150 characters + description_contains_html: Description still contains HTML tags + price_empty: Price is zero or empty form: feed: name: Name diff --git a/src/Resources/views/admin/feed/preview.html.twig b/src/Resources/views/admin/feed/preview.html.twig new file mode 100644 index 00000000..6ff76717 --- /dev/null +++ b/src/Resources/views/admin/feed/preview.html.twig @@ -0,0 +1,154 @@ +{% extends '@SyliusAdmin/layout.html.twig' %} + +{% block content %} +
+

{{ 'setono_sylius_feed.ui.preview'|trans }} — {{ feed.code }}

+ +
{{ 'setono_sylius_feed.ui.preview_context'|trans }}: {{ context.key }}
+ +

{{ 'setono_sylius_feed.ui.funnel'|trans }}

+ + + + + + + + + + + + + + + + + + + + + + + +
{{ 'setono_sylius_feed.ui.funnel_source'|trans }}{{ preview.funnel.source }}
{{ 'setono_sylius_feed.ui.funnel_after_pre_filters'|trans }}{{ preview.funnel.afterPreFilters }}
{{ 'setono_sylius_feed.ui.funnel_after_mapping_validation'|trans }}{{ preview.funnel.afterMappingValidation }}
{{ 'setono_sylius_feed.ui.funnel_after_post_filters'|trans }}{{ preview.funnel.afterPostFilters }}
{{ 'setono_sylius_feed.ui.funnel_included'|trans }}{{ preview.funnel.included }}
+ +

{{ 'setono_sylius_feed.ui.audit'|trans }}

+ +

{{ 'setono_sylius_feed.ui.fill_rates'|trans }}

+ + + + + + + + + {% for field, rate in audit.fillRates %} + + + + + {% else %} + + + + {% endfor %} + +
{{ 'setono_sylius_feed.ui.field'|trans }}{{ 'setono_sylius_feed.ui.fill_rates'|trans }}
{{ field }}{{ (rate * 100)|round(1) }}%
{{ 'setono_sylius_feed.ui.no_included_items'|trans }}
+ + {% if audit.warnings is not empty %} +

{{ 'setono_sylius_feed.ui.soft_warnings'|trans }}

+ + + + + + + + + + {% for warning in audit.warnings %} + + + + + + {% endfor %} + +
{{ 'setono_sylius_feed.ui.type'|trans }}{{ 'setono_sylius_feed.ui.field'|trans }}{{ 'setono_sylius_feed.ui.count'|trans }}
{{ ('setono_sylius_feed.ui.warning.' ~ warning.type)|trans }}{{ warning.field }}{{ warning.count }}
+ {% endif %} + + {% if audit.distributions is not empty %} +

{{ 'setono_sylius_feed.ui.distributions'|trans }}

+ {% for field, values in audit.distributions %} +

{{ field }}

+ + + + + + + + + {% for value, count in values %} + + + + + {% endfor %} + +
{{ 'setono_sylius_feed.ui.value'|trans }}{{ 'setono_sylius_feed.ui.count'|trans }}
{{ value }}{{ count }}
+ {% endfor %} + {% endif %} + +

{{ 'setono_sylius_feed.ui.included_sample'|trans }}

+
+ + + + {% for column in columns %} + + {% endfor %} + + + + {% for bag in preview.included %} + + {% for column in columns %} + + {% endfor %} + + {% else %} + + + + {% endfor %} + +
{{ column }}
{{ bag[column] is iterable ? bag[column]|join(', ') : bag[column] }}
{{ 'setono_sylius_feed.ui.no_included_items'|trans }}
+
+ +

{{ 'setono_sylius_feed.ui.excluded_sample'|trans }}

+ + + + + + + + + {% for row in preview.excluded %} + + + + + {% else %} + + + + {% endfor %} + +
{{ 'setono_sylius_feed.ui.item'|trans }}{{ 'setono_sylius_feed.ui.reason'|trans }}
{{ row.item ?? '—' }}{{ row.reason }}
{{ 'setono_sylius_feed.ui.no_excluded_items'|trans }}
+ + {{ 'sylius.ui.back'|trans }} +
+{% endblock %} diff --git a/tests/Unit/Audit/FeedAuditServiceTest.php b/tests/Unit/Audit/FeedAuditServiceTest.php new file mode 100644 index 00000000..8de6f8ff --- /dev/null +++ b/tests/Unit/Audit/FeedAuditServiceTest.php @@ -0,0 +1,63 @@ + 150-char title, a zero price), and counts value + * distributions (availability) — without excluding anything. + */ +final class FeedAuditServiceTest extends TestCase +{ + /** + * @test + */ + public function it_computes_fill_rates_soft_warnings_and_distributions(): void + { + $longTitle = str_repeat('a', 151); + + $result = new PreviewResult( + new PreviewFunnel(3, 3, 3, 3, 3), + [ + ['g:id' => 'SKU-1', 'g:title' => $longTitle, 'g:gtin' => '123', 'g:availability' => 'in_stock', 'g:price' => '9.99 USD'], + ['g:id' => 'SKU-2', 'g:title' => 'Short', 'g:availability' => 'in_stock', 'g:price' => '0.00 USD'], + ['g:id' => 'SKU-3', 'g:title' => 'Also short', 'g:gtin' => '', 'g:availability' => 'out_of_stock', 'g:price' => '5.00 USD'], + ], + [], + ); + + $report = (new FeedAuditService())->audit($result); + + // fill rates: g:id is present and non-empty in all 3 items; g:gtin only in 1 of 3 + self::assertSame(1.0, $report->fillRates['g:id']); + self::assertEqualsWithDelta(1 / 3, $report->fillRates['g:gtin'], 0.0001); + + // soft warnings: exactly one over-long title and exactly one zero price + self::assertContains(['type' => 'title_too_long', 'field' => 'g:title', 'count' => 1], $report->warnings); + self::assertContains(['type' => 'price_empty', 'field' => 'g:price', 'count' => 1], $report->warnings); + + // distribution: availability value counts + self::assertSame(['in_stock' => 2, 'out_of_stock' => 1], $report->distributions['g:availability']); + } + + /** + * @test + */ + public function it_returns_empty_reports_when_nothing_is_included(): void + { + $result = new PreviewResult(new PreviewFunnel(0, 0, 0, 0, 0), [], []); + + $report = (new FeedAuditService())->audit($result); + + self::assertSame([], $report->fillRates); + self::assertSame([], $report->warnings); + self::assertSame([], $report->distributions); + } +} diff --git a/tests/Unit/Command/ProcessFeedCommandTest.php b/tests/Unit/Command/ProcessFeedCommandTest.php index 94f18997..8d2cd2c7 100644 --- a/tests/Unit/Command/ProcessFeedCommandTest.php +++ b/tests/Unit/Command/ProcessFeedCommandTest.php @@ -7,9 +7,16 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\PhpUnit\ProphecyTrait; +use Setono\SyliusFeedPlugin\Audit\AuditReport; +use Setono\SyliusFeedPlugin\Audit\FeedAuditServiceInterface; use Setono\SyliusFeedPlugin\Command\ProcessFeedCommand; +use Setono\SyliusFeedPlugin\Context\ContextFactoryInterface; +use Setono\SyliusFeedPlugin\Context\FeedContext; use Setono\SyliusFeedPlugin\Message\Command\ProcessFeed; use Setono\SyliusFeedPlugin\Model\FeedInterface; +use Setono\SyliusFeedPlugin\Preview\PreviewFunnel; +use Setono\SyliusFeedPlugin\Preview\PreviewResult; +use Setono\SyliusFeedPlugin\Preview\PreviewServiceInterface; use Sylius\Component\Resource\Repository\RepositoryInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; @@ -42,7 +49,7 @@ public function it_dispatches_a_process_message_for_each_enabled_feed(): void $commandBus = $this->prophesize(MessageBusInterface::class); $commandBus->dispatch(Argument::type(ProcessFeed::class))->willReturn(new Envelope(new \stdClass()))->shouldBeCalledOnce(); - $tester = new CommandTester(new ProcessFeedCommand($repository->reveal(), $commandBus->reveal())); + $tester = new CommandTester($this->command($repository->reveal(), $commandBus->reveal())); $exitCode = $tester->execute([]); self::assertSame(Command::SUCCESS, $exitCode); @@ -62,7 +69,7 @@ public function it_dispatches_only_the_requested_feed(): void $commandBus = $this->prophesize(MessageBusInterface::class); $commandBus->dispatch(Argument::type(ProcessFeed::class))->willReturn(new Envelope(new \stdClass()))->shouldBeCalledOnce(); - $tester = new CommandTester(new ProcessFeedCommand($repository->reveal(), $commandBus->reveal())); + $tester = new CommandTester($this->command($repository->reveal(), $commandBus->reveal())); self::assertSame(Command::SUCCESS, $tester->execute(['--feed' => 'google'])); } @@ -78,7 +85,7 @@ public function it_warns_when_there_are_no_feeds_to_process(): void $commandBus = $this->prophesize(MessageBusInterface::class); $commandBus->dispatch(Argument::cetera())->shouldNotBeCalled(); - $tester = new CommandTester(new ProcessFeedCommand($repository->reveal(), $commandBus->reveal())); + $tester = new CommandTester($this->command($repository->reveal(), $commandBus->reveal())); $exitCode = $tester->execute([]); self::assertSame(Command::SUCCESS, $exitCode); @@ -96,8 +103,107 @@ public function it_skips_repository_results_that_are_not_feeds(): void $commandBus = $this->prophesize(MessageBusInterface::class); $commandBus->dispatch(Argument::cetera())->shouldNotBeCalled(); - $tester = new CommandTester(new ProcessFeedCommand($repository->reveal(), $commandBus->reveal())); + $tester = new CommandTester($this->command($repository->reveal(), $commandBus->reveal())); self::assertSame(Command::SUCCESS, $tester->execute([])); } + + /** + * @test + */ + public function it_previews_the_funnel_instead_of_generating_when_preview_is_passed(): void + { + $feed = $this->feed(); + $context = new FeedContext(null, 'en_US', 'USD'); + + $repository = $this->prophesize(RepositoryInterface::class); + $repository->findBy(['enabled' => true])->willReturn([$feed]); + + $commandBus = $this->prophesize(MessageBusInterface::class); + $commandBus->dispatch(Argument::cetera())->shouldNotBeCalled(); + + $contextFactory = $this->prophesize(ContextFactoryInterface::class); + $contextFactory->create($feed)->willReturn([$context]); + + $preview = new PreviewResult( + new PreviewFunnel(3, 2, 1, 1, 1), + [['id' => 'SKU-1']], + [['item' => 'SKU-2', 'reason' => 'filter:pre:availability']], + ); + $previewService = $this->prophesize(PreviewServiceInterface::class); + $previewService->preview($feed, $context, 3)->willReturn($preview)->shouldBeCalledOnce(); + + $auditService = $this->prophesize(FeedAuditServiceInterface::class); + $auditService->audit(Argument::cetera())->shouldNotBeCalled(); + + $tester = new CommandTester($this->command( + $repository->reveal(), + $commandBus->reveal(), + $contextFactory->reveal(), + $previewService->reveal(), + $auditService->reveal(), + )); + + self::assertSame(Command::SUCCESS, $tester->execute(['--preview' => '3'])); + + $display = $tester->getDisplay(); + self::assertStringContainsString('SKU-1', $display); + self::assertStringContainsString('filter:pre:availability', $display); + } + + /** + * @test + */ + public function it_prints_the_audit_when_audit_is_passed(): void + { + $feed = $this->feed(); + $context = new FeedContext(null, 'en_US', 'USD'); + + $repository = $this->prophesize(RepositoryInterface::class); + $repository->findBy(['enabled' => true])->willReturn([$feed]); + + $commandBus = $this->prophesize(MessageBusInterface::class); + $commandBus->dispatch(Argument::cetera())->shouldNotBeCalled(); + + $contextFactory = $this->prophesize(ContextFactoryInterface::class); + $contextFactory->create($feed)->willReturn([$context]); + + $preview = new PreviewResult(new PreviewFunnel(1, 1, 1, 1, 1), [['g:title' => 'x']], []); + $previewService = $this->prophesize(PreviewServiceInterface::class); + $previewService->preview($feed, $context, 50)->willReturn($preview); + + $audit = new AuditReport(['g:title' => 1.0], [['type' => 'title_too_long', 'field' => 'g:title', 'count' => 1]], []); + $auditService = $this->prophesize(FeedAuditServiceInterface::class); + $auditService->audit($preview)->willReturn($audit)->shouldBeCalledOnce(); + + $tester = new CommandTester($this->command( + $repository->reveal(), + $commandBus->reveal(), + $contextFactory->reveal(), + $previewService->reveal(), + $auditService->reveal(), + )); + + self::assertSame(Command::SUCCESS, $tester->execute(['--audit' => true])); + + $display = $tester->getDisplay(); + self::assertStringContainsString('title_too_long', $display); + self::assertStringContainsString('100.0%', $display); + } + + private function command( + RepositoryInterface $repository, + MessageBusInterface $commandBus, + ?ContextFactoryInterface $contextFactory = null, + ?PreviewServiceInterface $previewService = null, + ?FeedAuditServiceInterface $auditService = null, + ): ProcessFeedCommand { + return new ProcessFeedCommand( + $repository, + $commandBus, + $contextFactory ?? $this->prophesize(ContextFactoryInterface::class)->reveal(), + $previewService ?? $this->prophesize(PreviewServiceInterface::class)->reveal(), + $auditService ?? $this->prophesize(FeedAuditServiceInterface::class)->reveal(), + ); + } } diff --git a/tests/Unit/Controller/Admin/PreviewFeedActionTest.php b/tests/Unit/Controller/Admin/PreviewFeedActionTest.php new file mode 100644 index 00000000..54ff39bd --- /dev/null +++ b/tests/Unit/Controller/Admin/PreviewFeedActionTest.php @@ -0,0 +1,92 @@ +setCode('google'); + + $repository = $this->prophesize(FeedRepositoryInterface::class); + $repository->find(1)->willReturn($feed); + + $context = new FeedContext(null, 'en_US', 'USD'); + $contextFactory = $this->prophesize(ContextFactoryInterface::class); + $contextFactory->create($feed)->willReturn([$context]); + + $preview = new PreviewResult( + new PreviewFunnel(2, 2, 1, 1, 1), + [['id' => 'SKU-1', 'title' => 'Shoe']], + [['item' => null, 'reason' => 'validation:missing']], + ); + $previewService = $this->prophesize(PreviewServiceInterface::class); + $previewService->preview($feed, $context)->willReturn($preview); + + $audit = new AuditReport(['id' => 1.0], [], []); + $auditService = $this->prophesize(FeedAuditServiceInterface::class); + $auditService->audit($preview)->willReturn($audit); + + $twig = $this->prophesize(Environment::class); + $twig->render( + '@SetonoSyliusFeedPlugin/admin/feed/preview.html.twig', + Argument::that(static fn (array $context): bool => $context['feed'] === $feed && + $context['preview'] === $preview && + $context['audit'] === $audit && + ['id', 'title'] === $context['columns']), + )->willReturn('preview'); + + $response = (new PreviewFeedAction( + $repository->reveal(), + $contextFactory->reveal(), + $previewService->reveal(), + $auditService->reveal(), + $twig->reveal(), + ))(1); + + self::assertSame('preview', $response->getContent()); + } + + /** + * @test + */ + public function it_404s_for_a_missing_feed(): void + { + $repository = $this->prophesize(FeedRepositoryInterface::class); + $repository->find(999)->willReturn(null); + + $this->expectException(NotFoundHttpException::class); + + (new PreviewFeedAction( + $repository->reveal(), + $this->prophesize(ContextFactoryInterface::class)->reveal(), + $this->prophesize(PreviewServiceInterface::class)->reveal(), + $this->prophesize(FeedAuditServiceInterface::class)->reveal(), + $this->prophesize(Environment::class)->reveal(), + ))(999); + } +} diff --git a/tests/Unit/Generator/FeedGeneratorFilterTest.php b/tests/Unit/Generator/FeedGeneratorFilterTest.php index 246f6b4e..54d042ab 100644 --- a/tests/Unit/Generator/FeedGeneratorFilterTest.php +++ b/tests/Unit/Generator/FeedGeneratorFilterTest.php @@ -24,6 +24,7 @@ use Setono\SyliusFeedPlugin\Lookup\InMemoryLookup; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\FieldType; +use Setono\SyliusFeedPlugin\Mapping\MappingResolver; use Setono\SyliusFeedPlugin\Mapping\ScopeDimension; use Setono\SyliusFeedPlugin\MappingPreset\MappingPresetRegistryInterface; use Setono\SyliusFeedPlugin\Model\FeedField; @@ -125,7 +126,7 @@ private function createGenerator(): FeedGenerator return new FeedGenerator( $feedTypeRegistry->reveal(), - $presetRegistry->reveal(), + new MappingResolver($presetRegistry->reveal()), $formatRegistry->reveal(), $writerRegistry->reveal(), new FieldMappingEvaluator( diff --git a/tests/Unit/Generator/FeedGeneratorMemoryTest.php b/tests/Unit/Generator/FeedGeneratorMemoryTest.php index b92746ee..1da428ea 100644 --- a/tests/Unit/Generator/FeedGeneratorMemoryTest.php +++ b/tests/Unit/Generator/FeedGeneratorMemoryTest.php @@ -23,6 +23,7 @@ use Setono\SyliusFeedPlugin\Lookup\InMemoryLookup; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\FieldType; +use Setono\SyliusFeedPlugin\Mapping\MappingResolver; use Setono\SyliusFeedPlugin\Mapping\ScopeDimension; use Setono\SyliusFeedPlugin\MappingPreset\GoogleShoppingMappingPreset; use Setono\SyliusFeedPlugin\MappingPreset\MappingPresetRegistryInterface; @@ -99,7 +100,7 @@ private function createGenerator(Filesystem $filesystem): FeedGenerator return new FeedGenerator( $feedTypeRegistry->reveal(), - $presetRegistry->reveal(), + new MappingResolver($presetRegistry->reveal()), $formatRegistry->reveal(), $writerRegistry->reveal(), new FieldMappingEvaluator( diff --git a/tests/Unit/Generator/FeedGeneratorTest.php b/tests/Unit/Generator/FeedGeneratorTest.php index 2b673eb5..2b1dbc64 100644 --- a/tests/Unit/Generator/FeedGeneratorTest.php +++ b/tests/Unit/Generator/FeedGeneratorTest.php @@ -26,6 +26,7 @@ use Setono\SyliusFeedPlugin\Lookup\InMemoryLookup; use Setono\SyliusFeedPlugin\Mapping\FieldDefinition; use Setono\SyliusFeedPlugin\Mapping\FieldType; +use Setono\SyliusFeedPlugin\Mapping\MappingResolver; use Setono\SyliusFeedPlugin\Mapping\ScopeDimension; use Setono\SyliusFeedPlugin\MappingPreset\GoogleShoppingMappingPreset; use Setono\SyliusFeedPlugin\MappingPreset\MappingPresetRegistryInterface; @@ -259,7 +260,7 @@ private function createGenerator(?array $presets = null): FeedGenerator return new FeedGenerator( $feedTypeRegistry->reveal(), - $presetRegistry->reveal(), + new MappingResolver($presetRegistry->reveal()), $formatRegistry->reveal(), $writerRegistry->reveal(), new FieldMappingEvaluator( diff --git a/tests/Unit/Mapping/MappingResolverTest.php b/tests/Unit/Mapping/MappingResolverTest.php new file mode 100644 index 00000000..149a1276 --- /dev/null +++ b/tests/Unit/Mapping/MappingResolverTest.php @@ -0,0 +1,110 @@ +prophesize(FeedSourceInterface::class); + $source->getFields()->willReturn(new ArrayCollection([ + $this->feedField('title', 'title', 1), + $this->feedField('id', 'id', 0), + ])); + + $presetRegistry = $this->prophesize(MappingPresetRegistryInterface::class); + + $mappings = (new MappingResolver($presetRegistry->reveal()))->resolve( + $this->prophesize(FeedInterface::class)->reveal(), + $source->reveal(), + ); + + self::assertCount(2, $mappings); + self::assertSame('id', $mappings[0]->getOutputField()); + self::assertSame('title', $mappings[1]->getOutputField()); + } + + /** + * @test + */ + public function it_falls_back_to_the_matching_preset_when_there_are_no_feed_fields(): void + { + $source = $this->prophesize(FeedSourceInterface::class); + $source->getFields()->willReturn(new ArrayCollection()); + $source->getFeedType()->willReturn('product_variant'); + + $feed = $this->prophesize(FeedInterface::class); + $feed->getFormat()->willReturn('google_rss'); + + $expected = [FieldMapping::field('g:id', 'id')]; + + $nonMatching = $this->prophesize(MappingPresetInterface::class); + $nonMatching->getFormat()->willReturn('csv'); + + $matching = $this->prophesize(MappingPresetInterface::class); + $matching->getFormat()->willReturn('google_rss'); + $matching->getMapping()->willReturn($expected); + + $presetRegistry = $this->prophesize(MappingPresetRegistryInterface::class); + $presetRegistry->forFeedType('product_variant')->willReturn([$nonMatching->reveal(), $matching->reveal()]); + + $mappings = (new MappingResolver($presetRegistry->reveal()))->resolve($feed->reveal(), $source->reveal()); + + self::assertSame($expected, $mappings); + } + + /** + * @test + */ + public function it_returns_no_mappings_when_neither_fields_nor_a_matching_preset_exist(): void + { + $source = $this->prophesize(FeedSourceInterface::class); + $source->getFields()->willReturn(new ArrayCollection()); + $source->getFeedType()->willReturn('product_variant'); + + $feed = $this->prophesize(FeedInterface::class); + $feed->getFormat()->willReturn('google_rss'); + + $presetRegistry = $this->prophesize(MappingPresetRegistryInterface::class); + $presetRegistry->forFeedType('product_variant')->willReturn([]); + + $mappings = (new MappingResolver($presetRegistry->reveal()))->resolve($feed->reveal(), $source->reveal()); + + self::assertSame([], $mappings); + } + + private function feedField(string $outputField, string $sourceField, int $position): FeedFieldInterface + { + $field = new FeedField(); + $field->setOutputField($outputField); + $field->setSourceType('field'); + $field->setSourceValue($sourceField); + $field->setPosition($position); + + return $field; + } +} diff --git a/tests/Unit/Preview/PreviewServiceTest.php b/tests/Unit/Preview/PreviewServiceTest.php new file mode 100644 index 00000000..e77767a5 --- /dev/null +++ b/tests/Unit/Preview/PreviewServiceTest.php @@ -0,0 +1,372 @@ +createPreviewService($this->catalog()); + + $result = $service->preview($this->feed(), new FeedContext(null, 'en_US', 'USD')); + + // three sampled → one dropped by the pre-filter, one dropped by validation (empty id) → one kept + self::assertSame(3, $result->funnel->source); + self::assertSame(2, $result->funnel->afterPreFilters); + self::assertSame(1, $result->funnel->afterMappingValidation); + self::assertSame(1, $result->funnel->afterPostFilters); + self::assertSame(1, $result->funnel->included); + + self::assertCount(1, $result->included); + self::assertSame(['id' => 'SKU-1', 'title' => 'Shoe', 'availability' => 'in_stock'], $result->included[0]); + + $reasons = array_column($result->excluded, 'reason'); + self::assertContains('filter:pre:availability', $reasons); + self::assertNotEmpty(array_filter($reasons, static fn (string $reason): bool => str_starts_with($reason, 'validation:'))); + } + + /** + * @test + */ + public function it_bounds_the_sample_to_the_limit(): void + { + $entities = []; + for ($i = 1; $i <= 5; ++$i) { + $entities[] = (object) ['id' => 'SKU-' . $i, 'title' => 'Product ' . $i, 'availability' => 'in_stock']; + } + + $service = $this->createPreviewService($entities); + + $result = $service->preview($this->feed(), new FeedContext(null, 'en_US', 'USD'), 2); + + self::assertSame(2, $result->funnel->source, 'only the first $limit items per source are sampled'); + self::assertSame(2, $result->funnel->included); + self::assertCount(2, $result->included); + } + + /** + * @test + */ + public function it_previews_a_single_included_item_by_id(): void + { + $service = $this->createPreviewService($this->catalog()); + + $verdict = $service->previewItem($this->feed(), new FeedContext(null, 'en_US', 'USD'), 'SKU-1'); + + self::assertTrue($verdict['included']); + self::assertNull($verdict['reason']); + self::assertSame('SKU-1', $verdict['output']['id']); + } + + /** + * @test + */ + public function it_reports_the_rule_that_drops_a_single_item(): void + { + $service = $this->createPreviewService($this->catalog()); + + $verdict = $service->previewItem($this->feed(), new FeedContext(null, 'en_US', 'USD'), 'SKU-2'); + + self::assertFalse($verdict['included']); + self::assertSame('filter:pre:availability', $verdict['reason']); + } + + /** + * @test + */ + public function it_reports_not_found_for_an_unknown_id(): void + { + $service = $this->createPreviewService($this->catalog()); + + $verdict = $service->previewItem($this->feed(), new FeedContext(null, 'en_US', 'USD'), 'DOES-NOT-EXIST'); + + self::assertFalse($verdict['included']); + self::assertSame('not_found', $verdict['reason']); + self::assertSame([], $verdict['output']); + } + + /** + * @return list + */ + private function catalog(): array + { + return [ + (object) ['id' => 'SKU-1', 'title' => 'Shoe', 'availability' => 'in_stock'], + (object) ['id' => 'SKU-2', 'title' => 'Boot', 'availability' => 'out_of_stock'], + (object) ['id' => '', 'title' => 'Ghost', 'availability' => 'in_stock'], + ]; + } + + /** + * @param list $entities + */ + private function createPreviewService(array $entities): PreviewService + { + $feedTypeRegistry = $this->prophesize(FeedTypeRegistryInterface::class); + $feedTypeRegistry->get('product_variant')->willReturn($this->feedType($entities)); + + $presetRegistry = $this->prophesize(MappingPresetRegistryInterface::class); + + $formatRegistry = $this->prophesize(FormatRegistryInterface::class); + $formatRegistry->get('test_format')->willReturn($this->format()); + + return new PreviewService( + $feedTypeRegistry->reveal(), + new MappingResolver($presetRegistry->reveal()), + $formatRegistry->reveal(), + new FieldMappingEvaluator( + new TransformationChain(new TransformationRegistry([])), + new ReferenceResolver(), + new OperatorRegistry([new Equals(), new IsTrue()]), + new ExpressionEvaluator(new InMemoryLookup()), + new SandboxedTwigRenderer(new FeedTemplateSecurityPolicy(), new InMemoryLookup()), + new NullLookupReferenceResolver(), + ), + new FilterEvaluator(new ReferenceResolver(), new OperatorRegistry([new Equals()])), + new NullLookupReferenceResolver(), + new FeedItemValidator(Validation::createValidator(), new RequiredFieldsValidator()), + new EventDispatcher(), + ); + } + + /** + * @param list $entities + */ + private function feedType(array $entities): FeedTypeInterface + { + $fields = [ + 'id' => $this->field('id'), + 'title' => $this->field('title'), + 'availability' => $this->field('availability'), + ]; + + $dataSource = new class($entities) implements DataSourceInterface { + /** + * @param list $entities + */ + public function __construct(private readonly array $entities) + { + } + + public function getResourceClass(): string + { + return \stdClass::class; + } + + public function getItems(FeedContext $context, FilterSet $filters): iterable + { + yield from $this->entities; + } + + public function count(FeedContext $context, FilterSet $filters): int + { + return count($this->entities); + } + }; + + return new class($dataSource, $fields) implements FeedTypeInterface { + /** + * @param array $fields + */ + public function __construct( + private readonly DataSourceInterface $dataSource, + private readonly array $fields, + ) { + } + + public function getCode(): string + { + return 'product_variant'; + } + + public function getLabel(): string + { + return 'test.product_variant'; + } + + public function getDataSource(): DataSourceInterface + { + return $this->dataSource; + } + + public function createItem(object $entity, FeedContext $context): FeedItem + { + return new FeedItem($entity, $context); + } + + public function getScopeDimensions(): array + { + return []; + } + + public function getAvailableFields(): array + { + return $this->fields; + } + }; + } + + /** + * A field whose resolver reads the same-named property off the entity, so each stub entity + * resolves to its own values. + */ + private function field(string $name): FieldDefinition + { + $resolver = new class($name) implements ValueResolverInterface { + public function __construct(private readonly string $name) + { + } + + public function getName(): string + { + return $this->name; + } + + public function getLabel(): string + { + return 'test.' . $this->name; + } + + public function getType(): FieldType + { + return FieldType::STRING; + } + + public function supports(string $resourceClass): bool + { + return true; + } + + public function resolve(object $entity, FeedContext $context): mixed + { + return ((array) $entity)[$this->name] ?? null; + } + }; + + return new FieldDefinition($name, $resolver->getLabel(), FieldType::STRING, $resolver); + } + + private function format(): FormatInterface + { + return new class() implements FormatInterface { + public function getCode(): string + { + return 'test_format'; + } + + public function getWriter(): string + { + return 'csv'; + } + + public function getConfig(): WriterConfigInterface + { + return new CsvWriterConfig(); + } + + public function getRequiredFields(): array + { + return ['id']; + } + + public function getItemValidationGroups(): array + { + return []; + } + }; + } + + private function feed(string $stage = FeedFilterInterface::STAGE_PRE): FeedInterface + { + $filter = new FeedFilter(); + $filter->setField('availability'); + $filter->setOperator('equals'); + $filter->setValue("'out_of_stock'"); + $filter->setAction(FeedFilterInterface::ACTION_EXCLUDE); + $filter->setStage($stage); + + $source = $this->prophesize(FeedSourceInterface::class); + $source->getFeedType()->willReturn('product_variant'); + $source->getPosition()->willReturn(0); + $source->getFilters()->willReturn(new ArrayCollection([$filter])); + $source->getFields()->willReturn(new ArrayCollection([ + $this->feedField('id', 'id', 0), + $this->feedField('title', 'title', 1), + $this->feedField('availability', 'availability', 2), + ])); + + $feed = $this->prophesize(FeedInterface::class); + $feed->getCode()->willReturn('shop'); + $feed->getFormat()->willReturn('test_format'); + $feed->getSources()->willReturn(new ArrayCollection([$source->reveal()])); + + return $feed->reveal(); + } + + private function feedField(string $outputField, string $sourceField, int $position): FeedFieldInterface + { + $field = new FeedField(); + $field->setOutputField($outputField); + $field->setSourceType('field'); + $field->setSourceValue($sourceField); + $field->setPosition($position); + + return $field; + } +}