Skip to content

Conversation

@dcshzj
Copy link
Contributor

@dcshzj dcshzj commented Aug 6, 2025

Problem

What problem are you trying to solve? What issue does this close?

Split from #2403

Solution

How did you solve the problem?

Features:

  • Start writing RedirectDestination into the redis cache

Tests

What tests should be run to confirm functionality?

Deploy Notes

Notes regarding deployment of the contained body of work. These should note any
new dependencies, new scripts, etc.

Copy link
Collaborator

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

generally no issues, but can i be abit more onerous here, cos the blast radius of this chnage is huge.

would it be possible to add a test case for back-compat (so now, my values in redis are string and not json) then check that reads still work gracefully.

understand that we do a try/catch then resolve with defaults:

            resolve({
              longUrl: cacheLongUrl,
              isFile: false,
              safeBrowsingExpiry: null,
            })

but just paranoid haha

Copy link
Collaborator

@seaerchin seaerchin left a comment

Choose a reason for hiding this comment

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

discarding my previous review, noticed existing tests use a string already

@dcshzj dcshzj force-pushed the feat/actively-check-malicious branch from 647fee7 to 2203591 Compare August 20, 2025 10:25
@dcshzj dcshzj changed the base branch from feat/actively-check-malicious to develop August 20, 2025 12:21
@dcshzj dcshzj force-pushed the feat/write-redirectdestination-cache branch from e2b1075 to 9455244 Compare August 20, 2025 12:21
@dcshzj dcshzj merged commit c9da75b into develop Aug 20, 2025
16 checks passed
dcshzj added a commit that referenced this pull request Aug 21, 2025
* feat: start writing RedirectionDestination in redis cache (#2407)

* chore: remove sgid (#2394)

* chore: delete sgid files

* chore: remove sgid

* chore: update package lock

* 1.84.0

---------

Co-authored-by: seaerchin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants