Fix deletion of redirected URLs

[Atmuri-SatyaPrakash]

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]

You missed this: https://github.com/apache/stormcrawler/blame/e1d54a6572438a5518d772cb88ac51e8dada57fb/README.md#L43

[dpol1]

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]

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.

[dpol1]

Thanks — the concerns from my previous comment are addressed: temporary redirects are excluded and the ERROR/FETCHED behaviour is now pinned.

One remaining observation about the resulting contract: the PR is described as deleting permanent redirects, but a meta refresh carries no permanence signal. Could we narrow this feature to HTTP 301/308 only and put it behind status.updater.delete.redirections (default false)?

Deletion should also require redirect following to be enabled; otherwise redirections.allowed=false can suppress the target while deleting the source. A characterization test should also pin REDIRECTION without fetch.statusCode as no deletion.

Daily re-emission would still affect topologies opting in. I'd keep that as a separate scheduler/idempotency follow-up rather than consider it solved by the default.

[Atmuri-SatyaPrakash]

Thanks, I've addressed these points in the latest commit:

Redirect deletion is now opt-in via status.updater.delete.redirections (default false).
Deletion requires redirections.allowed=true.
Only HTTP 301/308 redirects are deleted.
Meta-refresh redirects and REDIRECTION without fetch.statusCode are not deleted.
Added regression tests covering these cases.

The targeted test suite passes all 9 tests.

[dpol1]

Thanks @Atmuri-SatyaPrakash! - LGTM now.