Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add function support for timeWindow #357

Merged
merged 2 commits into from
Jan 28, 2024
Merged

Conversation

mindrunner
Copy link
Contributor

@mindrunner mindrunner commented Jan 12, 2024

This update allows timeWindow to be a function that can dynamically determine the time window based on the request and key. The code refactor has been applied across both the Redis and Local stores. Tests for this new functionality have been added to ensure correctness. The TypeScript definition was also updated to reflect this change.

#283

Checklist

@mindrunner mindrunner mentioned this pull request Jan 12, 2024
2 tasks
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the tests for the types?

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@mindrunner
Copy link
Contributor Author

Could you update the tests for the types?

done

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gurgunday
Copy link
Member

@mindrunner can you try targeting next?

@jsumners jsumners dismissed their stale review January 18, 2024 14:59

My concern has been addressed.

@mindrunner mindrunner changed the base branch from master to next January 19, 2024 08:50
@mindrunner
Copy link
Contributor Author

@mindrunner can you try targeting next?

done

index.js Outdated Show resolved Hide resolved
Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@gurgunday gurgunday merged commit 9fc0a86 into fastify:next Jan 28, 2024
13 checks passed
@gurgunday
Copy link
Member

Thanks again for the PR

jsumners added a commit that referenced this pull request Jul 10, 2024
* Updating for v5 (#338)

* ci: add node 21 to test matrix and remove node 14 and 16

* package.json update for v5

* merge main to next (#348)

* Update README.md to reflect #307 (#342)

* Redis should work with ban (#336)

* Redis should work with ban

* fix ttl

* simplify

* format

* remove warning

* move property

* add test

* fix test

* fix test

* fix test

* still fix the same test

* make it non-breaking

* make it non-breaking

* make it non-breaking

* make it non-breaking

* perf: add ban back

* Bumped v8.1.0

Signed-off-by: Matteo Collina <[email protected]>

* Bumped v9.0.0

Signed-off-by: Matteo Collina <[email protected]>

* Fix in-memory cache bug when there is only one entry (#345)

* Bumped v9.0.1

---------

Signed-off-by: Matteo Collina <[email protected]>
Co-authored-by: Trevor Lund <[email protected]>
Co-authored-by: Matteo Collina <[email protected]>
Co-authored-by: Igor Savin <[email protected]>

* Revert "merge main to next (#348)"

This reverts commit 5446122.

* revert workflow update as its causing issues (#350)

* revert: handle ban in store (#347)

* remove ban handling from store

* don't use negation

* add undef

* don't pass ban to promis

* fix accidental explicit checks

* fix accidental line change

* inline ban

* Workflow v4 (#354)

* refactor: make RedisStore default key a default parameter (#356)

* refactor redis parameters and throw if no instance was provided

* revert error

* Add function support for timeWindow (#357)

* feat: dynamic time window support

* fix: proper usage of timeWindowString

---------

Co-authored-by: lukas <[email protected]>

* update docs (#360)

* fix: remove unnecessary parameter timeWindow (#363)

* refactor: consistent option handling (#365)

* small refactor in option handling

* simplify

* simplify

* perf: pregenerate `timeWindow` string when possible and use `noop` as default function (#364)

* pregenerate timeWindowString if possible

* simplify

* simplify

* noop

* simplify

* simple-example

* rename

* update for v5 (#370)

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update for v5

* update fastify deps

---------

Signed-off-by: Matteo Collina <[email protected]>
Co-authored-by: Eliphaz Bouye <[email protected]>
Co-authored-by: Gürgün Dayıoğlu <[email protected]>
Co-authored-by: Trevor Lund <[email protected]>
Co-authored-by: Matteo Collina <[email protected]>
Co-authored-by: Igor Savin <[email protected]>
Co-authored-by: lukas <[email protected]>
Co-authored-by: lukas <[email protected]>
Co-authored-by: Gürgün Dayıoğlu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants