feat(events): add filter rules for prefixEqualsIgnoreCase, suffixEqualsIgnoreCase, wildcard, and anythingBut* matches #32063

jaw111 · 2024-11-08T08:35:10Z

Issue # (if applicable)

Closes #28462.

Reason for this change

Add support for:

Prefix matching while ignoring case
Suffix matching while ignoring case
Wildcard matching
Anything-but matching on suffixes
Anything-but matching using wildcards
Anything-but matching while ignoring case

Extend anything-but matching on prefixes to support a list of prefix values.

Description of changes

Added functions on Match class:

prefixEqualsIgnoreCase()
suffixEqualsIgnoreCase()
wildcard()
anythingButSuffix()
anythingButWildcard()
anythingButEqualsIgnoreCase()

Modified anythingButPrefix() to support rest parameters.

Description of how you validated changes

Added unit and integration tests for new and changed functions.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

jaw111 · 2024-11-08T13:09:13Z

The build fails, which seems to be due to failing tests for @aws-cdk/aws-scheduler-targets-alpha:test.

It's not clear from the contributing guidelines what/how to fix.

badmintoncryer · 2024-11-09T00:46:44Z

@jaw111 Thank you for your contribution!

It appears that aws-events integ test has failed, which may indicate that this modification introduces a breaking change.

@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3594926594/src/github.com/aws/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.rule.js
@aws-cdk-testing/framework-integ: Error: Some tests failed!

Could you please clarify the reason for failure?

badmintoncryer · 2024-11-09T00:49:12Z

Oh, it seems the integration test you updated this time has failed. Could you please update the snapshot for integ.rule.ts?

jaw111 · 2024-11-09T08:06:59Z

I think I am missing something, the contributing guide says to run integration tests on the .js file, whilst I modified the .ts file.

The changes I made to the .ts file are not reflected into the .js file, and the .js files appear to be ignored by Git.

How are those created/updated?

* add functions anythingButSuffix, anythingButWildcard, and anythingButEqualsIgnoreCase * change anythingButPrefix to support multiple values * implement unit tests * implement integration tests * update snapshots

jaw111 · 2024-11-09T10:00:29Z

packages/aws-cdk-lib/aws-events/lib/event-pattern.ts

+      throw new Error('anythingBut matchers must be non-empty lists');
+    }
+
+    return this.fromObjects([{ 'anything-but': { prefix: values } }]);


This will always output an array also when only a single value is supplied.

To avoid changes to existing rules created with this pattern, I can add an exception to handle such cases.

packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.rule.ts

badmintoncryer · 2024-11-09T13:51:31Z

It seems to be working well, but let me share the procedure just in case.

modify integ.rule.ts
execute yarn tsc at aws-cdk/packages/@aws-cdk-testing/framework-integ
execute yarn integ path/to/integ.rule.js --update-on-failed

badmintoncryer · 2024-11-09T13:57:15Z

packages/aws-cdk-lib/aws-events/lib/event-pattern.ts

@@ -109,8 +116,45 @@ export class Match implements IResolvable {
  /**
   * Matches any string that doesn't start with the given prefix.
   */
-  public static anythingButPrefix(prefix: string): string[] {
-    return this.fromObjects([{ 'anything-but': { prefix: prefix } }]);
+  public static anythingButPrefix(...values: string[]): string[] {


Changing an existing argument from a single string to string[] is a breaking change.
If you implement this, you would need to use feature flags, which would increase the complexity of the PR.
Please either avoid breaking changes as much as possible or modify the PR to use feature flags.

Example PRs:

fix(lambda-nodejs): remove smithy models from bundling for AWS SDK v3 runtimes (under feature flag) #31639

fix(ecs): reduce ecs service task role cloudwatch permissions when no log configured (under feature flag) #31475

Is it a breaking change? If the function is called with a single value like anythingButPrefix('foo') then it still works as before, so it is backwards compatible.

Otherwise I can make it something like this:

public static anythingButPrefix(prefix: string, ...values: string[]): string[] { values.unshift(prefix); // rest of code }

Another option would be to use function overloading:

public static anythingButPrefix(prefix: string): string[]; public static anythingButPrefix(...values: string[]): string[] { return this.anythingButConjunction('prefix', values); }

I'm really sorry for my misunderstandings.. This is not a breaking change and you don't need to update any more.

No problem, thanks for taking the time to review.

What are the next step? Should I merge/rebase the latest changes from the main branch?

…rdingly

codecov · 2024-11-12T14:03:38Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.67%. Comparing base (8189c82) to head (e745df0).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32063   +/-   ##
=======================================
  Coverage   78.67%   78.67%           
=======================================
  Files         107      107           
  Lines        7237     7237           
  Branches     1329     1329           
=======================================
  Hits         5694     5694           
  Misses       1357     1357           
  Partials      186      186

Flag	Coverage Δ
suite.unit	`78.67% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`78.67% <ø> (ø)`

badmintoncryer

Thank you very much for your assistance!

In this PR, I believe not only the wildcard but also matches like equals-ignore-case have been newly added. Could you please make the following adjustments accordingly?

Modify the PR title and description
Add usage examples for all newly added public static methods in the README

badmintoncryer

Thanks!

jaw111 · 2024-11-25T21:54:52Z

@badmintoncryer @mrgrain what are the next steps, is anything still needed from me?

badmintoncryer · 2024-11-28T10:40:57Z

@jaw111 Could you please merge the main branch? Since this PR has passed community review, the `needs-maintainer-review' label will be attached and the maintainer would review this.

gracelu0

A few minor comments, otherwise LGTM! Once addressed I'll approve.

packages/aws-cdk-lib/aws-events/README.md

packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.rule.ts

Co-authored-by: Grace Luo <54298030+gracelu0@users.noreply.github.com>

Pull request has been modified.

gracelu0

Thank you for contributing!

mergify · 2024-12-02T22:51:43Z

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).

mergify · 2024-12-02T22:57:18Z

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).

mergify · 2024-12-04T14:38:41Z

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).

mergify · 2024-12-04T19:54:11Z

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).

mergify · 2024-12-04T20:55:11Z

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).

ScottRobinson03 · 2024-12-05T15:45:07Z

@gracelu0 Looks like the codecov check is broken here too, which is preventing this PR from merging (and because of the queue it's also preventing other PRs from merging).

aws-cdk-automation · 2024-12-05T18:28:49Z

AWS CodeBuild CI Report

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

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

mergify · 2024-12-05T18:29:04Z

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).

github-actions · 2024-12-05T19:08:31Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

jaw111 added 6 commits November 8, 2024 08:33

Add wildcard match in event-pattern.ts

6bfa0ba

add test for wildcard match in matchers.test.ts

dd23fea

Update README.md

07d6d55

add integration test for wildcard in integ.rule.ts

ce497a5

simplify wildcard pattern in integration test

b1f8342

update integration test snapshot

0ac3c00

aws-cdk-automation requested a review from a team November 8, 2024 08:35

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Nov 8, 2024

jaw111 added 2 commits November 9, 2024 08:32

update integration test and snapshot

4b70c5e

extend anythingBut rules

0982d12

* add functions anythingButSuffix, anythingButWildcard, and anythingButEqualsIgnoreCase * change anythingButPrefix to support multiple values * implement unit tests * implement integration tests * update snapshots

jaw111 commented Nov 9, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 9, 2024

jaw111 commented Nov 9, 2024

View reviewed changes

packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.rule.ts Show resolved Hide resolved

badmintoncryer reviewed Nov 9, 2024

View reviewed changes

jaw111 and others added 6 commits November 9, 2024 14:02

refactor and abstract anything-but conjunctions and adjust tests acco…

c6709c2

…rdingly

refactor and add comment for clarity

e6729ef

update integration test snapshot

98fbb4f

add support for prefix and suffix matching while ignoring case

b362fb1

add integration tests and update snapshot

2a7f428

Merge branch 'main' into 28462-aws-events-match-wildcard

4f8520c

badmintoncryer suggested changes Nov 12, 2024

View reviewed changes

Merge branch 'main' into 28462-aws-events-match-wildcard

38cbf8a

badmintoncryer approved these changes Nov 14, 2024

View reviewed changes

Merge branch 'main' into 28462-aws-events-match-wildcard

da62a0e

Merge branch 'main' into 28462-aws-events-match-wildcard

98c5e38

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Nov 28, 2024

gracelu0 previously requested changes Nov 29, 2024

View reviewed changes

packages/aws-cdk-lib/aws-events/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk-testing/framework-integ/test/aws-events/test/integ.rule.ts Show resolved Hide resolved

Update packages/aws-cdk-lib/aws-events/README.md

07a1234

Co-authored-by: Grace Luo <54298030+gracelu0@users.noreply.github.com>

