feat!: DeleteAll match pattern and v5 adjustments

[moritzraho]

Description

MAJOR: This will go in v5

Add support for a match pattern for deleting a selected set of keys.

Breaking changes:

• deleteAll safeguard, must set the match option
• fix!: hide 404s on container operations #175

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have signed the Adobe Open Source CLA.
• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (709330b) to head (f4a2ccf).

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #174   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          251       265   +14     
  Branches        43        48    +5     
=========================================
+ Hits           251       265   +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[shazron]

just double checking - if the dev just calls deleteAll(() it will fail right? (based on the regex) and they have to add * to match if they want to delete everything.

[shazron]

nvm - I see that the test is there where deleteAll should delete everything. Giving me nightmares if somehow I had a typo in there or wrong/null object. Perhaps as a "confirmation" step, where if you really want to deleteAll it should really be deleteAll({ match: '*' }) WDYT?

[moritzraho]

nvm - I see that the test is there where deleteAll should delete everything. Giving me nightmares if somehow I had a typo in there or wrong/null object. Perhaps as a "confirmation" step, where if you really want to deleteAll it should really be deleteAll({ match: '*' }) WDYT?

yes felt this too, now the issue is that we already exposed deleteAll()... and because this is already used as part of the deleteService we would need to make it a breaking change. Which could be acceptable at that point, wdyt?

Edit: Another alternative without requiring a breaking change would be state.delete('supports*pattern') and keeping state.deleteAll()

[shazron]

nvm - I see that the test is there where deleteAll should delete everything. Giving me nightmares if somehow I had a typo in there or wrong/null object. Perhaps as a "confirmation" step, where if you really want to deleteAll it should really be deleteAll({ match: '*' }) WDYT?

yes felt this too, now the issue is that we already exposed deleteAll()... and because this is already used as part of the deleteService we would need to make it a breaking change. Which could be acceptable at that point, wdyt?

Edit: Another alternative without requiring a breaking change would be state.delete('supports*pattern') and keeping state.deleteAll()

Let's put it up for discussion. Seeing that this is a "dangerous" function to use (with no going back), perhaps we need a guard rail in there imo, and it warrants a breaking change.

[MichaelGoberling]

lgtm!