DEV Community: Jonas Elfering

When `@deprecated` cries wolf: Making Shopware’s next major upgrades easier

Jonas Elfering — Tue, 25 Aug 2026 06:48:24 +0000

When PHPStan reports that your extension calls a deprecated method, the expected next step is quite clear: find the replacement and migrate your code.

But what if there is no replacement?

Consider Context::scope(). Previously, its planned change for Shopware 6.8 was announced like this:

/**
 * @deprecated tag:v6.8.0 - reason:new-optional-parameter - parameter $states will be added
 */
public function scope(string $scope, \Closure $callback): mixed

Static analysis sees @deprecated and reports every call to the method. However, the method is not going away. A new optional parameter will be added, so existing calls will continue to work without any changes.

There is no alternative API to migrate to and no warning to resolve. In this situation, @deprecated is effectively crying wolf.

With Shopware 6.7.14.0, we are changing how these planned API changes are communicated. Real deprecations remain deprecations. Other backward-compatibility changes are now described with dedicated, structured PHP attributes.

The immediate result is less noise for extension developers.

Additionally, the new attributes give us a foundation for preparing extensions for Shopware 6.8 - and future major releases - before those releases arrive.

TL;DR

Shopware now uses two different signals for two different purposes:

@deprecated means that an API is obsolete and will be removed or replaced. Extension developers need to migrate away from it.
BC-change attributes describe a future change to an API that remains available, such as a new parameter, a narrower return type, or a class becoming final.

The attributes also distinguish between changes that affect code calling an API and changes that affect classes extending it.

This means deprecation warnings become trustworthy and actionable again, while planned contract changes carry enough structured information for PHPStan, Rector, IDEs, and other tools to reason about them.

We were asking `@deprecated` to do two different jobs

The commonly understood meaning of @deprecated in the PHP ecosystem is quite narrow: this API should no longer be used. IDEs may strike it through, and static-analysis tools warn when application code still depends on it.

This is useful behaviour. If a method is going away, extension developers should know about it as early as possible.

Shopware, however, also used @deprecated as an internal planning mechanism for the next major version. A reason:* marker could announce that:

A return type will become narrower.
A parameter type will change.
A new optional parameter will be added.
A class will become final or internal.
A method’s visibility will change.

These are all relevant backward-compatibility changes, but they are not all deprecations.

In many cases, the annotated method remains the correct API to use. Whether an extension needs to change anything depends on how it uses that API. Calling a method and overriding it in a subclass are two very different forms of dependency.

The standard @deprecated signal cannot express that distinction. It also cannot express a future type, parameter name, default value, or affected audience in a machine-readable form. Static-analysis tools should not be expected to understand Shopware-specific prose inside a PHPDoc annotation.

We could have recommended that every extension add more ignore patterns. But a blanket ignore would also hide genuine deprecations (the warnings where an extension developer really does need to act).

The problem was therefore not PHPStan being too strict. We were giving it the wrong signal.

A more precise model for backward-compatibility changes

The new attributes live in the Shopware\Core\Framework\Deprecation\BCChange namespace. There are currently 15 concrete attributes covering changes to types, parameters, visibility, inheritance, and exception contracts.

Every attribute belongs to one or both of two audiences:

CallSiteCompatibilityChange describes a change that may affect code calling a method.
ExtenderCompatibilityChange describes a change that may affect a subclass or method override.

For example, narrowing a parameter type affects callers that currently pass a value outside the future type. Widening that parameter type does not affect callers, but it may make an overriding method incompatible.

Return types work in the opposite direction because PHP return types are covariant. A narrower return type does not break callers, but subclasses overriding the method may need to adjust their declaration.

That is also why the attributes have directional names such as ReturnTypeNarrowing, ReturnTypeWidening, ParameterTypeNarrowing, and ParameterTypeWidening. The direction tells us who may be affected.

Some changes affect both sides. If a method becomes internal, for example, code calling it and code extending it may both need to stop relying on it.

Whether you need to act is therefore no longer answered with a generic “this method is deprecated.” It is answered by the concrete change and the way your extension uses the API.

The design started as a community-driven RFC and was refined through discussion and implementation feedback. It was introduced through the attribute family, PHPStan enforcement, and the migration of existing planning annotations; ParameterDefaultValueChange followed as a later refinement.

How extension developers can use the new information

The attributes themselves are marked @internal. They are not intended as a new extension API that plugins should instantiate or add to their own code.

What extension developers can rely on is the meaning of the announcement on a Shopware Core API: what changes, when it changes, and whether callers or extenders may be affected.

Let us return to Context::scope(). Its planned change is now expressed as structured metadata:

#[NewOptionalParameter(
    version: 'v6.8.0',
    parameterName: 'states',
    parameterType: 'array',
    defaultValue: [],
)]
public function scope(string $scope, \Closure $callback): mixed

For callers, there is nothing to do. The new parameter is optional, so existing calls remain valid.

For a class overriding the method, the situation is different. The override can add the optional parameter today:

public function scope(
    string $scope,
    \Closure $callback,
    array $states = [],
): mixed {
    // ...
}

Additional optional parameters are compatible with the current parent declaration. Once Shopware 6.8 adds the parameter, the override remains compatible. The extension can therefore prepare without a Shopware version check or two separate implementations.

Return-type narrowing provides another example. Field::setFlags() currently returns self, but will return static in Shopware 6.8:

#[ReturnTypeNarrowing(version: 'v6.8.0', newType: 'static')]
public function setFlags(Flag ...$flags): self

Callers are not affected because every static value is also a valid self value. An overriding method can already declare static, though, because PHP permits covariant return types:

public function setFlags(Flag ...$flags): static
{
    // ...
}

Again, one declaration works with both the current version and the future one.

Parameter renames mainly affect named arguments. For example:

#[ParameterNameChange(
    version: 'v6.8.0',
    parameterName: 'filename',
    newName: 'fileName',
)]
public function __construct(string $content, string $filename)

A call using filename: will break after the rename. Switching to positional arguments works with both parameter names:

new DomainVerificationRequestStruct($content, $fileName);

The backward-compatibility guide contains the full attribute inventory and a per-change guide explaining which adaptations can already be made without dropping compatibility with the current Shopware version.

Static metadata does not replace runtime deprecations

Not every compatibility problem should be left entirely to static analysis.

If incompatible legacy usage can be detected when the method runs, Shopware still emits a conditional runtime deprecation. For example, when a parameter will be narrowed from ?string to string, Core can detect that a caller still passes null and trigger Feature::triggerDeprecationOrThrow() for that call.

This is more precise than deprecating the entire method. Callers already passing a string receive no warning, while callers depending on the behaviour that will break in 6.8 get an actionable message.

Other changes cannot be detected reliably at runtime. A method cannot generally know that an extension overrides it with a future-incompatible declaration. A class becoming final cannot warn only the subclasses that matter without also creating noise for test doubles and other legitimate current usage.

For those changes, structured static information is the better tool.

Shopware’s PHPStan rules validate these announcements inside Core. They verify that parameter names and types resolve, that the announced state differs from the current state, and that runtime deprecations exist where incompatible use can be detected. Attributes also fail validation after their target version has arrived, so they cannot silently become stale documentation.

A separate rule prevents the old @deprecated reason:* planning markers from being reintroduced.

What becomes possible next

The first benefit for extension projects requires no additional tooling: after updating to Shopware 6.7.14.0, Core’s internal planning notes no longer appear as generic deprecation errors.

The structured metadata also allows us to go further.

A forward-looking proof of concept in phpstan-shopware explores an opt-in future-compatibility ruleset. Instead of analysing only the declarations available today, it can interpret the announced contracts and check whether extension code is already compatible with the next major version.

Such a ruleset can, for example, find:

Calls that pass values outside a future narrowed parameter type.
Named arguments using a parameter’s old name.
Overrides missing a future optional parameter.
Classes extending something that will become final or internal.
Code that does not handle a future widened return type.

This remains forward-looking work and is deliberately separate from the default PHPStan rules. Extension projects should be able to opt into stricter future-compatibility analysis when it fits their support strategy.

The same metadata could be used as input for custom Rector rules. Instead of parsing free-form PHPDoc text, a rule can read a specific change type and its payload, then apply a focused migration. IDE inspections, LSP diagnostics, compatibility reports, and upgrade-note generators are other possible consumers.

The important part is that we now have reliable input on which such tools can be built.

Stop ignoring deprecations

When @deprecated cries wolf often enough, developers stop listening. That was the real cost of using the same signal for both removals and internal BC planning: every false alarm made the next genuine deprecation easier to ignore.

With the two concerns now separated, that excuse is gone. Extension developers should take three concrete actions.

Remove broad deprecation suppressions

An @deprecated annotation on a public Shopware Core API is actionable. The API will be removed or replaced, and extensions using it need to follow the documented migration path.

Broad ignore patterns that hide Shopware deprecations should therefore be removed. They now risk hiding work that must be completed before the next major release.

Review the remaining deprecation reports in your extension, identify the documented replacement, and treat the migration as required maintenance rather than optional cleanup.

Check BC-change attributes on the APIs you use

Removing false warnings is only one half of the improvement. Extension developers also need to pick up the new BC-change signals.

When an API used by your extension carries a BC-change attribute, check whether it applies to call sites, extenders, or both. Then compare the announced change with how your extension uses that API.

The attributes do not make an extension forward-compatible by themselves. They provide the information needed to do so. If they are ignored until the major upgrade, the incompatibilities will still arrive all at once.

Adopt forward-compatible declarations early

Many announced changes can be handled before the next major release without dropping support for the current Shopware version.

The examples above show the concrete benefit:

An override can add an announced optional parameter today.
An override can adopt an announced narrower return type today.
A call site can stop relying on a parameter name that will change.

These are not merely preparations written down for later. They are changes extension developers can make now. One implementation can remain backward-compatible with the current Shopware version while already being compatible with the announced future contract.

Make these checks part of regular extension maintenance and static analysis instead of postponing them until the major upgrade. That turns one disruptive migration into a series of smaller, safer changes.

Shopware 6.8 is the first major upgrade to benefit from this model, but it will not be the last. Future major changes can use the same structured announcements, while PHPStan, Rector, and other tools can become increasingly effective at turning them into concrete migration work.

The goal is to stop @deprecated from crying wolf. When it appears now, treat it as a warning that demands action. When a BC-change attribute appears, use the head start it provides.

How to update php-redis extension (and why you might need to do so during shopware updates)

Jonas Elfering — Fri, 27 Feb 2026 09:00:00 +0000

This post gives you an explanation on why you might need to update the php-redis when upgrading to the latest shopware versions and how you can do so hassle-free

Why you might need to update the `php-redis` extension during shopware updates?

The latest shopware 6.7.7.0 version included the upgrade to Symfony LTS version 7.4. Because the previously used Symfony 7.3 version stopped being actively supported in Jan 2026, shopwares 6.6 LTS version will receive the Symfony update as well as part of the next release.
The update from Symfony 7.3 to 7.4 is only a minor upgrade, so it should not include any breaking changes. While that is true on the code level of Symfony itself, Symfony did increase the needed minimum version of the php-redis extension to 6.1 as part of the minor update. So if you currently use an older version of the extension, you will need to update the extension in order to successfully update shopware. This might especially be the case if you rely on older versions of Linux distributions; the 24.04 LTS version of Ubuntu, for example, ships php-redis extension in version 5.3, which is not compatible, and you need to take action to update the package accordingly.

How can I figure out which `php-redis` version I'm currently running?

You can check the installed version of php-redis extension by running the following command:

php --re redis | head -1

When that command indicates that you are running on version 6.1 or newer, you are ready to update to the latest shopware version without further action. If you're currently running an older version, you need to update it first.

Additionally, Symfony added a composer conflict with older php-redis versions, which means that the shopware upgrade (with the included Symfony upgrade) will throw a composer error if you still use an older version.

Should I update my `php-redis` extension now?

Generally, you should only look at upgrading php-redis when you plan to upgrade to the latest shopware versions, which include the upgrade to Symfony 7.4. Upgrading the php-redis version in existing, running applications provides no real benefit. And quite to the contrary, older shopware versions (especially when you are still on shopware 6.5) might not work with the latest php-redis version. Therefore, you shouldn't upgrade just in case, only if you are affected and want to upgrade to the latest shopware versions that require this. This guide shouldn't be considered a general best practice, but rather a troubleshooting guide for when you're upgrading and run into the need to update the php-redis extension.

How to update the `php-redis` extension in Docker

Docker makes it easy to control the full environment; therefore, it is straightforward to update the php-redis extension in your Dockerfile.

First you should download the php extension installer for Docker and make it executable:

ADD https://github.com/mlocati/docker-php-extension-installer/releases/latest/download/install-php-extensions /usr/local/bin/
RUN chmod +x /usr/local/bin/install-php-extensions

That script helps you to manage php extensions.

Next you can install the php-redis extension:

RUN install-php-extensions redis

By default, that will install the latest stable version of the extension. It might be the case that you pinned the installed version, and you need to upgrade that:

-RUN install-php-extensions redis-6.0
+RUN install-php-extensions redis-6.3

When you use the official shopware/docker image, that update is already done automatically, and you just need to make sure that you update to the latest version of the image.

How to update the `php-redis` extension in Ubuntu

You can install the php-redis extension in Ubuntu through the default package manager:

sudo apt install -y php-redis

However, depending on the Ubuntu version you are using, the extension available in the official package repository might be too old.

In that case, you can install the package from the ondrej/php package repository, which is the trusted source for pre-built php packages:

sudo add-apt-repository ppa:ondrej/php # Press enter when prompted.
sudo apt update

After that when you install the php-redis extension it should pick up the latest version:

sudo apt install -y php-redis

How to install the extension locally (with PIE)

There is a new extension installer in the PHP ecosystem called PIE. PIE makes installing extensions easier and installs the latest versions from Packagist. PIE is intended to replace the old PECL installer and works similarly to Composer.

To install PIE follow the installation instructions from the official repository:

curl -fL --output /tmp/pie.phar https://github.com/php/pie/releases/latest/download/pie.phar \
  && gh attestation verify --owner php /tmp/pie.phar \
  && sudo mv /tmp/pie.phar /usr/local/bin/pie \
  && sudo chmod +x /usr/local/bin/pie

When you have PIE installed, you can install the php-redis extension by running:

pie install phpredis/phpredis

How to install the extension locally (with PECL)

PECL is the old-school extension installer in the PHP ecosystem; setting it up is not as straightforward as PIE, so it is not recommended anymore. However, if you have already set up PECL, you can continue using that.

If you have PECL set up, you can install the latest extension by running:

pecl install redis

Mocks and their Influence on Software Design

Jonas Elfering — Fri, 08 Sep 2023 09:05:00 +0000

When speaking about unit testing, one automatically also speaks about mocks and the need to mock away dependencies.

It seems to be quite a typical attitude towards mocks along the lines of "Mock every external dependency of the class under test" and this attitude can be quite dangerous.

