Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor FilterChain #208

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

gsteel
Copy link
Member

@gsteel gsteel commented Dec 10, 2024

Refactors filter chain to require the plugin manager as a constructor dependency and…

  • Drops setOptions
  • Removes inheritance
  • Implement newly added FilterChainInterface
  • Remove the constant DEFAULT_PRIORITY
  • Drops getter and setter for the plugin manager
  • Drops the plugin method
  • Drops getFilter
  • Stops pretending that chains can be serialized

Introduces ImmutableFilterChain which is effectively the same as FilterChain but immutable and lacks the merge method which has limited use cases.

ImmutableFilterChain and FilterChainInterface can be backported to 2.x so that libs can switch to it (input-filter, form etc) without breaking BC (Possibly)

Docs are still todo but feedback appreciated 👍

Refactors filter chain to require the plugin manager as a constructor dependency and…

- Drops `setOptions`
- Removes inheritance
- Implement newly added `FilterChainInterface`
- Remove the constant `DEFAULT_PRIORITY`
- Drops getter and setter for the plugin manager
- Drops the `plugin` method
- Drops `getFilter`
- Stops pretending that chains can be serialized

Introduces `ImmutableFilterChain` which is effectively the same as `FilterChain` but immutable and lacks the `merge` method which has limited use cases.

 `ImmutableFilterChain` and `FilterChainInterface` can be backported to 2.x so that libs can switch to it (input-filter, form etc) without breaking BC _(Possibly)_

Signed-off-by: George Steel <[email protected]>
Copy link
Member

@froschdesign froschdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like deleting code, therefore: great improvement! 👍🏻

@gsteel
Copy link
Member Author

gsteel commented Dec 17, 2024

@froschdesign
I've updated the docs for FilterChain and writing filters, could you please cast your eye over them? Thanks :)

@gsteel gsteel requested a review from froschdesign December 17, 2024 23:24
Copy link
Member

@froschdesign froschdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! 👍🏻

@gsteel gsteel self-assigned this Dec 17, 2024
@gsteel gsteel merged commit 8f75078 into laminas:3.0.x Dec 17, 2024
16 of 17 checks passed
@gsteel gsteel deleted the v3/immutable-filter-chain branch December 17, 2024 23:31
gsteel added a commit to gsteel/laminas-filter that referenced this pull request Dec 17, 2024
See laminas#208

Signed-off-by: George Steel <[email protected]>
This was referenced Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants