abhinav-phi commented on PR #2122:
URL: https://github.com/apache/stormcrawler/pull/2122#issuecomment-5601576950

   That makes sense @rzo1 — thanks for chasing it through with @jnioche. 
Reworked as suggested (force-free branch update, commit eeb106db + merge of 
current main):
   
   **What changed:**
   - **Moved the canonicalisation into `AbstractStatusUpdaterBolt.execute`** — 
the single write path — exactly per your constraints: only when `status == 
DISCOVERED` (reusing `potentiallyNew`), applied **before** the dedup-cache 
lookup and before the `AS_IS_NEXTFETCHDATE` early return, so the raw-spelling 
pair hits the cache as one key too.
   - **Behind a config flag, default off:** `status.updater.canonicalise.host` 
(boolean). The param javadoc documents the release-notes caveat: enabling it 
makes URLs already stored under a raw spelling get re-discovered once under the 
canonical form (bounded, one record per old alias).
   - **Reverted** the `FetcherBolt` / `SimpleFetcherBolt` / 
`HttpRobotRulesParser` changes — with the store normalised, `u.getHost()` at 
those sites already yields the canonical spelling for anything that 
round-tripped the store.
   - **Kept the partitioner handling** (`URLPartitioner` / 
`URLPartitionerBolt`) — freshly emitted outlinks are partitioned on the emit 
path before any store round-trip, so that's where `byIP`/`byDomain` would 
otherwise still split alias spellings; matches your "possibly just the 
partitioner, if anything" note.
   - New `URLUtil.canonicaliseHostInUrl(String)`: rewrites only the host (keeps 
scheme/port/userinfo/path/query/fragment byte-for-byte), returns malformed / 
hostless / IPv6 URLs unchanged, and never throws.
   
   **Tests (20 relevant, all green locally, then full core suite before push):**
   - `URLUtilCanonicalHostTest` — rewriting, preservation of port/userinfo, 
idempotency, look-alike host text in the *path* untouched, 
malformed/hostless/IPv6 unchanged, null-safe.
   - `AbstractStatusUpdaterBoltTest` extended — DISCOVERED rewritten with the 
flag on, unchanged with it off, and **never** rewritten for `FETCHED` (hot path 
untouched).
   - Existing `URLPartitionerTest` / `URLPartitionerBoltTest` kept.
   
   Local verification before this push: `mvn -B -Prat -DskipTests verify 
-Dskip.format.code=false` (clean, no formatter diff) + full `core` module test 
run on JDK 25.
   
   PTAL — happy to squash further if you'd rather it land as one commit.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to