Why did PSR-6 make it OK to violate LSP?

[Gabriel O]

Hello all,

I would like to ask for help to understand why was line

Calling Libraries SHOULD NOT assume that an Item created by one Implementing Library is compatible with a Pool from another Implementing Library.

introduced in commit 9cfeacf3b9913793aa2a159ffe1d04080349b281

This example is now used as justification for rejecting non-specific classes that interface says implementation supports.

This is visible in Symfony PSR-6 implementation https://github.com/symfony/symfony/blob/b0a85ad015ced23f66ebd6215176e2cf5161e9ca/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php#L199

and Symfony HTTP client that is being designed now https://github.com/symfony/symfony/pull/30414#pullrequestreview-211896443

I'm afraid LSP breakages will become now more common.

[Larry Garfield]

Hi Gabriel.

The reason for that admonition is that for flexibility for the library implementer. Specifically, some implementations of a cache object may (and I believe do) pass a backend connection to the cache Item. For instance, a Redis connection or a PDO instance. That allows the Item object to lazy load a value rather than being just a dumb value object.

If it were a dumb value object then yes, it would most certainly make sense to keep it portable. However, it was designed such that logic could live in either the Pool or the Item depending on the implementation but still expose the same API to calling libraries; cache Items should only ever be retrieved from the Pool and then put back into it, most likely within the same function. They're not something you would be passing around to different functions.

As Nicolas notes in the thread you linked above, there's more to defined behavior and a contract than can be captured in a type hint. To use a trivial example, you can type hint something as an "iterable" but it has to be an iterable of something specific; passing an iterable of ints to a function that expects an iterable of PSR-7 Request objects is not going to end well, even if strictly speaking the type declaration would allow it.

LSP is an important guideline, but there is also more to interface and behavior specification than just type compatibility.

--Larry Garfield

[Gabriel O]

Thank you for your response!

Not sure I understand the lazy connection example though. If item with connection is passed to another pool, laziness should still work, since item is managing its state and loading itself, isn't it? On the other hand, when passing non-lazy item without connection to pool that expects lazy items, shouldn't it work as well, since there is nothing to load anymore?

And yes, it’s clear to me that contracts are not limited to just type hints, but such expectations needs to be documented in contract. Same would be in your iterable case. If it wasn’t documented what type of elements it should hold in contract, it’s expected it doesn’t matter.

-- 
You received this message because you are subscribed to a topic in the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/php-fig/G2ukoH88AjE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/3c18ffb6-625e-488b-8b0c-079cd16f12f5%40www.fastmail.com.
For more options, visit https://groups.google.com/d/optout.

[Larry Garfield]

On Sat, Mar 9, 2019, at 8:11 AM, Gabriel O wrote:
> Thank you for your response!
>
> Not sure I understand the lazy connection example though. If item with
> connection is passed to another pool, laziness should still work, since
> item is managing its state and loading itself, isn't it? On the other
> hand, when passing non-lazy item without connection to pool that
> expects lazy items, shouldn't it work as well, since there is nothing
> to load anymore?
>
> And yes, it’s clear to me that contracts are not limited to just type
> hints, but such expectations needs to be documented in contract. Same
> would be in your iterable case. If it wasn’t documented what type of
> elements it should hold in contract, it’s expected it doesn’t matter.

Imagine 2 Pool implementations; one is backed by Redis, and greedily loads data into the item object. The other is backed by MySQL and does so lazily by passing the PDO object to the Item.

Consider trying to update an item like this:

$item = $redisPool->getItem('foo');
$item->set('new value');
$sqlPool->save($item);

SqlPool internally does something like $item->saveYourself(), assuming that the Item has a PDO connection in it that it will use. A RedisItem, however, expects that RedisPool will call get() and its own $item->getExpiresAt() to extract information and write to Redis itself, using the RedisItem as just a dumb data carrier.

The Item interface is just the calling library-facing portion. The Item and Pool may have other methods they use to communicate that are not part of the spec, and thus not standardized. Passing an Item to the wrong pool can then end up calling a method that does not exist.

Hence the contract here is documented as "it only works with the pool it came from", for exactly that reason.

--Larry Garfield