chore(lambda-event-sources): refactoring the filters type

[marciocadev]

Description

Changing filters type from Array<{[key: string]: any}> to Array<FilterCriteria>.

This way it will be easier for the developer to understand what type of parameter should be passed

---

All Submissions:

• [x] Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

• [ ] This PR adds new unconventional dependencies following the process described here

New Features

• [ ] Have you added the new feature to an integration test?
  • [ ] Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[gitpod-io[bot]]

[mergify[bot]]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[marciocadev]

Hi @TheRealAmazonKendra

I had to update the branch last time because somehow PR Linter broke, you can check if it's good

[marciocadev]

@kaizencc

Your comments are perfect I will make the changes to make sure this new implementation doesn't breaks what was done previously

[kaizencc]

@marciocadev, refactoring is tough when we have a stable api! I think what we had before with deprecating filter and not using it internally (so we don't have to use testDeprecated) is the best way to go here.

[marciocadev]

@kaizencc I don't undestand, if I deprecate filters a lot of test have to be changed to testDeprecated

The initial problem is that the filters parameter is of type {[key:string]: any} but needs to receive a FilterCriteria parameter

I don't know if I need to deprecate filters and create a new variable or change filters type

[kaizencc]

Hi @marciocadev:

The initial problem is that the filters parameter is of type {[key:string]: any} but needs to receive a FilterCriteria parameter

That isn't necessarily true. FilterCriteria is still a {[key: string]: any}, so the developer doesn't really get anything out of the type being Array<FilterCriteria>. AFAIK, the current user experience is:

new EventSourceMapping(stack, 'test', {
        target: fn,
        eventSourceArn: eventSourceArn,
        kafkaTopic: topicNameParam.valueAsString,
        filters: [
          FilterCriteria.filter({
            numericEquals: FilterRule.isEqual(1),
          }),
        ],
      });

And it's weird because the user doesn't know to supply a FilterCriteria.filter. However, that isn't necessary. The user could simply do:

new EventSourceMapping(stack, 'test', {
        target: fn,
        eventSourceArn: eventSourceArn,
        kafkaTopic: topicNameParam.valueAsString,
        filters: [
          pattern: {
            numericEquals: FilterRule.isEqual(1),
          }),
        ],
      });

Which is perfectly fine and should work. The alternative you've suggested doesn't really help. In fact, I think it will confuse users because the only difference is whether you supply pattern or not:

new EventSourceMapping(stack, 'test', {
        target: fn,
        eventSourceArn: eventSourceArn,
        kafkaTopic: topicNameParam.valueAsString,
        filters: [
          {
            numericEquals: FilterRule.isEqual(1),
          },
        ],
      });

Am I missing something here? What was the original motivation behind having a FilterCriteria.filter? To my eye, it doesn't seem helpful.

[marciocadev]

Hi @kaizencc

My motivation was to make it clearer that the ideal was to use FilterCriteria to generate the pattern since the type that filters receives is {[key: string]: any}

I think it's complicated to use it as it is today

But if you think the way it is today is enough, I can close this PR

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 0bb519d3f4c60dc0bd4d30fdcedfd2b674c9494a
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[kaizencc]

As it stands, I don't think FilterCriteria.filter adds much. All it does is abstract out the pattern keyword. Ideally, we'd type filters better than {[key: string]: any} as has been discussed on the accompanying issue. If we can figure that out, then I think the path forward is to deprecate this filters property for a more strongly typed property. I'm going to close this PR in favor of that path. If you're interested in working on that, please go ahead! Let's talk design on a new issue and land on a good proposal.