From 9b7f82f1c62cbd1c9597e4e472355403668f4ad9 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 31 Jan 2023 09:21:35 +0100 Subject: [PATCH] Refactor link checker. Use ReportFlags as value class for easier filtering. Use sorted tables for output. Use WebClient for concurrent resource I/O. See #22 --- pom.xml | 27 +- .../data/release/build/BuildOperations.java | 7 +- .../data/release/build/MavenBuildSystem.java | 4 - .../documentation/DocumentationCommands.java | 145 ++++++++-- .../DocumentationConfiguration.java | 51 ++++ .../DocumentationOperations.java | 256 ++++++++++-------- 6 files changed, 333 insertions(+), 157 deletions(-) create mode 100644 src/main/java/org/springframework/data/release/documentation/DocumentationConfiguration.java diff --git a/pom.xml b/pom.xml index 67106e4..eb6bfe2 100644 --- a/pom.xml +++ b/pom.xml @@ -1,6 +1,6 @@ + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> 4.0.0 org.springframework.data.build spring-data-release-cli @@ -34,6 +34,16 @@ spring-web + + org.springframework + spring-webflux + + + + io.projectreactor.netty + reactor-netty-http + + org.springframework.data spring-data-commons @@ -90,6 +100,12 @@ 1.15.3 + + com.google.guava + guava + 31.1-jre + + org.projectlombok lombok @@ -144,13 +160,6 @@ test - - com.google.guava - guava - 31.1-jre - test - - javax.servlet javax.servlet-api diff --git a/src/main/java/org/springframework/data/release/build/BuildOperations.java b/src/main/java/org/springframework/data/release/build/BuildOperations.java index 3365239..922e013 100644 --- a/src/main/java/org/springframework/data/release/build/BuildOperations.java +++ b/src/main/java/org/springframework/data/release/build/BuildOperations.java @@ -108,7 +108,6 @@ public class BuildOperations { return doWithBuildSystem(iteration, (system, module) -> system.prepareVersion(module, phase)); } - /** * Opens a repository to stage artifacts for this {@link ModuleIteration}. * @@ -151,15 +150,15 @@ public class BuildOperations { public void buildDocumentation(TrainIteration iteration) { - executor.doWithBuildSystemOrdered(Streamable.of(iteration.getModulesExcept(BOM, COMMONS, BUILD)), BuildSystem::triggerDocumentationBuild); + executor.doWithBuildSystemOrdered(Streamable.of(iteration.getModulesExcept(BOM, COMMONS, BUILD)), + BuildSystem::triggerDocumentationBuild); logger.log(iteration, "Documentation build finished"); } public void buildDocumentation(ModuleIteration iteration) { - // TODO: check if this call is fine - // executor.doWithBuildSystemOrdered(Streamable.of(iteration), BuildSystem::triggerDocumentationBuild); + doWithBuildSystem(iteration, BuildSystem::triggerDocumentationBuild); logger.log(iteration, "Documentation build finished"); } diff --git a/src/main/java/org/springframework/data/release/build/MavenBuildSystem.java b/src/main/java/org/springframework/data/release/build/MavenBuildSystem.java index cf06232..9242301 100644 --- a/src/main/java/org/springframework/data/release/build/MavenBuildSystem.java +++ b/src/main/java/org/springframework/data/release/build/MavenBuildSystem.java @@ -359,10 +359,6 @@ class MavenBuildSystem implements BuildSystem { public M triggerDocumentationBuild(M module) { Project project = module.getProject(); - if(!isMavenProject(project)) { - logger.log(project, "Skipping project as no pom.xml could be found in the working directory!"); - return module; - } mvn.execute(project, CommandLine.of(Goal.CLEAN, Goal.INSTALL, SKIP_TESTS, profile("distribute"))); diff --git a/src/main/java/org/springframework/data/release/documentation/DocumentationCommands.java b/src/main/java/org/springframework/data/release/documentation/DocumentationCommands.java index 958baa7..6f7f642 100644 --- a/src/main/java/org/springframework/data/release/documentation/DocumentationCommands.java +++ b/src/main/java/org/springframework/data/release/documentation/DocumentationCommands.java @@ -17,16 +17,25 @@ package org.springframework.data.release.documentation; import static org.springframework.data.release.model.Projects.*; -import java.io.File; -import java.util.Arrays; -import java.util.concurrent.ExecutorService; - import lombok.NonNull; import lombok.RequiredArgsConstructor; + +import java.io.File; +import java.util.Collection; +import java.util.Comparator; +import java.util.Optional; +import java.util.concurrent.ExecutorService; +import java.util.function.BiConsumer; +import java.util.stream.Collectors; + +import org.fusesource.jansi.Ansi; +import org.fusesource.jansi.Ansi.Color; import org.springframework.data.release.CliComponent; import org.springframework.data.release.TimedCommand; import org.springframework.data.release.build.BuildOperations; import org.springframework.data.release.cli.StaticResources; +import org.springframework.data.release.documentation.DocumentationOperations.CheckedLink; +import org.springframework.data.release.documentation.DocumentationOperations.PageStats; import org.springframework.data.release.documentation.DocumentationOperations.ReportFlags; import org.springframework.data.release.io.Workspace; import org.springframework.data.release.model.ModuleIteration; @@ -35,8 +44,11 @@ import org.springframework.data.release.model.TrainIteration; import org.springframework.data.release.utils.ExecutionUtils; import org.springframework.data.release.utils.Logger; import org.springframework.data.util.Streamable; +import org.springframework.http.HttpStatus; import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.core.annotation.CliOption; +import org.springframework.shell.support.table.Table; +import org.springframework.shell.support.table.TableHeader; import org.springframework.util.StringUtils; /** @@ -53,62 +65,139 @@ public class DocumentationCommands extends TimedCommand { private final @NonNull Logger logger; @CliCommand("docs check-links") - public void checkLinks(@CliOption(key = "", mandatory = true) TrainIteration iteration, @CliOption(key = "project", mandatory = false) Project project, @CliOption(key = "local", mandatory = false, unspecifiedDefaultValue = "false") boolean preview, @CliOption(key = "report", mandatory = false) String options) { + public Table checkLinks(@CliOption(key = "", mandatory = true) TrainIteration iteration, + @CliOption(key = "project", mandatory = false) Project project, + @CliOption(key = "local", mandatory = false, unspecifiedDefaultValue = "false") boolean preview, + @CliOption(key = "report", mandatory = false) String options) { if (project != null) { - checkLinks(iteration.getModule(project), preview, options); - return; + return checkLinks(iteration.getModule(project), preview, options); } - ExecutionUtils.run(executorService, Streamable.of(iteration.getModulesExcept(BUILD, BOM, COMMONS)), module -> { - checkLinks(module, preview, options); - }); + Collection> optionals = ExecutionUtils.runAndReturn(executorService, + Streamable.of(iteration.getModulesExcept(BUILD, BOM, COMMONS)), module -> { + Optional path = prepareDocumentationCheck(module, preview); + return path.map(it -> operations.checkDocumentation(module.getProject(), it)); + }); + + return render(optionals.stream().filter(Optional::isPresent).map(Optional::get).collect(Collectors.toList()), + options); } @CliCommand("check-links") - public void checkLinks(@CliOption(key = "", mandatory = true) String url, @CliOption(key = "report", mandatory = false) String options) { + public Table checkLinks(@CliOption(key = "", mandatory = true) String url, + @CliOption(key = "report", mandatory = false) String options) { - String result = operations.checkDocumentation(url).prettyPrint(readFlags(options)); - System.out.printf("Link Statistic:\r\n%s", result); + PageStats pageStats = operations.checkDocumentation(BOM, url); + + return render(pageStats, options); } - public void checkLinks(ModuleIteration module, boolean preview, String options) { + public Optional prepareDocumentationCheck(ModuleIteration module, boolean preview) { String path; - if (preview) { + if (preview) { buildOperations.buildDocumentation(module); + File projectDirectory = workspace.getProjectDirectory(module.getProject()); + if (!projectDirectory.exists()) { logger.warn(module, "Unable to locate project directory"); - return; + return Optional.empty(); } + File source = new File(projectDirectory, "target/site/reference/html/index.html"); + if (!source.exists()) { logger.warn(module, "Unable to locate reference documentation html %", source); - return; + return Optional.empty(); } + path = source.getPath(); } else { path = new StaticResources(module).getDocumentationUrl(); } - if(!StringUtils.hasText(path)) { + if (!StringUtils.hasText(path)) { logger.warn(module, "Empty path for reference documentation."); - return; + return Optional.empty(); } - String result = operations.checkDocumentation(path).prettyPrint(readFlags(options)); - logger.log(module, "%s Documentation Link Statistic:\r\n%s", module, result); + return Optional.of(path); } - private static ReportFlags[] readFlags(String options) { + private Table checkLinks(ModuleIteration module, boolean preview, String options) { - ReportFlags[] flags = new ReportFlags[]{ReportFlags.ALL}; - if (options != null) { - flags = Arrays.stream(options.split(",")).map(ReportFlags::valueOf).toArray(ReportFlags[]::new); - } - return flags; + Optional path = prepareDocumentationCheck(module, preview); + + return path.map(s -> { + + PageStats pageStats = operations.checkDocumentation(module.getProject(), s); + return render(pageStats, options); + }).orElse(null); + } + + private Table render(Collection results, String options) { + + return createTable(options, (table, reportFlags) -> { + for (PageStats pageStats : results) { + addToTable(pageStats.getProject(), pageStats.filter(reportFlags), table); + } + }); + } + + private Table render(PageStats pageStats, String options) { + + return createTable(options, + (table, reportFlags) -> addToTable(pageStats.getProject(), pageStats.filter(reportFlags), table)); + } + + private Table createTable(String options, BiConsumer c) { + + Table table = createResultTable(); + ReportFlags flags = ReportFlags.parse(options); + + c.accept(table, flags); + return table; + } + + private void addToTable(Project project, PageStats pageStats, Table table) { + + pageStats.sort(Comparator. comparing(it -> it.getResult().value()) + .thenComparing(CheckedLink::getUrl)).forEach(checkedLink -> { + + Ansi ansi = Ansi.ansi(); + HttpStatus status = checkedLink.getResult(); + if (status.is2xxSuccessful()) { + ansi.fg(Color.GREEN); + } else if (status.is4xxClientError()) + ansi.fg(Color.RED); + else if (status.is3xxRedirection()) { + ansi.fg(Color.YELLOW); + } + + String renderedStatus = ansi.a(checkedLink.getResult().value()).fg(Color.DEFAULT).toString(); + + table.addRow(project.getName(), status.value() + "", checkedLink.getUrl()); + }); + } + + private static Table createResultTable() { + + Table table = new Table() { + @Override + public void calculateColumnWidths() {} + }; + table.addHeader(1, newHeader("Module", 15)); + table.addHeader(2, newHeader("Status", 15)); + table.addHeader(3, newHeader("URL", 150)); + return table; + } + + private static TableHeader newHeader(String desc, int width) { + TableHeader tableHeader = new TableHeader(desc, width); + tableHeader.setMaxWidth(width); + return tableHeader; } } - diff --git a/src/main/java/org/springframework/data/release/documentation/DocumentationConfiguration.java b/src/main/java/org/springframework/data/release/documentation/DocumentationConfiguration.java new file mode 100644 index 0000000..fff3e49 --- /dev/null +++ b/src/main/java/org/springframework/data/release/documentation/DocumentationConfiguration.java @@ -0,0 +1,51 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.release.documentation; + +import java.time.Duration; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.client.reactive.ReactorClientHttpConnector; +import org.springframework.http.client.reactive.ReactorResourceFactory; +import org.springframework.web.reactive.function.client.WebClient; + +/** + * @author Mark Paluch + */ +@Configuration +public class DocumentationConfiguration { + + @Bean + ReactorResourceFactory reactorResourceFactory() { + + ReactorResourceFactory resourceFactory = new ReactorResourceFactory(); + resourceFactory.setUseGlobalResources(true); + resourceFactory.setShutdownTimeout(Duration.ZERO); + resourceFactory.setShutdownQuietPeriod(Duration.ZERO); + return resourceFactory; + } + + @Bean + WebClient documentationWebClient(ReactorResourceFactory resourceFactory) { + + return WebClient.builder().clientConnector(new ReactorClientHttpConnector(resourceFactory, httpClient -> { + + return httpClient.responseTimeout(Duration.ofSeconds(5)); + })).build(); + } + +} diff --git a/src/main/java/org/springframework/data/release/documentation/DocumentationOperations.java b/src/main/java/org/springframework/data/release/documentation/DocumentationOperations.java index 8d39e3b..a3649cd 100644 --- a/src/main/java/org/springframework/data/release/documentation/DocumentationOperations.java +++ b/src/main/java/org/springframework/data/release/documentation/DocumentationOperations.java @@ -15,27 +15,33 @@ */ package org.springframework.data.release.documentation; -import java.io.File; -import java.io.IOException; -import java.net.HttpURLConnection; -import java.net.URL; -import java.util.Comparator; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.function.Function; -import java.util.stream.Collectors; - +import lombok.AccessLevel; +import lombok.Getter; import lombok.NonNull; import lombok.RequiredArgsConstructor; -import org.fusesource.jansi.Ansi; -import org.fusesource.jansi.Ansi.Color; +import lombok.Value; + +import java.io.File; +import java.io.IOException; +import java.net.URL; +import java.util.Arrays; +import java.util.Comparator; +import java.util.EnumSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.function.Consumer; +import java.util.stream.Collectors; + import org.jsoup.Jsoup; import org.jsoup.nodes.Document; import org.jsoup.select.Elements; +import org.springframework.data.release.model.Project; import org.springframework.data.release.utils.Logger; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Component; -import org.springframework.util.ObjectUtils; +import org.springframework.web.reactive.function.client.WebClient; /** * @author Christoph Strobl @@ -45,112 +51,134 @@ import org.springframework.util.ObjectUtils; class DocumentationOperations { private final @NonNull Logger logger; + private final WebClient webClient; - PageStats checkDocumentation(String url) { - PageStats stats = new PageStats(); - stats.links(new LinkChecker(logger).inspect(url)); - return stats; + PageStats checkDocumentation(Project project, String url) { + return new PageStats(project, new LinkChecker(logger).inspect(project, url)); } - enum ReportFlags { + enum ReportFlag { - ERROR(1), REDIRECT(2), OK(4), ALL(8); + ERROR(1), REDIRECT(2), OK(4), ALL(8), TO_BE_UPDATED(16); int bin; - ReportFlags(int bin) { + ReportFlag(int bin) { this.bin = bin; } - static int flagsOf(ReportFlags... flags) { - int value = 0; - for (ReportFlags opt : flags) { - value = value | opt.bin; + } + + @RequiredArgsConstructor(access = AccessLevel.PRIVATE) + static class ReportFlags { + + private final EnumSet flags; + + public static ReportFlags parse(String options) { + + EnumSet set = EnumSet.noneOf(ReportFlag.class); + if (options != null) { + Arrays.stream(options.split(",")).map(ReportFlag::valueOf).forEach(set::add); + } else { + set.add(ReportFlag.TO_BE_UPDATED); } - return value; + + return new ReportFlags(set); } - } - static class PageStats { + public boolean isIncluded(HttpStatus status) { - public static final int ERROR = 1; // Binary 00001 - public static final int REDIRECT = 2; // Binary 00010 - public static final int OK = 4; // Binary 00100 - public static final int ALL = 8; // Binary 01000 - - LinkStats linkStats; - - String prettyPrint(ReportFlags... options) { - - if (linkStats != null) { - return linkStats.prettyPrint(options); + if (flags.contains(ReportFlag.ALL)) { + return true; } - return "PageStats n/a"; - } - void links(LinkStats linkStats) { - this.linkStats = linkStats; + if (flags.contains(ReportFlag.TO_BE_UPDATED)) { + return !status.is2xxSuccessful(); + } + + if (status.is2xxSuccessful() && flags.contains(ReportFlag.OK)) { + return true; + } + if (status.is3xxRedirection() && flags.contains(ReportFlag.REDIRECT)) { + return true; + } + if (status.is4xxClientError() && flags.contains(ReportFlag.ERROR)) { + return true; + } + + return false; } } - - static class LinkStats { - - Map resultMap = new LinkedHashMap<>(200); - - public HttpStatus computeIfAbsent(String key, Function mappingFunction) { - return resultMap.computeIfAbsent(key, mappingFunction); - } - - int size() { - return resultMap.size(); - } - - String prettyPrint() { - return prettyPrint(ReportFlags.ALL); - } - - String prettyPrint(ReportFlags... options) { - - int flags = ObjectUtils.isEmpty(options) ? ReportFlags.flagsOf(ReportFlags.ALL) : ReportFlags.flagsOf(options); - - return resultMap.entrySet().stream().filter(entry -> { - if ((flags & ReportFlags.ALL.bin) == ReportFlags.ALL.bin) { - return true; - } - if (entry.getValue().is2xxSuccessful() && (flags & ReportFlags.OK.bin) == ReportFlags.OK.bin) { - return true; - } - if (entry.getValue().is3xxRedirection() && (flags & ReportFlags.REDIRECT.bin) == ReportFlags.REDIRECT.bin) { - return true; - } - if (entry.getValue().is4xxClientError() && (flags & ReportFlags.ERROR.bin) == ReportFlags.ERROR.bin) { - return true; - } - return false; - }).sorted(Comparator.comparingInt(o -> o.getValue().value())).map(entry -> { - Ansi ansi = Ansi.ansi(); - if (entry.getValue().is2xxSuccessful()) { - ansi.fg(Color.GREEN); - } else if (entry.getValue().is4xxClientError()) - ansi.fg(Color.RED); - else if (entry.getValue().is3xxRedirection()) { - ansi.fg(Color.YELLOW); - } - return ansi.a(entry.getValue()).fg(Color.DEFAULT).a(": " + entry.getKey()).toString(); - }).collect(Collectors.joining("\r\n")); - } - } - - @RequiredArgsConstructor - static class LinkChecker { + static class PageStats { + + public static final int ERROR = 1; // Binary 00001 + public static final int REDIRECT = 2; // Binary 00010 + public static final int OK = 4; // Binary 00100 + public static final int ALL = 8; // Binary 01000 + + @Getter final Project project; + final LinkStats linkStats; + + public PageStats filter(ReportFlags reportFlags) { + return new PageStats(project, linkStats.filter(reportFlags)); + } + + public PageStats sort(Comparator comparator){ + return new PageStats(project, linkStats.sort(comparator)); + } + + public void forEach(Consumer consumer) { + linkStats.checkedLinks.forEach(consumer); + } + } + + @RequiredArgsConstructor + static class LinkStats { + + final List checkedLinks; + + int size() { + return checkedLinks.size(); + } + + List getResults() { + return checkedLinks; + } + + public LinkStats filter(ReportFlags reportFlags) { + + List filtered = checkedLinks.stream().filter(entry -> { + HttpStatus status = entry.getResult(); + return reportFlags.isIncluded(status); + }).collect(Collectors.toList()); + + return new LinkStats(filtered); + } + + public LinkStats sort(Comparator comparator) { + + List sorted = checkedLinks.stream().sorted(comparator).collect(Collectors.toList()); + + return new LinkStats(sorted); + } + } + + @Value + static class CheckedLink { + String url; + HttpStatus result; + } + + @RequiredArgsConstructor + class LinkChecker { private final Logger logger; - LinkStats inspect(String url) throws RuntimeException { + LinkStats inspect(Project project, String url) throws RuntimeException { - logger.log("", "Collecting links from: %s", url); + logger.log(project, "Collecting links from: %s", url); Document doc = null; try { @@ -168,35 +196,39 @@ class DocumentationOperations { Elements links = doc.select("a[href]"); // a with href - logger.log("", "Found %s links.", links.size()); + logger.log(project, "Found %s links.", links.size()); - LinkStats stats = new LinkStats(); + Map> resultMap = new LinkedHashMap<>(200); links.forEach(link -> { - if (link.attr("href").startsWith("#")) { + String href = link.attr("href"); + + if (href.startsWith("#")) { return; } - checkUrl(link.attr("href"), stats); + + if (href.contains("#")) { + href = href.substring(0, href.indexOf('#')); + } + + checkUrl(resultMap, href); }); - logger.log("", "Analyzed %s external links.", stats.size()); + List checkedLinks = resultMap.entrySet().stream() + .map(it -> new CheckedLink(it.getKey(), it.getValue().join())).collect(Collectors.toList()); + + LinkStats stats = new LinkStats(checkedLinks); + + logger.log(project, "Analyzed %s external links.", stats.size()); return stats; } - private void checkUrl(String url, LinkStats stats) { + private void checkUrl(Map> resultMap, String url) { - stats.computeIfAbsent(url, key -> { - try { - HttpURLConnection connection = (HttpURLConnection) new URL(key).openConnection(); - connection.setRequestMethod("HEAD"); - connection.setConnectTimeout(5000); - connection.setReadTimeout(8000); - return HttpStatus.valueOf(connection.getResponseCode()); - } catch (Exception ex) { - return HttpStatus.valueOf(500); - } - }); + resultMap.computeIfAbsent(url, key -> webClient.get().uri(url) + .exchangeToMono(clientResponse -> clientResponse.toBodilessEntity().thenReturn(clientResponse.statusCode())) + .onErrorReturn(HttpStatus.INTERNAL_SERVER_ERROR).toFuture()); } } }