Skip to content

Fix deletion of redirected URLs - #2050

Open
Atmuri-SatyaPrakash wants to merge 5 commits into
apache:mainfrom
Atmuri-SatyaPrakash:fix-delete-redirected-pages
Open

Fix deletion of redirected URLs#2050
Atmuri-SatyaPrakash wants to merge 5 commits into
apache:mainfrom
Atmuri-SatyaPrakash:fix-delete-redirected-pages

Conversation

@Atmuri-SatyaPrakash

@Atmuri-SatyaPrakash Atmuri-SatyaPrakash commented Aug 10, 2026

Copy link
Copy Markdown

Description

Fix deletion of URLs that are permanently redirected.

Previously, redirected URLs were not emitted to the deletion stream, which could leave the old URL in the index after a permanent redirect.

This change:

  • Deletes URLs for permanent HTTP redirects (301 and 308).
  • Does not delete URLs for temporary redirects such as 302.
  • Handles JSoup meta-refresh redirects, which are emitted as REDIRECTION with HTTP status 200.
  • Preserves the existing ERROR deletion behavior.

Testing

Added regression tests covering:

  • 301 permanent redirect → deletion
  • 308 permanent redirect → deletion
  • 302 temporary redirect → no deletion
  • Meta-refresh redirect → deletion
  • ERROR → exactly one deletion
  • FETCHED → no deletion

Tested with:

mvn -pl core -Deditorconfig.skip=true -Dtest=AbstractStatusUpdaterBoltTest test

Result:

Tests run: 6, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

@dpol1

dpol1 commented Aug 11, 2026

Copy link
Copy Markdown
Member

@dpol1

dpol1 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Real gap, thanks - good regression test too.

Couple of questions:

  1. What happens with a temporary redirect? All 3xx end up as REDIRECTION, so a site doing a 302 during maintenance would get its doc deleted - and with the default schedule that delete re-fires daily. Should we limit this to 301/308 (via fetch.statusCode - though JSoup meta-refresh emits REDIRECTION with status 200), or make it opt-in via config for existing topologies?
  2. Since the ERROR branch got restructured, could we also assert ERROR still emits exactly one delete and FETCHED none? And does assertEquals(metadata, ...) actually test anything? Metadata has no equals(), so it's comparing the object with itself - maybe assert on a key instead.

@Atmuri-SatyaPrakash

Copy link
Copy Markdown
Author

Thanks Davide! I’ve addressed both points.

  1. Permanent HTTP redirects (301/308) are now emitted to the deletion stream, while temporary redirects such as 302 are not. JSoup meta-refresh redirects are still handled separately through the 200 + _redirTo case.

  2. I also expanded the regression tests to verify 301, 308, 302, meta-refresh, ERROR, and FETCHED behavior. For the emitted metadata, I’m asserting the relevant metadata keys rather than relying on Metadata.equals().

The updated test suite passes with 6 tests, 0 failures, and 0 errors.

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