Therefore, here are some words of caution on the implication that the heavy use of mocks in your code base can have in terms of the overall system design or architecture.

TL;DR

Mocks are used to provide isolation in unit tests, and thus should help to create software that is easy to test and easy to change. But in fact, an over-usage of mocks in any unit test suite can cover up underlying issues in the software design that make the software hard to test and hard to change. So Mocks should be treated as one tool in your toolbox that one can use to build high-quality and maintainable software. That means one should know when to apply this specific tool, or when should rather use another tool from the toolbox.

Mocks are hard to refactor

Be cautious when utilizing mocks extensively because it can be hard to automatically refactor classes. This is because IDEs do not provide robust support for refactoring classes that are heavily mocked, and tools like PHPStan may not effectively detect these mock-related issues.

More broadly speaking, it is hard to guarantee that the mock behaves in the same/or intended manner as the real implementation (especially when the underlying implementation changes).

Use mocks only where you need to because:

creating the objects is hard as you need tons of nested dependencies to create the object.

the class produces some side effects that you don't want in unit tests (e.g., DB writes).

For all other cases, use real implementations and rely as minimally as possible on the magic of PHPUnit's mocking framework.

Focus on behavior, not implementation: Effective unit testing principles

Relying heavily on mocks creates a bad pattern in unit tests of testing how something is implemented and not what the implementation actually does. If tests are implemented in a mock-heavy way, they are tightly coupled to the implementation, meaning they rely on implementation details and may fail more often when the implementation details change than when the actual behavior of the class under test changes. Consider these two example changes to some classes:

Before:

$id = $this->repository->search($criteria, $context)->first()?->getId();

After:

$id = $this->repository->searchIds($criteria, $context)->firstId();

Before:

$values = $this->connection->fetchAllAssociative('SELECT first, second FROM foo ...');
$values = $this->mapToKeyValue($values);

After:

$values = $this->connection->fetchKeyValue('SELECT first, second FROM foo ...');

By definition, both changes are a pure example of refactoring:

Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior.
-> Martin Fowler

