Remove encoding writing stuff

[ajgarlag]

Q	A
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	yes
License	MIT

What's in this PR?

This PR removes all stuff related with write filters in the Http\Message\Encoding namespace.

Why?

The abstract class Http\Message\Encoding\FilteredStream defines an abstract method named getWriteFilter that is not called anywhere and produces dead code not being tested. IMO this code must be removed or at least deprecated.

Example Usage

You can run the tests to see that this code is not being used nor tested.

$composer test

Checklist

• [x] Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix

  To Do

• [x] Squash the commits before merge

[sagikazarmark]

As far as I can see it's being called here: https://github.com/php-http/message/blob/master/src/Encoding/FilteredStream.php#L55

[ajgarlag]

Yes, but the writeFilterCallback is not being called anywhere

[sagikazarmark]

That's true, but technically the methods you removed are still called.

[sagikazarmark]

Ping @joelwurtz

[joelwurtz]

My intent was to propose a method to write into a filtered stream which is the reverse filter of the reading action.

This can be useful on server side or library manipulating stream (like transfering data from a FilteredStream to another FilteredStream should result in the same data, which is not the case actually as it would only decode multiple times but never reencode)

However i nerver implemented the method as ATM there was no need for that, removing this is not a problem as like you said it's never used (and it's not a BC break).

But there should be more to that, the filtered stream should only be readable and not writable (so isWritable should return false, and write method should throw an exception) and also it should not be seekable (but not the purpose of this PR, can be reported as a separate Issue or write as a separate PR)

[sagikazarmark]

Well, the fact that we don't use it is enough for removal?

Also, IMO it's a BC break, no matter if it's used by US or not, it's a public API change.

[dbu]

this is a massive BC break in our public API. if we decide we don't need this we should deprecate the methods and remove them in version 2 only. if they are useful as is, we should instead write some tests that make sure they work as intended.

[joelwurtz]

Ah yes didn't see the public visibility, i think it's a mistake in the original code as a user never need to get the encoding or decoding filter it's only an implementation detail, they should be deprecated then and removed in 2.0.

[dbu]

so we can keep this PR open but mark it to be version 2 only, and do another one to deprecate the methods? should we also trigger a deprecation warning like symfony does?

[ajgarlag]

@dbu I've already modified the PR to deprecate the methods.

[ajgarlag]

@sagikazarmark ready for review

[sagikazarmark]

LGTM