Reorg of trigger filtering code

[slinkydeveloper]

Signed-off-by: Francesco Guardiani francescoguard@gmail.com

This PR reorganizes the filtering code to be a little bit more extensible, although it doesn't add, nor modify, any current behaviour of the code. It just makes it simpler to contribute to extend it 😄 I did it to use as a base to enable people experiment new filters (play nice with #3771 and #4279). I have a couple of ideas I'm gonna show in next PRs on filtering, and this serves as a good base. Took from #3771

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: slinkydeveloper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [slinkydeveloper]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[slinkydeveloper]

/hold

merge after the release

[codecov]

Codecov Report

Merging #4493 (ab8f31f) into master (eb54c75) will increase coverage by 0.00%.
The diff coverage is 89.13%.

@@           Coverage Diff           @@
##           master    #4493   +/-   ##
=======================================
  Coverage   81.27%   81.27%           
=======================================
  Files         282      284    +2     
  Lines        8004     8017   +13     
=======================================
+ Hits         6505     6516   +11     
- Misses       1112     1113    +1     
- Partials      387      388    +1     

Impacted Files	Coverage Δ
pkg/eventfilter/filter.go	83.33% <83.33%> (ø)
pkg/eventfilter/attributes/filter.go	86.36% <86.36%> (ø)
pkg/apis/sources/v1beta1/sinkbinding_lifecycle.go	92.06% <100.00%> (-0.25%)	⬇️
pkg/channel/fanout/fanout_message_handler.go	91.47% <100.00%> (-0.07%)	⬇️
pkg/mtbroker/filter/filter_handler.go	79.05% <100.00%> (-0.47%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb54c75...e179173. Read the comment docs.

[slinkydeveloper]

/unhold

[tommyreddad]

/assign

[zhongduo]

/assign

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-eventing-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/eventfilter/attributes/filter.go	Do not exist	86.7%
pkg/eventfilter/filter.go	Do not exist	92.3%
pkg/mtbroker/filter/filter_handler.go	87.2%	87.7%	0.5

[tommyreddad]

/lgtm

[zhongduo]

IMO comment is required for all exported functions. And in this case this function should have comment unless we don't have to export it, which is actually the preferred way.

[slinkydeveloper]

So there are 2 reasons why IMO it doesn't make sense to comment here:

• the receiver is unexported, so user cannot address it directly anyway
• we're implementing a contract and there is nothing, in this particular impl, which deviates particularly from the comment in the contract itself. So what the comment should say? The same comment in eventfilter.Filter? Isn't better to just explicitly show in the code the implementation with the type assertions (like i added) so the developer goes to the original contract and checks the updated docs?