github-actions bot mentioned this pull request Dec 1, 2024

Monthly PR metrics report #32340

Open

gracelu0 approved these changes Dec 2, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 2, 2024

mergify bot added 2 commits December 3, 2024 20:10

Merge branch 'main' into 28462-aws-events-match-wildcard

f0a3940

Merge branch 'main' into 28462-aws-events-match-wildcard

f5e3f56

mergify bot added 2 commits December 4, 2024 19:20

Merge branch 'main' into 28462-aws-events-match-wildcard

d823c86

Merge branch 'main' into 28462-aws-events-match-wildcard

c1d531a

Merge branch 'main' into 28462-aws-events-match-wildcard

34ca646

Merge branch 'main' into 28462-aws-events-match-wildcard

e745df0

mergify bot merged commit 0ce71fc into aws:main Dec 5, 2024
17 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(events): add filter rules for prefixEqualsIgnoreCase, suffixEqualsIgnoreCase, wildcard, and anythingBut* matches #32063

feat(events): add filter rules for prefixEqualsIgnoreCase, suffixEqualsIgnoreCase, wildcard, and anythingBut* matches #32063

jaw111 commented Nov 8, 2024 •

edited

Loading

jaw111 commented Nov 8, 2024

badmintoncryer commented Nov 9, 2024

badmintoncryer commented Nov 9, 2024

jaw111 commented Nov 9, 2024

jaw111 Nov 9, 2024

badmintoncryer commented Nov 9, 2024

badmintoncryer Nov 9, 2024 •

edited

Loading

jaw111 Nov 9, 2024

jaw111 Nov 10, 2024

badmintoncryer Nov 11, 2024

jaw111 Nov 11, 2024

codecov bot commented Nov 12, 2024 •

edited

Loading

badmintoncryer left a comment •

edited

Loading

badmintoncryer left a comment

jaw111 commented Nov 25, 2024

badmintoncryer commented Nov 28, 2024

gracelu0 left a comment

gracelu0 left a comment

mergify bot commented Dec 2, 2024

mergify bot commented Dec 2, 2024

mergify bot commented Dec 4, 2024

mergify bot commented Dec 4, 2024

mergify bot commented Dec 4, 2024

ScottRobinson03 commented Dec 5, 2024

aws-cdk-automation commented Dec 5, 2024

mergify bot commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

feat(events): add filter rules for prefixEqualsIgnoreCase, suffixEqualsIgnoreCase, wildcard, and anythingBut* matches #32063

feat(events): add filter rules for prefixEqualsIgnoreCase, suffixEqualsIgnoreCase, wildcard, and anythingBut* matches #32063

Conversation

jaw111 commented Nov 8, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

jaw111 commented Nov 8, 2024

badmintoncryer commented Nov 9, 2024

badmintoncryer commented Nov 9, 2024

jaw111 commented Nov 9, 2024

jaw111 Nov 9, 2024

Choose a reason for hiding this comment

badmintoncryer commented Nov 9, 2024

badmintoncryer Nov 9, 2024 • edited Loading

Choose a reason for hiding this comment

jaw111 Nov 9, 2024

Choose a reason for hiding this comment

jaw111 Nov 10, 2024

Choose a reason for hiding this comment

badmintoncryer Nov 11, 2024

Choose a reason for hiding this comment

jaw111 Nov 11, 2024

Choose a reason for hiding this comment

codecov bot commented Nov 12, 2024 • edited Loading

Codecov Report

badmintoncryer left a comment • edited Loading

Choose a reason for hiding this comment

badmintoncryer left a comment

Choose a reason for hiding this comment

jaw111 commented Nov 25, 2024

badmintoncryer commented Nov 28, 2024

gracelu0 left a comment

Choose a reason for hiding this comment

gracelu0 left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 2, 2024

mergify bot commented Dec 2, 2024

mergify bot commented Dec 4, 2024

mergify bot commented Dec 4, 2024

mergify bot commented Dec 4, 2024

ScottRobinson03 commented Dec 5, 2024

aws-cdk-automation commented Dec 5, 2024

AWS CodeBuild CI Report

mergify bot commented Dec 5, 2024

github-actions bot commented Dec 5, 2024

jaw111 commented Nov 8, 2024 •

edited

Loading

badmintoncryer Nov 9, 2024 •

edited

Loading

codecov bot commented Nov 12, 2024 •

edited

Loading

badmintoncryer left a comment •

edited

Loading