Add utilities to detect and replace broken links. #1366

barsh404error · 2025-12-30T10:52:45Z

Thanks alot to @christolis for helping me out on making this pull request.
Added two utulity methods isLinkBroken and replaceDeadLinks

-isLinkBroken(String url) checks the link availability using a HEAD request
I used HEAD request instead of GET request to check link availability without downloading the response body, reducing bandwidth and improving the performance.

-replaceDeadLinks (String text, String replacement) replaces unreachable/broken links asynchronously.

This change does not have any behavior changes to the existing code.

Part of #1276, implements the mentioned utility but doesnt apply it.

CLAassistant · 2025-12-30T10:52:53Z

All committers have signed the CLA.

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

barsh404error · 2025-12-31T13:34:06Z

@tj-wazei I've added your request and fixed them
I’ve updated the implementation to avoid mutating shared state in async callbacks and now collect the results before doing replacements sequentially.
I also reuse a single HttpClient and added a GET fallback when HEAD is not supported.
Let me know if anything else should be adjusted ^^

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

Zabuzard

The added methods are utility methods and for those it is crucial that they have a proper Javadoc explaining what they do and give examples.

The isLinkBroken method needs to explain what it means for a link to be broken and how it behaves in edge case, for example if the given text isnt a url and so on.

The other method needs to explain in more detail what the two parameters are and how they work, maybe giving a concrete example in the javadoc.

barsh404error · 2026-01-06T16:29:53Z

The added methods are utility methods and for those it is crucial that they have a proper Javadoc explaining what they do and give examples.

The isLinkBroken method needs to explain what it means for a link to be broken and how it behaves in edge case, for example if the given text isnt a url and so on.

The other method needs to explain in more detail what the two parameters are and how they work, maybe giving a concrete example in the javadoc.

Yes, i got that solved made javadoc for isLinkBroken and replaceDeadLinks.
i explained what does it do step by step altho idk if i wrote to long

tj-wazei

LGTM

Zabuzard · 2026-01-07T08:13:23Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+                int status = response.statusCode();
+                return status < 200 || status >= 400;
+            })
+            .exceptionally(ignored -> true)


the idiomatic name for something you ignore is _

Renamed ignored lambda parameters to _ where applicable (e.g. exceptionally(_ -> ...), thenApply(_ -> ...)) to clearly indicate intentional non-usage.

you forgot one:

.exceptionally(ignored -> true); // still never null

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

Zabuzard · 2026-01-07T08:25:25Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+        List<CompletableFuture<String>> deadLinkFutures = links.stream()
+            .distinct()
+            .map(link -> isLinkBroken(link)
+                .thenApply(isBroken -> Boolean.TRUE.equals(isBroken) ? link : null))


better: instead of map(foo -> ...thenApply(...)), use .map(foo -> ...).filter(...) then you also dont have all these null items in ur list, polluting it

not done yet

Zabuzard · 2026-01-07T08:25:46Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+        return CompletableFuture.allOf(deadLinkFutures.toArray(new CompletableFuture[0]))
+            .thenApply(ignored -> deadLinkFutures.stream()
+                .map(CompletableFuture::join)
+                .filter(Objects::nonNull)


.filter(Objects::nonNull) that one isnt needed anymore with the above fix

not done yet

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

barsh404error · 2026-01-09T11:41:04Z

I focused primarily on improving the Javadocs as requested @Zabuzard
While doing so, I slightly refactored the internal stream to use Optional instead of null to better match the documented behavior and avoid null handling
No external behavior was changed.

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

Zabuzard · 2026-01-09T11:50:35Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

            .map(link -> isLinkBroken(link)
-                .thenApply(isBroken -> Boolean.TRUE.equals(isBroken) ? link : null))
-
+                .thenApply(isBroken -> isBroken ? Optional.of(link) : Optional.<String>empty()))


u misunderstood me. its better if u simply filter out these, skip them here instead of later.
.filter(...) so they are not even part of the list

not done yet

barsh404error · 2026-01-13T16:45:47Z

Thanks for the detailed review @Zabuzard I went through the comments one by one and addressed them explicitly:

Javadoc & behavior clarification

Expanded Javadoc for isLinkBroken to clearly define what “broken” means:

HTTP request failure

HTTP status codes outside the 200–399 range

Clarified that:

Status 200 is considered valid even with an empty body

Response body content is not inspected

Invalid URL formats result in an IllegalArgumentException

HEAD / GET request logic

HEAD is used as an initial, cheaper check

If HEAD indicates failure, a GET request is used as a fallback to handle servers that don’t implement HEAD correctly

Any exception during either request is treated as a broken link

Naming improvements

Renamed request variables to describe intent rather than HTTP mechanics (e.g. link status validation rather than fallback flow)

Asynchronous behavior

replaceDeadLinks performs all link checks asynchronously

CompletableFuture.allOf(...) is used only as a synchronization point; no blocking is introduced

thenApply(_ -> …) is intentionally used to ignore the unused result and avoid misleading variable names

replaceDeadLinks behavior

Added a concrete usage example to the Javadoc showing input → output transformation

Only links detected as broken are replaced

Working links and non-URL content remain unchanged

