From f5b409222c65e7861df7e8000311888e26c33fa0 Mon Sep 17 00:00:00 2001 From: Jim Bethancourt Date: Sun, 4 Oct 2026 21:42:32 -0500 Subject: [PATCH 1/9] Assembling links instead of providing pre-rendered renderedLabel for class relationships --- .../refactorfirst/report/JsonGenerator.java | 92 ++++--------------- .../report/model/ClassRelationshipDTO.java | 5 +- .../templates/refactor-first-report.mustache | 4 +- .../report/JsonGeneratorTest.java | 80 +++++++++++----- .../report/MustacheTemplateRenderingTest.java | 11 ++- ...geRelationshipClassEdgeCycleCountTest.java | 4 +- ...PackageRelationshipClassEdgeOrderTest.java | 2 +- 7 files changed, 87 insertions(+), 111 deletions(-) diff --git a/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java b/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java index f7102e97..b328af0f 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/JsonGenerator.java @@ -266,25 +266,16 @@ public RefactorFirstReportDTO generateReportData( String startVertex = vertexes[0].trim(); String endVertex = vertexes[1].trim(); - String sourcePath = - codebaseGraphDTO.getClassToSourceFilePathMapping().get(startVertex); - String targetPath = - codebaseGraphDTO.getClassToSourceFilePathMapping().get(endVertex); - String sourceUrl = (sourcePath != null && !sourcePath.isBlank()) ? repoUrl + sourcePath : ""; - String targetUrl = (targetPath != null && !targetPath.isBlank()) ? repoUrl + targetPath : ""; - - // Build rendered label with links to class files - String renderedLabel = - buildClassLinkLabel(startVertex, endVertex, sourceUrl, targetUrl, classesToRemove, (int) - classGraph.getEdgeWeight(edgeInfo.getEdge())); - classRelList.add(ClassRelationshipDTO.builder() .sourceClass(startVertex) .targetClass(endVertex) + .sourceClassPath(classSourcePath(startVertex, codebaseGraphDTO)) + .targetClassPath(classSourcePath(endVertex, codebaseGraphDTO)) + .simpleSourceClassName(getClassName(startVertex)) + .simpleTargetClassName(getClassName(endVertex)) .sourceMarked(classesToRemove.contains(startVertex)) .targetMarked(classesToRemove.contains(endVertex)) .weight((int) classGraph.getEdgeWeight(edgeInfo.getEdge())) - .renderedLabel(renderedLabel) .priority(edgeInfo.getPriority()) .cycleCount(edgeInfo.getCycleCount()) .effortRank(edgeInfo.getEffortRank()) @@ -386,7 +377,7 @@ public RefactorFirstReportDTO generateReportData( } List breakClassRels = buildClassRelationshipsToBreakPackage( - classRelationshipsInPackageRelationship, repoUrl, codebaseGraphDTO); + classRelationshipsInPackageRelationship, codebaseGraphDTO); // Build rendered label with links to package directories String renderedLabel = @@ -850,7 +841,7 @@ private String buildPackageLinkLabel( * feedback-arc-set members — as members of zero class cycles. */ List buildClassRelationshipsToBreakPackage( - Set classEdges, String repoUrl, CodebaseGraphDTO codebaseGraphDTO) { + Set classEdges, CodebaseGraphDTO codebaseGraphDTO) { if (classEdges == null || classEdges.isEmpty()) { return List.of(); } @@ -866,29 +857,30 @@ List buildClassRelationshipsToBreakPackage( String[] vertexes = extractVertexes(classEdge); String startVertex = vertexes[0].trim(); String endVertex = vertexes[1].trim(); - - String sourcePath = - codebaseGraphDTO.getClassToSourceFilePathMapping().get(startVertex); - String targetPath = - codebaseGraphDTO.getClassToSourceFilePathMapping().get(endVertex); - String sourceUrl = (sourcePath != null && !sourcePath.isBlank()) ? repoUrl + sourcePath : ""; - String targetUrl = (targetPath != null && !targetPath.isBlank()) ? repoUrl + targetPath : ""; int weight = (int) classGraph.getEdgeWeight(classEdge); relationships.add(ClassRelationshipDTO.builder() .sourceClass(startVertex) .targetClass(endVertex) + .sourceClassPath(classSourcePath(startVertex, codebaseGraphDTO)) + .targetClassPath(classSourcePath(endVertex, codebaseGraphDTO)) + .simpleSourceClassName(getClassName(startVertex)) + .simpleTargetClassName(getClassName(endVertex)) .sourceMarked(classesToRemove.contains(startVertex)) .targetMarked(classesToRemove.contains(endVertex)) .weight(weight) - .renderedLabel( - buildClassLinkLabel(startVertex, endVertex, sourceUrl, targetUrl, classesToRemove, weight)) .cycleCount(cycleCounts.get(classEdge)) .build()); } return relationships; } + /** Returns the class's source file path relative to the project root, or an empty string. */ + private String classSourcePath(String className, CodebaseGraphDTO codebaseGraphDTO) { + String path = codebaseGraphDTO.getClassToSourceFilePathMapping().get(className); + return path != null ? path : ""; + } + /** * Counts how many of the detected class cycles contain the given class edge — the same * counting {@code CycleRemovalComputer} applies to the edges it marks for removal. Returns @@ -906,56 +898,4 @@ private int countClassCyclesContaining(DefaultWeightedEdge classEdge) { } return cycleCount; } - - /** - * Builds a rendered label with HTML links to class files. - * Transforms "GraphMetricsCollector → DependencyCollector : 1" - * into "GraphMetricsCollector → DependencyCollector* : 1" - */ - private String buildClassLinkLabel( - String startVertex, - String endVertex, - String sourceUrl, - String targetUrl, - Set classesToRemove, - int weight) { - // Extract simple class names for display - String startClass = getClassName(startVertex); - String endClass = getClassName(endVertex); - - // Check if classes are marked for removal - boolean startMarked = classesToRemove.contains(startVertex); - boolean endMarked = classesToRemove.contains(endVertex); - - // Build the new label with links - StringBuilder newLabel = new StringBuilder(); - if (sourceUrl != null && !sourceUrl.isBlank()) { - newLabel.append("") - .append(escapeHtmlLabel(startClass)) - .append(""); - } else { - newLabel.append(escapeHtmlLabel(startClass)); - } - if (startMarked) { - newLabel.append("*"); - } - newLabel.append(" → "); - if (targetUrl != null && !targetUrl.isBlank()) { - newLabel.append("") - .append(escapeHtmlLabel(endClass)) - .append(""); - } else { - newLabel.append(escapeHtmlLabel(endClass)); - } - if (endMarked) { - newLabel.append("*"); - } - newLabel.append(" : ").append(weight); - - return newLabel.toString(); - } } diff --git a/report/src/main/java/org/hjug/refactorfirst/report/model/ClassRelationshipDTO.java b/report/src/main/java/org/hjug/refactorfirst/report/model/ClassRelationshipDTO.java index 46b52c0d..6e5d7329 100644 --- a/report/src/main/java/org/hjug/refactorfirst/report/model/ClassRelationshipDTO.java +++ b/report/src/main/java/org/hjug/refactorfirst/report/model/ClassRelationshipDTO.java @@ -12,10 +12,13 @@ public class ClassRelationshipDTO { private String sourceClass; private String targetClass; + private String sourceClassPath; + private String targetClassPath; + private String simpleSourceClassName; + private String simpleTargetClassName; private boolean sourceMarked; private boolean targetMarked; private int weight; - private String renderedLabel; private int priority; private int cycleCount; private int effortRank; diff --git a/report/src/main/resources/templates/refactor-first-report.mustache b/report/src/main/resources/templates/refactor-first-report.mustache index 7f1faf27..7c517985 100644 --- a/report/src/main/resources/templates/refactor-first-report.mustache +++ b/report/src/main/resources/templates/refactor-first-report.mustache @@ -210,7 +210,7 @@ {{#classRelationshipsToRemove.relationships}} - {{renderedLabel}} + {{simpleSourceClassName}}{{#sourceMarked}}*{{/sourceMarked}} → {{simpleTargetClassName}}{{#targetMarked}}*{{/targetMarked}} {{priority}} {{cycleCount}} {{effortRank}} @@ -298,7 +298,7 @@ {{cycleCount}} {{effortRank}} {{#classRelationshipsToBreakPackage}} - {{{renderedLabel}}}
{{/classRelationshipsToBreakPackage}} + {{simpleSourceClassName}}{{#sourceMarked}}*{{/sourceMarked}} → {{simpleTargetClassName}}{{#targetMarked}}*{{/targetMarked}}
{{/classRelationshipsToBreakPackage}} {{/packageRelationshipsToRemove.relationships}} diff --git a/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java b/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java index 7f5ed731..7f0e3138 100644 --- a/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java +++ b/report/src/test/java/org/hjug/refactorfirst/report/JsonGeneratorTest.java @@ -337,10 +337,12 @@ public String execute() { } /** - * Verifies that sourceUrl and targetUrl are set in ClassRelationshipDTO and PackageRelationshipDTO. + * Verifies that sourceClassPath, targetClassPath, simpleSourceClassName and + * simpleTargetClassName are set in ClassRelationshipDTO, and that the label + * is no longer pre-rendered server-side. */ @Test - void given_circularDependency_when_reportGenerated_then_relationshipUrlsAreSet() throws Exception { + void given_circularDependency_when_reportGenerated_then_classPathsAndSimpleNamesAreSet() throws Exception { // Given File repoDir = tempDir.toFile(); new File(repoDir, ".git").mkdirs(); @@ -402,30 +404,32 @@ public ClassB(ClassA classA) { RefactorFirstReportDTO report = objectMapper.readValue(jsonFile.toFile(), RefactorFirstReportDTO.class); assertNotNull(report); - // Verify ClassRelationshipDTO renderedLabel contains HTML links + // Verify ClassRelationshipDTO carries the source file paths and simple class names + // so the viewer can combine them with project.repoUrl itself assertNotNull(report.getClassRelationshipsToRemove()); assertNotNull(report.getClassRelationshipsToRemove().getRelationships()); assertFalse(report.getClassRelationshipsToRemove().getRelationships().isEmpty()); var classRel = report.getClassRelationshipsToRemove().getRelationships().get(0); - assertNotNull(classRel.getRenderedLabel(), "ClassRelationshipDTO should have renderedLabel"); - assertTrue(classRel.getRenderedLabel().contains("= 1, "weight should be at least 1"); - assertNotNull(classRel.getRenderedLabel(), "ClassRelationshipDTO should have renderedLabel"); assertTrue( - classRel.getRenderedLabel().contains("→"), - "renderedLabel should contain the relationship arrow"); + classRel.getSourceClassPath().startsWith("src/main/java/com/example/pkg"), + "sourceClassPath should be the source file path relative to the project root: " + + classRel.getSourceClassPath()); + assertTrue( + classRel.getTargetClassPath().startsWith("src/main/java/com/example/pkg"), + "targetClassPath should be the source file path relative to the project root: " + + classRel.getTargetClassPath()); + assertTrue( + classRel.getSimpleSourceClassName().startsWith("Class"), + "simpleSourceClassName should be the simple source class name: " + + classRel.getSimpleSourceClassName()); + assertTrue( + classRel.getSimpleTargetClassName().startsWith("Class"), + "simpleTargetClassName should be the simple target class name: " + + classRel.getSimpleTargetClassName()); } } assertTrue(totalClassRelationships > 0, "Expected at least one class relationship to break the package cycle"); + + // The nested class relationships must not carry a server-rendered label either + com.fasterxml.jackson.databind.JsonNode root = objectMapper.readTree(jsonFile.toFile()); + for (com.fasterxml.jackson.databind.JsonNode packageRel : + root.get("packageRelationshipsToRemove").get("relationships")) { + for (com.fasterxml.jackson.databind.JsonNode classRel : + packageRel.get("classRelationshipsToBreakPackage")) { + assertFalse(classRel.has("renderedLabel"), "Nested class relationships must not carry a renderedLabel"); + assertTrue(classRel.has("sourceClassPath")); + assertTrue(classRel.has("targetClassPath")); + assertTrue(classRel.has("simpleSourceClassName")); + assertTrue(classRel.has("simpleTargetClassName")); + } + } } } diff --git a/report/src/test/java/org/hjug/refactorfirst/report/MustacheTemplateRenderingTest.java b/report/src/test/java/org/hjug/refactorfirst/report/MustacheTemplateRenderingTest.java index 499fd894..626a49bf 100644 --- a/report/src/test/java/org/hjug/refactorfirst/report/MustacheTemplateRenderingTest.java +++ b/report/src/test/java/org/hjug/refactorfirst/report/MustacheTemplateRenderingTest.java @@ -123,7 +123,10 @@ void testTemplateRendersClassRelationshipTable() throws Exception { .sourceMarked(true) .targetMarked(false) .weight(5) - .renderedLabel("A → B") + .sourceClassPath("src/main/java/com/example/A.java") + .targetClassPath("src/main/java/com/example/B.java") + .simpleSourceClassName("A")); + // The cell renders the simple class names separated by the arrow, with the removal + // marker; Mustache escapes the names so no markup can be injected + assertTrue(rendered.contains("A<script>* → B")); + assertFalse(rendered.contains("