bugsnag-java icon indicating copy to clipboard operation
bugsnag-java copied to clipboard

Make FilteredMap key matching case-insensitive

Open eager opened this issue 4 years ago • 1 comments

Goal

FilteredMap is currently case-sensitive, making complete matching tedious. For example, to match the keys password, USER_PASSWORD, and adminPassword would require three separate filters, password, PASSWORD, and Password. Filtering becomes simpler, and less surprising, if matching is case-insensitive.

Changeset

Changed

FilteredMap#shouldFilterKey converts both the key and filter to lower case before matching.

Tests

FilteredMapTest has additional test cases.

Discussion

Alternative Approaches

None

Outstanding Questions

None

Linked issues

Fixes #152

Review

For the submitter, initial self-review:

  • [x] Commented on code changes inline explain the reasoning behind the approach
  • [x] Reviewed the test cases added for completeness and possible points for discussion
  • [ ] A changelog entry was added for the goal of this pull request
  • [x] Check the scope of the changeset - is everything in the diff required for the pull request?
  • This pull request is ready for:
    • [x] Initial review of the intended approach, not yet feature complete
    • [x] Structural review of the classes, functions, and properties modified
    • [ ] Final review

For the pull request reviewer(s), this changeset has been reviewed for:

  • [ ] Consistency across platforms for structures or concepts added or modified
  • [ ] Consistency between the changeset and the goal stated above
  • [ ] Internal consistency with the rest of the library - is there any overlap between existing interfaces and any which have been added?
  • [ ] Usage friction - is the proposed change in usage cumbersome or complicated?
  • [ ] Performance and complexity - are there any cases of unexpected O(n^3) when iterating, recursing, flat mapping, etc?
  • [ ] Concurrency concerns - if components are accessed asynchronously, what issues will arise
  • [ ] Thoroughness of added tests and any missing edge cases
  • [ ] Idiomatic use of the language

eager avatar Jan 27 '20 20:01 eager

Hi @eager - thanks for the PR. We would be interested in making this configuration option capable of taking regexes. If you were interested in making a PR for this, we'd be happy to take a look.

phillipsam avatar Feb 05 '20 10:02 phillipsam