Contracts example

[herbertroth]

This pull request includes changes to add an example for contracts.

[herbertroth]

@alexz707 @martineiber My idea how to implement this for separate public api and intern usage.

[jdreesen]

I don't really get it. Should this also implement DbResolverContractInterface? Or what's the advantage of duplicating the interface?

[jdreesen]

But why do we need the DbResolverContract decorator then?

[herbertroth]

@jdreesen
Hi,
That’s a great question!

We recognize the problem and needs outlined in Issue #70 of this repository. Let me start by explaining the original intent behind the repository.

Our goal was to eliminate static calls, as they make it practically impossible to unit test classes that depend on them. To address this, we created a resolver bundle that uses a Resolver class with an interface. This approach allows us to create mocks or stubs from the interfaces.

These interfaces were never intended for use outside of this purpose, so we decided to add methods to the resolver and its interface only as needed. However, we didn’t anticipate that these interfaces could also be used in custom implementations for users’ own classes. This has introduced a challenge with semantic versioning whenever we add a new method to an interface, as described in Issue #70.

To solve this, we’ve discussed multiple approaches and have come up with a possible solution. The idea is to separate resolvers and interfaces into two categories: one for internal use and one stable version that adheres to semantic versioning.

Here’s how it would work:
Resolver and Interface

• The Resolver class will be marked as final and designated for internal use only.
• Similarly, the interface will also be marked as internal.
• These internal components can evolve continuously, with new methods added as needed without adhering to semantic versioning.

Contract Resolver and Contract Interface

• The “contract” resolver and interface will be part of the public API.
• They will not be marked as final and will follow semantic versioning rules.
• These components are designed for custom implementations by users.

For every major release, we will incorporate all additions made to the internal resolver and interface into the contract classes and interfaces, ensuring they remain up to date with new features while adhering to semantic versioning rules.

The difference between these two categories is subtle but impactful. Internal components provide flexibility for rapid iteration, while the public contract ensures stability for external use.

This is just an idea of how to handle this issue. We haven’t finalized a decision on how to proceed with this problem yet.

[jdreesen]

Thanks for the explanation. This is also what I thought this was supposed to be when I looked at it.
But the implementation doesn't make sense to me yet. What is the advantage if DbResolverContract is a decorator of the DbResolverInterface?
If you “only” want to add new methods, something like this should be enough, right?

interface DbResolverContractInterface
{
    public function getConnection(): Connection;

    public function reset(): Connection;

    public function get(): Connection;

    public function close(): void;
}

/**
 * @internal
 */
interface DbResolverInterface extends DbResolverContractInterface
{
    public function some();

    public function new();

    public function methods();
}

/**
 * @internal
 */
class DbResolver implements DbResolverInterface
{
    public function getConnection(): Connection
    {
        return Db::getConnection();
    }

    public function reset(): Connection
    {
        return Db::reset();
    }

    public function get(): Connection
    {
        return Db::get();
    }

    public function close(): void
    {
        Db::close();
    }

    public function some() 
    {
      // ...
    }

    public function new() 
    {
      // ...
    }

    public function methods() 
    {
      // ...
    }
}

You could then add new methods to the DbResolverInterface and implement them in DbResolver and in the next major move them from DbResolverInterface to DbResolverContractInterface. The DbResolverContract would not be needed.

But maybe I'm missing something 🤔

[mattamon]

@jdreesen this (your description) was also the first idea for me, when we discussed about it.

[herbertroth]

@jdreesen @mattamon The problem is that the DbResolver is internal. This means that every time you want to use the contract interface, you either have to implement the class yourself or extend/use an internal class. One potential solution could be to make the DbResolver not internal. However, in doing so, the class would still implement an internal interface, which is also a bit awkward.

interface DataObjectResolverContractInterface
{
    public function getById(int|string $id, array $params = []): ?DataObject;

    public function getByPath(string $path, array $params = []): ?DataObject;

    public function getList(array $config = []): Listing;
}

/**
 * @internal
 */
interface DataObjectResolverInterface extends DataObjectResolverContractInterface
{
    public function getTypes(): array;

    public function setHideUnpublished(bool $hideUnpublished): void;
}

/**
 * @internal
 */
class DataObjectResolver implements DataObjectResolverInterface
{
    public function getById(int|string $id, array $params = []): ?DataObject
    {
        return DataObject::getById($id, $params);
    }

    public function getByPath(string $path, array $params = []): ?DataObject
    {
        return DataObject::getByPath($path, $params);
    }

    public function getList(array $config = []): Listing
    {
        return DataObject::getList($config);
    }

    public function getTypes(): array
    {
        return DataObject::getTypes();
    }

    public function setHideUnpublished(bool $hideUnpublished): void
    {
        DataObject::setHideUnpublished($hideUnpublished);
    }
}


interface CarResolverInterface extends DataObjectResolverContractInterface
{
    public function getByColor(string $color): Car;
}

// All fine till you need a class to extend.
class CarResolver extends DataObjectResolver implements CarResolverInterface
{
    public function getByColor(string $color): Car
    {
        return DataObject\Car::getByColor($color);
    }
}

// Or implement it by your own. But again a decorator. 
// It need to be done in every custom Resolver. 
// For major updates you have to modify it by your own risk.
class CarResolverWithDi implements CarResolverInterface
{

    public function __construct(private readonly CarResolverInterface $dataObjectResolver)
    {
    }

    public function getById(int|string $id, array $params = []): ?DataObject
    {
        return $this->dataObjectResolver->getById($id, $params);
    }

    public function getByPath(string $path, array $params = []): ?DataObject
    {
        return $this->dataObjectResolver->getByPath($path, $params);
    }

    public function getList(array $config = []): Listing
    {
        return $this->dataObjectResolver->getList($config);
    }
    
    public function getByColor(string $color): Car
    {
        DataObject\Car::getByColor($color);
    }
}

[herbertroth]

@jdreesen @mattamon I like the idea of splitting this up and do not use a decorator. Much simpler and cleaner. So still a Contract and Internal Class/Interface but like in you example, no decorator. Thanks for food for thought.

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[herbertroth]

Got some rebase problems on this. Will create a new one.