Duplicate links are checked once and replaced consistently

Edge cases

Empty input or no detected links returns the original text

Empty response bodies are not treated as broken

Exceptions during HTTP checks are handled defensively and result in “broken”

sorry for stopping to answer those comments one by one it was annoying and frustrating lol

Zabuzard · 2026-01-14T07:46:53Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java


-    private static final Set<LinkFilter> DEFAULT_FILTERS =
+    /**
+     * Default filters used when extracting links: skip suppressed URLs and non-http schemes.


You are repeating what the code already says, this javadoc isnt helpful. Instead, make an example so people understand the why 👍

Zabuzard · 2026-01-14T07:49:48Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+                StringBuilder sb = new StringBuilder(text);
                for (String deadLink : deadLinks) {
-                    result = result.replace(deadLink, replacement);
+                    int idx = sb.indexOf(deadLink);
+                    while (idx != -1) {
+                        sb.replace(idx, idx + deadLink.length(), replacement);
+                        idx = sb.indexOf(deadLink, idx + replacement.length());
+                    }


i find that code a bit too complex to just put it between everything else. move it into a quick private static helper method. sth like private static void replace(StringBuilder haystack, String needle, String replacement)

Zabuzard · 2026-01-14T07:51:49Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+
+

double empty line

Zabuzard · 2026-01-14T07:55:13Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+     * @return a future containing the modified text
+     */
+
+    public static CompletableFuture<String> replaceDeadLinks(String text, String replacement) {


not done yet

Zabuzard · 2026-01-14T07:57:16Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+        List<CompletableFuture<String>> deadLinkFutures = links.stream()
+            .distinct()
+            .map(link -> isLinkBroken(link)
+                .thenApply(isBroken -> Boolean.TRUE.equals(isBroken) ? link : null))


not done yet

Zabuzard · 2026-01-14T07:57:24Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

+        return CompletableFuture.allOf(deadLinkFutures.toArray(new CompletableFuture[0]))
+            .thenApply(ignored -> deadLinkFutures.stream()
+                .map(CompletableFuture::join)
+                .filter(Objects::nonNull)


not done yet

Zabuzard · 2026-01-14T07:58:09Z

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java

            .map(link -> isLinkBroken(link)
-                .thenApply(isBroken -> Boolean.TRUE.equals(isBroken) ? link : null))
-
+                .thenApply(isBroken -> isBroken ? Optional.of(link) : Optional.<String>empty()))


not done yet

Add utilities to detect and replace broken links.

9004e6b

barsh404error requested a review from a team as a code owner December 30, 2025 10:52

tj-wazei requested changes Dec 30, 2025

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java Outdated Show resolved Hide resolved

style: run spotlessApply

7153833

Add utilities to detect and replace broken links V2

d205074

barsh404error requested a review from tj-wazei January 2, 2026 10:22

christolis requested changes Jan 2, 2026

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java Show resolved Hide resolved

application/src/main/java/org/togetherjava/tjbot/features/utils/LinkDetection.java Outdated Show resolved Hide resolved

Add utilities to detect and replace broken links V2

c3a64e3

barsh404error requested a review from christolis January 2, 2026 19:14

Add utilities to detect and replace broken links V2

48c23ab

barsh404error changed the title ~~Add utilities to detect and replace broken links.~~ Add utilities to detect and replace broken links. Fixed #1276 Jan 3, 2026

christolis changed the title ~~Add utilities to detect and replace broken links. Fixed #1276~~ Add utilities to detect and replace broken links. Jan 4, 2026

Zabuzard self-requested a review January 4, 2026 15:37

Zabuzard requested changes Jan 4, 2026

View reviewed changes

Add utilities to detect and replace broken links V2

d556a33

barsh404error requested a review from Zabuzard January 6, 2026 16:30

tj-wazei previously approved these changes Jan 6, 2026

View reviewed changes

Zabuzard reviewed Jan 7, 2026

View reviewed changes

Zabuzard requested changes Jan 7, 2026

View reviewed changes

Add utilities to detect and replace broken links V2

66b8d6e

barsh404error dismissed tj-wazei’s stale review via 66b8d6e January 9, 2026 11:39

Zabuzard requested changes Jan 9, 2026

View reviewed changes

Add utilities to detect and replace broken links V2

24998cf

barsh404error requested a review from Zabuzard January 13, 2026 16:48

Zabuzard requested changes Jan 14, 2026

View reviewed changes

Uh oh!

Add utilities to detect and replace broken links. #1366

Are you sure you want to change the base?

Add utilities to detect and replace broken links. #1366

Uh oh!

Conversation

barsh404error commented Dec 30, 2025 • edited by Zabuzard Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

barsh404error commented Dec 31, 2025

Uh oh!

Uh oh!

Uh oh!

Zabuzard left a comment

Choose a reason for hiding this comment

Uh oh!

barsh404error commented Jan 6, 2026

Uh oh!

tj-wazei left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

barsh404error commented Jan 9, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barsh404error commented Jan 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

barsh404error commented Dec 30, 2025 •

edited by Zabuzard

Loading

CLAassistant commented Dec 30, 2025 •

edited

Loading