But when the unit test mocked the repository or connection dependencies, the unit tests will fail after the change, even though the external behaviour (that's what a test should really test) was not changed.

Using mocks is ok in some cases, but not all.

Probably the examples from above are ones that are totally valid (as the mocked classes rely on a DB), which can be commonly encountered in real life.

Furthermore, the intention of this document is to keep you aware of the downsides that come with using mocks.

Mocks might indicate your class is not well-designed

In a well-designed and testable system, it is relatively easy to isolate individual classes or modules and distinguish them between the components that contain the core business logic, which should be extensively unit tested, and the portions responsible for interfacing with the external environment and generating side effects. These side-effect-prone elements should be substituted in the unit tests. In fact, it is advisable to perform integration testing since their primary purpose is to abstract and facilitate the replacement of side effects in tests.

This kind of abstraction follows when you apply the principles of Domain Driven Design and Hexagonal Architecture (aka Ports & Adapters). I've written in more detail about those architectural considerations in my previous post.

The absence of such abstraction in an existing codebase is one of the reasons why it can be quite hard to write "good" unit tests.

So, a heavy reliance on mocks when writing unit tests can indicate a potential issue with the software design, suggesting insufficient encapsulation. Hence, designing code to promote better encapsulation and reduce the need for extensive mocking is advisable. This can lead to improved testability and overall software quality.

Better options than mocks

There are better options, but that depends on the use cases. Here are a few alternatives:

Use the real implementation (this means the real dependency is easy to create and does not produce side effects)
Use a hand-crafted dummy implementation of the real dependency, that is easy to configure and behaves like a stub in that use case (this means that the real dependency probably needs to be designed in a way to be easy to replace)
Fallback to using a mocking framework (when the real dependency is not designed to be replaced easily)

The way you design your codebase directly impacts whether you can rely on option 1 or option 2 without resorting to heavy mocking.

Conclusion: Write tests first!

When you write tests first, most of the points described above should come out of the box!

Nobody who starts with a test would start with configuring a mock.

While we provide insights on this, it is essential to validate the information. So we encourage you to explore the following references to gain a deeper understanding and form your own opinion.

References

Frank De Jonge on the exact same topic (with more examples in PHP): https://blog.frankdejonge.nl/testing-without-mocking-frameworks/

Martin Fowler on the differences between mocks (option 3) and stubs (option 2): https://martinfowler.com/articles/mocksArentStubs.html

Presentation by Mathias Noback on testing hexagonal architectures: https://matthiasnoback.nl/talk/a-testing-strategy-for-hexagonal-applications/

Some good real-life examples on unit tests in php: https://github.com/sarven/unit-testing-tips

A great write-up on testing in general: https://dannorth.net/2021/07/26/we-need-to-talk-about-testing/

Quite an old (1997!) paper on how not to use code coverage: http://www.exampler.com/testing-com/writings/coverage.pdf

Great blog post series on how to avoid mocks: https://philippe.bourgau.net/categories/#how-to-avoid-mocks-series

Benefits of separating core code from infrastructure code

Jonas Elfering — Thu, 31 Aug 2023 12:54:00 +0000

When it comes to software design and software architecture, there are many theoretical approaches with fancy acronyms (like SOLID, CUPID, DDD, hexagonal architecture, clean architecture, …). However, it is not always easy to translate these concepts into the practical world, and it is not clear where they overlap or not. Therefore, it's important to think about the practices all academic design patterns have in common and start from first principles. One really concrete practice that is encouraged by almost all software design flavors is the separation of core and infrastructure code. This post will explore many of the benefits and challenges through a real-life, practical example, so that by the end you will have a better understanding of how and why any software system might implement this tenant of sustainable architecture.

What is core and what is infrastructure code anyways?

Every software system consists of core and infrastructure code. Infrastructure code, in simple terms, is the code that connects the main business logic (core code) to the outside world (e.g. to the web server or database). For a definition of core code, we can refer to Matthias Noback's book “Advanced Web Application Architecture”:

Rule 1:
Core code doesn’t directly depend on external systems, nor does it depend on code written for interacting with a specific type of external system.
Rule 2:
Core code doesn’t need a specific environment to run in, nor does it have dependencies that are designed to run in a specific context only.

A common symptom of weak software design is if, at the code level, it is difficult to distinguish between the two. This lack of separation reflects a web of dependencies that create a host of issues, all of which we will gradually walk through with the example, below.

For the purpose of this blog post, Infrastructure code should not be confused with “Infrastructure as Code” (the code needed to spin up the server/cloud infrastructure that the application runs on).

An example

Consider the following Symfony controller used to post a new review by a customer for a given product:

#[Route(path: '/store-api/product/{productId}/review', methods: ['POST']]
public function save(
    string $productId, 
    RequestDataBag $data, 
    SalesChannelContext $context
): NoContentResponse {
    /** @var CustomerEntity $customer */
    $customer = $context->getCustomer();
    $customerId = $customer->getId();

    if (!$data->has('name')) {
        $data->set('name', $customer->getFirstName());
    }

    if (!$data->has('lastName')) {
        $data->set('lastName', $customer->getLastName());
    }

    $data->set('customerId', $customerId);
    $data->set('productId', $productId);

    $this->validate($data, $context->getContext());

    $review = [
        'productId' => $productId,
        'customerId' => $customerId,
        'externalUser' => $data->get('name'),
        'content' => $data->get('content'),
        'points' => $data->get('points'),
        'status' => false,
    ];

    if ($data->get('id')) {
        $review['id'] = $data->get('id');
    }

    $this->repository->upsert([$review], $context->getContext());

    $this->eventDispatcher->dispatch(
        new ReviewFormEvent($productId, $customerId, $data, $context)
    );

    return new NoContentResponse();
}

The code is rather straight forward to read: the controller accepts some data, fills in some fall-back data based on the customer currently logged-in, validates the input, saves the review to the database and dispatches a domain event. In a nutshell, this controller does too much at once. Through the course of this post, we will use this example to study the problems this can cause, and gradually refactor it to illustrate how separating the core code from infrastructure code inherently minimizes these issues or resolves them entirely.

Evolve infrastructure and core independently

Over time, your understanding of the underlying domain you are working in will improve, and you naturally will update your business logic to reflect this learning. In the best case, you want to have a single place where this knowledge is stored. On the other hand, if you want to grow your business, there may be other use cases where you want to provide the same business functionality (posting reviews) in different contexts. So there is a need for the infrastructure code and the core code to be able to evolve independently.

Reusability

In the controller example above, one can easily imagine that a new business requirement may arise that a merchant should be able to bulk import reviews from an existing review system. In the current implementation, this is not easily possible, because all the code is in a single place and it is not really reusable. In this scenario, the same business rules should apply, and the same domain events should be dispatched, because for the core domain logic, it is probably not that important from which source a new review was posted. Instead of copy & pasting a bunch of code to make the new use case work, the better option would be to separate the domain logic that is responsible for posting a review from the controller that provides that functionality over an HTTP-API.

So our code may now look like this:

#[Route(path: '/store-api/product/{productId}/review', methods: ['POST'])]
public function save(
    string $productId, 
    RequestDataBag $data, 
    SalesChannelContext $context
): NoContentResponse {
    /** @var CustomerEntity $customer */
    $customer = $context->getCustomer();
    $customerId = $customer->getId();

    if (!$data->has('name')) {
        $data->set('name', $customer->getFirstName());
    }

    if (!$data->has('lastName')) {
        $data->set('lastName', $customer->getLastName());
    }

    $data->set('customerId', $customerId);
    $data->set('productId', $productId);

    $this->postProductReviewService->post($data, $context->getContext());

    return new NoContentResponse();
}

class PostProductReviewService
{
    public function post(RequestDataBag $data, Context $context): void
    {
        $this->validate($data, $context);

        $review = [
            'productId' => $data->get('productId'),
            'customerId' => $data->get('customerId'),
            'externalUser' => $data->get('name'),
            'content' => $data->get('content'),
            'points' => $data->get('points'),
            'status' => false,
        ];

        if ($data->get('id')) {
            $review['id'] = $data->get('id');
        }

        $this->repository->upsert([$review], $context);

        $this->eventDispatcher->dispatch(
            new ReviewFormEvent($productId, $customerId, $data, $context)
        );
    }
}

With that structure in place, it is now rather easy to implement the “Bulk Import Reviews” functionality, by reusing the extracted service.

Note: The new core service still expects a RequestDataBag parameter, which is not optimal but will be tackled further down.

This refactoring step also greatly helps in cases when there comes a requirement for additional validation, that a customer actually has purchased the product in the past before the review is saved. After the refactoring, there is a single place where that logic can be added and we can evolve the core code independently from the infrastructure.

Switching the underlying technology

Another case where infrastructure code may change but the core code should not be affected is when there is the need to switch the underlying technology. There is much talk about being able to (theoretically) switch the whole data storage layer for the complete application without affecting the core code. I have to admit that I have never seen that work out in real life, and I think that these kinds of replacements should not be our end goal, as such an application probably would be over-engineered. But the possibility to switch out underlying technology for specific parts of the application without having to touch the rest of the system can be a sweet spot.

Back to our code example, perhaps some requirement arises where product reviews should not be saved in the relational database, but rather posted to an external review system that should be integrated. To make this change easier, the code that actually takes care of saving the data should be extracted from the core service (as that is actually infrastructure code) and an interface should be introduced, so that the core service only relies on the interface.

The code could be refactored like this:

class PostProductReviewService
{
    public function post(RequestDataBag $data, Context $context): void
    {
        $this->validate($data, $context);

        $this->productReviewGateway->postReviewByCustomer($data, $context);

        $this->eventDispatcher->dispatch(
            new ReviewFormEvent($data->get('productId'), $data->get('customerId'), $data, $context)
        );
    }
}

interface ProductReviewGateway
{
    public function postReviewByCustomer(RequestData $data, Context $context): void;
}

class DalProductReviewDataGateway implements ProductReviewGateway
{
    public function postReviewByCustomer(ProductReviewPostData $data, ProductReviewStatus $status, Context $context): void
    {
        $review = [
            'productId' => $data->get('productId'),
            'customerId' => $data->get('customerId'),
            'externalUser' => $data->get('name'),
            'content' => $data->get('content'),
            'points' => $data->get('points'),
            'status' => false,
        ];

        if ($data->get('id')) {
            $review['id'] = $data->get('id');
        }

        $this->repository->upsert([$review], $context);
    }
}

The fact that we use the Shopware DAL to store the reviews is now an implementation detail. We can easily change that storage layer without having to touch the core business logic.

Caveat: Finding the right abstractions

When creating abstractions to hide technical decisions from the consumer of that code, a major challenge is defining the right level of abstraction.

On the one hand, it is easy to create an “abstraction” that is too specific and tightly coupled to the implementation by leaking technical details as part of the interface. This leads to a situation where even if there is an abstraction in place, it is not possible to switch the actual implementation without breaking the abstraction.

On the other hand, it might also happen that abstractions are too generic. If that is the case, then clients of the abstraction have to implement solutions for common cases that are actually better solved by the implementation of that abstraction.

So finding the right level of abstraction is always a trade-off between either too specific or too general.

Testability

Being able to easily write unit tests for your mission critical business logic is a prerequisite in order to deliver high-quality software. If the infrastructure and the core code are not separated, it is hard to write these unit tests, as basically the whole system has to be in place in order to perform them. Looking back at the original single controller example from the beginning, in order to write a test for that piece of code, a database needs to be set up, the Shopware DAL needs to be functional, which means that the Symfony container must be set up properly, and you have to fake a request in order for the tests to run at all.

We can already see that the refactoring we made in order to evolve the infrastructure separately from the core code already improves testability a lot, as we can now test the infrastructure code and the core code separately. The core code is especially easier to test, as we don’t need to have a running database and the Symfony container set up anymore; instead, we can use a Mock or dummy implementation of our ProductReviewGateway interface. The only thing that makes unit testing of our core code (the PostProductReviewService) harder than it needs to be is the dependency on an HTTP-Request in the form of the RequestDataBag argument. In order to make it easier to test, we can define a DTO that encapsulates the actual parameters that our service needs.

class ProductReviewPostData
{
    private function __construct(
        private string $productId,
        private string $customerId,
        private string $name,
        private string $content,
        private int $points,
    ) {
    }

    public static function fromRequestData(
        string $productId,
        string $customerId,
        RequestDataBag $data,
    ): self {
        return new self(
            $productId,
            $customerId,
            $data->get('name'),
            $data->get('content'),
            $data->get('points'),
        );
    }
}

This DTO object can in itself be easily unit tested, and the adjusted service is now also easier to test:

class PostProductReviewService
{
    public function post(ProductReviewPostData $data, Context $context): void
    {
        $this->validate($data, $context);

        $this->productReviewGateway->postReviewByCustomer($data, $context);

        $this->eventDispatcher->dispatch(
            new ReviewFormEvent($data->getProductId(), $data->getCustomerId(), $data, $context)
        );
    }
}

Not only is the service now easier to test, but it is also easier to consume the service API as the expected input parameters are now well defined in the DTO, and there is no need anymore to guess the parameters and their names based on the implementation. Additionally, the type system gives us some simple validation for free, like validating required and optional parameters as well as their basic types.

The controller now looks like this:

#[Route(path: '/store-api/product/{productId}/review', methods: ['POST'])]
public function save(
    string $productId, 
    RequestDataBag $data, 
    SalesChannelContext $context
): NoContentResponse {
    /** @var CustomerEntity $customer */
    $customer = $context->getCustomer();
    $customerId = $customer->getId();

    if (!$data->has('name')) {
        $data->set('name', $customer->getFirstName());
    }

    if (!$data->has('lastName')) {
        $data->set('lastName', $customer->getLastName());
    }

    $postData = ProductReviewPostData::fromRequestData($productId, $customerId, $data);

    $this->postProductReviewService->post($postData, $context->getContext());

    return new NoContentResponse();
}

The influence on complexity

In order to keep a system in a state where it can be easily changed and evolved based on current needs, it is essential to keep the complexity of the overall system low. In general, the more things the system does, the more complex it becomes, but separating core from infrastructure code will help to keep complexity minimal. To see how many different things our original code does, we can separate it into the following “concerns”:

How are the HTTP-Request parameters transformed so our application can handle them? (Infrastructure concern)
What business rules apply when a product review is posted? (Core concern)
How is a product review stored in a persistent storage? (Infrastructure concern)

Our initial code handled all three concerns at the same time. That means when you want to understand how one of those concerns is solved, the code related to the other concerns is in fact adding complexity, which makes the part of the code you are actually interested in harder to understand. By separating out the concerns and handling them separately, it is easier to focus on one of the concerns without having to care how the others are solved; the level of complexity is reduced to a minimum.

We can even improve our code a little bit further by moving the fallback handling (which gets the name from the currently logged-in customer) to where the rest of the DTO is created to have that in a single place as well. So the code might look like this:

#[Route(path: '/store-api/product/{productId}/review', methods: ['POST'])]
public function save(
    string $productId,
    RequestDataBag $data, 
    SalesChannelContext $context
): NoContentResponse {
    /** @var CustomerEntity $customer */
    $customer = $context->getCustomer();

    $postData = ProductReviewPostData::fromRequestData($productId, $customer, $data);

    $this->postProductReviewService->post($postData, $context->getContext());

    return new NoContentResponse();
}

class ProductReviewPostData
{
    private function __construct(
        private string $productId,
        private string $customerId,
        private string $name,
        private string $content,
        private int $points,
    ) {
    }

    public static function fromRequestData(
        string $productId,
        CustomerEntity $customer,
        RequestDataBag $data,
    ): self {
        $name = $data->get('name') ?? $customer->getFirstName();
        $lastName = $data->get('lastName') ?? $customer->getLastName();

        return new self(
            $productId,
            $customer->getId(),
            $name,
            $data->get('content'),
            $data->get('points'),
        );
    }
}

With that refactoring, it’s now obvious we added a lastName field to the request based on the logged-in user, in case it was not provided by the client. But we actually never used lastName anywhere when storing the review; the product review does not distinguish between first and last names, and has only one name field. So in the original code, there lay a bug in plain sight, but it was hidden by all the accidental complexity surrounding it. By separating the different concerns, the bug becomes obvious and easy to fix.

class ProductReviewPostData
{
    private function __construct(
        private string $productId,
        private string $customerId,
        private string $name,
        private string $content,
        private int $points,
    ) {
    }

    public static function fromRequestData(
        string $productId,
        CustomerEntity $customer,
        RequestDataBag $data,
    ): self {
        $name = $data->get('name') ?? ($customer->getFirstName() . ' ' . $customer->getLastName());

        return new self(
            $productId,
            $customer->getId(),
            $name,
            $data->get('content'),
            $data->get('points'),
        );
    }
}

Improvements in collaboration

Another big benefit of separating core code from infrastructure code is that it can help you improve the collaboration with the non-engineers in your team, be it QA, designers or the people from the business side. When you separate the infrastructure-related code from the core code, it is way easier to see how the business rules for your application are implemented and if they match what the domain experts would expect. In the example case, some of the most interesting parts of the code are quite hidden in the $this->validate($data, $context) call. That method currently looks like this:

public function validate(ProductReviewPostData $data, Context $context): void
{
    $definition = new DataValidationDefinition('product.create_rating');

    $definition->add('name', new NotBlank());
    $definition->add('content', new NotBlank(), new Length(['min' => 40]));
    $definition->add('points', new GreaterThanOrEqual(1), new LessThanOrEqual(5));

    $criteria = new Criteria();
    $criteria->addFilter(new EqualsFilter('customerId', $this->customerId));
    $criteria->addFilter(new EqualsFilter('productId', $this->productId));

    // ensure that the customer can only post one review per product
    $definition->add('id', new EntityNotExists([
        'entity' => 'product_review',
        'context' => $context,
        'criteria' => $criteria,
    ]));

    $this->validator->validate($data, $definition);
}

The code uses the Symfony validator component to perform the validation. The first part of the validation is rather straightforward: it performs some basic consistency checks on the data, e.g. that the name is filled, that the content of the review is at least 40 characters long and that the rating points are between one and five. With the new code structure, it would probably be best to move this part of the validation to the DTO object itself, so that it is not even possible to create a DTO that is invalid. One can still use the Symfony validator or perform those checks manually in the constructors.

But the more interesting part is the last part of the code. There it uses some Shopware DAL magic to validate that the customer has not already reviewed the same product. This is a core business rule that should be enforced, but for now it is quite hidden in the validation code. To bring it into plain sight, we want to move this check into the body of the service, as that method should read like a workflow of what happens when a review is posted. To do so, we first create a new Interface for this kind of check, as only the Infrastructure layer can provide the information we need in the service, but those two layers should be kept separate. The interface might look like this:

interface ProductReviewConstraints
{
    public function hasCustomerAlreadyReviewedProduct(
        string $productId, 
        string $customerId, 
        Context $context
    ): bool;
}

And the implementation using the Shopware DAL might look like this:

class DALProductReviewConstraints
{
    public function hasCustomerAlreadyReviewedProduct(
        string $productId, 
        string $customerId, 
        Context $context
    ): bool {
        $criteria = new Criteria();
        $criteria->addFilter(new EqualsFilter('productId', $productId));
        $criteria->addFilter(new EqualsFilter('customerId', $customerId));

        return $this->productReviewRepository->searchIds($criteria, $context)->firstId() !== null;
    }
}

The business rule that says a customer can only post a review for a product once is now more obvious in the domain flow:

class PostProductReviewService
{
    public function post(ProductReviewPostData $data, Context $context): void
    {
        if ($this->reviewConstraints->hasCustomerAlreadyReviewedProduct(
            $data->getProductId(),
            $data->getCustomerId(),
            $context
        )) {
            throw new CustomerAlreadyReviewedProductException($data->getCustomerId(), $data->getProductId());
        }

        $this->productReviewGateway->postReviewByCustomer($data, $context);

        $this->eventDispatcher->dispatch(
            new ReviewFormEvent($data->getProductId(), $data->getCustomerId(), $data, $context)
        );
    }
}

With a structure like that, it is easy to discuss with other peers if we actually implemented the requirements the right way or if there is something missing. It is easy to spot that there is no check in place whether the customer actually bought that product or not, so it can spark discussions about whether and how to add such an additional check.

Conclusion: Keep infrastructure code at the boundaries of your codebase

The code as we left it at the end of this post is by no means perfect, but it is a perfect starting point for further refactoring based on the advanced architecture patterns mentioned in the beginning (DDD, hexagonal architecture, clean architecture, …). For example, proponents of DDD probably would refactor the service in a way to encapsulate the business logic inside a rich domain model and not let it stay directly in the service itself. But especially in DDD, you also don’t want any outside dependencies in your domain model, so it’s a necessary step to first separate your core code from the infrastructure code.

By now, this example walkthrough should already explain a lot of the benefits of separating the two. It makes it easier to interact with and evolve the codebase for you and your peers even when the requirements or the technology might considerably change. Ultimately, this is a general pattern that should be applied regardless of the architecture you have in place in your application, as it lays a concrete foundation for quality, sustainability and, dare I suggest, pleasure to work within your codebase.

DEV Community: Jonas Elfering

When `@deprecated` cries wolf: Making Shopware’s next major upgrades easier

TL;DR

We were asking @deprecated to do two different jobs

A more precise model for backward-compatibility changes

How extension developers can use the new information

Static metadata does not replace runtime deprecations

What becomes possible next

Stop ignoring deprecations

Remove broad deprecation suppressions

Check BC-change attributes on the APIs you use

Adopt forward-compatible declarations early

How to update php-redis extension (and why you might need to do so during shopware updates)

Why you might need to update the php-redis extension during shopware updates?

How can I figure out which php-redis version I'm currently running?

Should I update my php-redis extension now?

How to update the php-redis extension in Docker

How to update the php-redis extension in Ubuntu

How to install the extension locally (with PIE)

How to install the extension locally (with PECL)

Mocks and their Influence on Software Design

TL;DR

Mocks are hard to refactor

Focus on behavior, not implementation: Effective unit testing principles

Mocks might indicate your class is not well-designed

Better options than mocks

Conclusion: Write tests first!

References

Benefits of separating core code from infrastructure code

What is core and what is infrastructure code anyways?

An example

Evolve infrastructure and core independently

Reusability

Switching the underlying technology

Caveat: Finding the right abstractions

Testability

The influence on complexity

Improvements in collaboration

Conclusion: Keep infrastructure code at the boundaries of your codebase

We were asking `@deprecated` to do two different jobs

Why you might need to update the `php-redis` extension during shopware updates?

How can I figure out which `php-redis` version I'm currently running?

Should I update my `php-redis` extension now?

How to update the `php-redis` extension in Docker

How to update the `php-redis` extension in Ubuntu