From b73e7b30b0fd31caa6d87c270694d4666613303a Mon Sep 17 00:00:00 2001 From: Marcus Sanchez Date: Sat, 6 Mar 2021 14:43:29 -0500 Subject: [PATCH 1/3] Fix Issue 56, added better way to extract the file path. --- .../appland/appmap/output/v1/CodeObject.java | 49 +++++++++++-------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/appland/appmap/output/v1/CodeObject.java b/src/main/java/com/appland/appmap/output/v1/CodeObject.java index 6b0e1369..fcdd2332 100644 --- a/src/main/java/com/appland/appmap/output/v1/CodeObject.java +++ b/src/main/java/com/appland/appmap/output/v1/CodeObject.java @@ -1,18 +1,15 @@ package com.appland.appmap.output.v1; import com.alibaba.fastjson.annotation.JSONField; - import com.appland.appmap.util.Logger; - import javassist.CtBehavior; import javassist.CtClass; +import javassist.bytecode.SourceFileAttribute; +import java.io.File; import java.lang.reflect.Modifier; -import java.util.ArrayDeque; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.Collections; +import java.util.*; +import java.util.regex.Matcher; /** * Represents a package, class or method. @@ -79,7 +76,7 @@ public CodeObject setLocation(String location) { public List getChildren() { return this.children; } - + /** * Return the list of children, or an empty list if there are * none. {@code getChildren} behaves differently to satisfy @@ -100,15 +97,15 @@ private CodeObject setFile(String file) { this.file = file; return this; } - + @JSONField(serialize = false, deserialize = false) private Integer lineno; private CodeObject setLineno(Integer lineno) { this.lineno = lineno; return this; } - - + + @Override public boolean equals(Object obj) { if (obj == null) { @@ -129,7 +126,7 @@ public boolean equals(Object obj) { && codeObject.isStatic == this.isStatic && (codeObject.file == null? this.file == null : codeObject.file.equals(this.file)) && codeObject.lineno == this.lineno; - + } /** @@ -194,12 +191,22 @@ public CodeObject(CodeObject src) { * @return An estimated source file path */ public static String getSourceFilePath(CtClass classType) { - String[] parts = { - "src", "main", "java", - classType.getPackageName().replace('.', '/'), - classType.getClassFile().getSourceFile() - }; - return String.join("/", parts); + + String escapedSeparator = Matcher.quoteReplacement(File.separator); + String sourceFilePath = null; + if (classType.getClassFile2().getAttribute("SourceFile") != null) { + //If debug info is available use it. + sourceFilePath = + classType.getName().substring(0, classType.getName().lastIndexOf(".")) + .replaceAll("\\.", escapedSeparator) + + ((SourceFileAttribute) classType.getClassFile2() + .getAttribute("SourceFile")).getFileName(); + } else { //If no debug info is available use new heuristics + sourceFilePath = + classType.getName().replaceAll("\\.", escapedSeparator) + .replaceAll("\\$\\w+", "") + ".java"; + } + return sourceFilePath; } /** @@ -321,7 +328,7 @@ public CodeObject get(String path) { public CodeObject findChild(String name, Boolean isStatic, int lineNumber) { for (CodeObject child : this.safeGetChildren()) { if (child.name.equals(name) - && child.isStatic == isStatic + && child.isStatic == isStatic && child.lineno == lineNumber) { return child; } @@ -357,7 +364,7 @@ private boolean equalBySubstring(String s1, String s2, int start, int end) { return true; } - + public CodeObject findChildBySubstring(String name, int start, int end) { for (CodeObject child : this.safeGetChildren()) { if (equalBySubstring(child.name, name, start, end)) { @@ -367,7 +374,7 @@ public CodeObject findChildBySubstring(String name, int start, int end) { return null; } - + /** * Add an immediate child to this CodeObject. * @param child The child to be added From aa4e086527a982106ab776a6e38e66af6def2003 Mon Sep 17 00:00:00 2001 From: Marcus Sanchez Date: Sat, 6 Mar 2021 15:52:16 -0500 Subject: [PATCH 2/3] Fix Issue 56, added better way to extract the file path. Added tests for this. Removed src/main/java from file path calculation. --- .../appland/appmap/output/v1/CodeObject.java | 28 +++++++++-------- .../appmap/output/v1/CodeObjectTest.java | 31 +++++++++++++++++++ .../output/v1/testclasses/Anonymous.java | 13 ++++++++ .../v1/testclasses/ExampleInnerClass.java | 11 +++++++ 4 files changed, 70 insertions(+), 13 deletions(-) create mode 100644 src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java create mode 100644 src/test/java/com/appland/appmap/output/v1/testclasses/Anonymous.java create mode 100644 src/test/java/com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java diff --git a/src/main/java/com/appland/appmap/output/v1/CodeObject.java b/src/main/java/com/appland/appmap/output/v1/CodeObject.java index fcdd2332..059a0af1 100644 --- a/src/main/java/com/appland/appmap/output/v1/CodeObject.java +++ b/src/main/java/com/appland/appmap/output/v1/CodeObject.java @@ -6,7 +6,6 @@ import javassist.CtClass; import javassist.bytecode.SourceFileAttribute; -import java.io.File; import java.lang.reflect.Modifier; import java.util.*; import java.util.regex.Matcher; @@ -191,24 +190,27 @@ public CodeObject(CodeObject src) { * @return An estimated source file path */ public static String getSourceFilePath(CtClass classType) { - - String escapedSeparator = Matcher.quoteReplacement(File.separator); String sourceFilePath = null; if (classType.getClassFile2().getAttribute("SourceFile") != null) { - //If debug info is available use it. - sourceFilePath = - classType.getName().substring(0, classType.getName().lastIndexOf(".")) - .replaceAll("\\.", escapedSeparator) - + ((SourceFileAttribute) classType.getClassFile2() - .getAttribute("SourceFile")).getFileName(); - } else { //If no debug info is available use new heuristics - sourceFilePath = - classType.getName().replaceAll("\\.", escapedSeparator) - .replaceAll("\\$\\w+", "") + ".java"; + sourceFilePath = getSourceFilePathWithDebugInfo(classType); + } else { + sourceFilePath = getSourceCodePath(classType); } return sourceFilePath; } + private static String getSourceFilePathWithDebugInfo(CtClass classType) { + return classType.getName().substring(0, classType.getName().lastIndexOf(".")) + .replaceAll("\\.", Matcher.quoteReplacement("/")) + + "/" + ((SourceFileAttribute) classType.getClassFile2() + .getAttribute("SourceFile")).getFileName(); + } + + private static String getSourceCodePath(CtClass classType) { + return classType.getName().replaceAll("\\.", Matcher.quoteReplacement("/")) + .replaceAll("\\$\\w+", "") + ".java"; + } + /** * Splits a package name and creates a tree of CodeObjects. For example, "com.appland.demo" would * be split into three different "package" CodeObjects: "com", "appland" and "demo". diff --git a/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java b/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java new file mode 100644 index 00000000..1c6edbdb --- /dev/null +++ b/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java @@ -0,0 +1,31 @@ +package com.appland.appmap.output.v1; + +import javassist.ClassPool; +import javassist.CtClass; +import javassist.NotFoundException; +import org.junit.Test; + +import static org.junit.Assert.*; + +public class CodeObjectTest { + + @Test + public void getSourceFilePath_for_RegularClass() throws NotFoundException { + CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.ExampleClass"); + assertEquals("com/appland/appmap/ExampleClass.java", CodeObject.getSourceFilePath(testCtClass)); + } + + @Test + public void getSourceFilePath_for_InnerClass_ResultInBaseClass() throws NotFoundException { + CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.output.v1.testclasses.ExampleInnerClass$StaticFinalInnerClass"); + assertEquals("com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java", CodeObject.getSourceFilePath(testCtClass)); + } + + @Test + public void getSourceFilePath_for_AnonymousClass_ResultInBaseClass() throws NotFoundException { + CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.output.v1.testclasses.Anonymous$1"); + assertEquals("com/appland/appmap/output/v1/testclasses/Anonymous.java", CodeObject.getSourceFilePath(testCtClass)); + } + + +} \ No newline at end of file diff --git a/src/test/java/com/appland/appmap/output/v1/testclasses/Anonymous.java b/src/test/java/com/appland/appmap/output/v1/testclasses/Anonymous.java new file mode 100644 index 00000000..d76e3793 --- /dev/null +++ b/src/test/java/com/appland/appmap/output/v1/testclasses/Anonymous.java @@ -0,0 +1,13 @@ +package com.appland.appmap.output.v1.testclasses; + +public class Anonymous { + + public static Runnable getAnonymousImpl(){ + return new Runnable() { + @Override + public void run() { + System.err.println("Hello Anonymous!"); + } + }; + } +} diff --git a/src/test/java/com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java b/src/test/java/com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java new file mode 100644 index 00000000..6495e09d --- /dev/null +++ b/src/test/java/com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java @@ -0,0 +1,11 @@ +package com.appland.appmap.output.v1.testclasses; + +import com.appland.appmap.ExampleClass; + +public class ExampleInnerClass extends ExampleClass { + + private static final class StaticFinalInnerClass { + + } + +} From dbde00805b3fbe72fcbd05a8bd3284307489d1f3 Mon Sep 17 00:00:00 2001 From: Dustin Byrne Date: Thu, 18 Mar 2021 13:38:03 -0400 Subject: [PATCH 3/3] prevent null access if no package is defined --- .../appland/appmap/output/v1/CodeObject.java | 64 +++++++++---- .../appmap/output/v1/CodeObjectTest.java | 94 +++++++++++++++---- 2 files changed, 121 insertions(+), 37 deletions(-) diff --git a/src/main/java/com/appland/appmap/output/v1/CodeObject.java b/src/main/java/com/appland/appmap/output/v1/CodeObject.java index 059a0af1..4efe6789 100644 --- a/src/main/java/com/appland/appmap/output/v1/CodeObject.java +++ b/src/main/java/com/appland/appmap/output/v1/CodeObject.java @@ -190,25 +190,39 @@ public CodeObject(CodeObject src) { * @return An estimated source file path */ public static String getSourceFilePath(CtClass classType) { - String sourceFilePath = null; - if (classType.getClassFile2().getAttribute("SourceFile") != null) { - sourceFilePath = getSourceFilePathWithDebugInfo(classType); - } else { - sourceFilePath = getSourceCodePath(classType); + String sourceFilePath = getSourceFilePathWithDebugInfo(classType); + + if (sourceFilePath == null) { + sourceFilePath = guessSourceFilePath(classType); } + return sourceFilePath; } private static String getSourceFilePathWithDebugInfo(CtClass classType) { - return classType.getName().substring(0, classType.getName().lastIndexOf(".")) - .replaceAll("\\.", Matcher.quoteReplacement("/")) - + "/" + ((SourceFileAttribute) classType.getClassFile2() - .getAttribute("SourceFile")).getFileName(); + final String sourceFile = classType.getClassFile2().getSourceFile(); + if (sourceFile == null) { + return null; + } + + final List tokens = new ArrayList<>(); + final String packageName = classType.getPackageName(); + + if (packageName != null) { + for(String token : packageName.split("\\.")) { + tokens.add(token); + } + } + + tokens.add(sourceFile); + return String.join("/", tokens); } - private static String getSourceCodePath(CtClass classType) { - return classType.getName().replaceAll("\\.", Matcher.quoteReplacement("/")) - .replaceAll("\\$\\w+", "") + ".java"; + private static String guessSourceFilePath(CtClass classType) { + return classType + .getName() + .replaceAll("\\.", Matcher.quoteReplacement("/")) + .replaceAll("\\$\\w+", "") + ".java"; } /** @@ -218,7 +232,13 @@ private static String getSourceCodePath(CtClass classType) { * @return The root CodeObject */ public static CodeObject createTree(String packageName) { - String[] packageTokens = packageName.split("\\."); + final List packageTokens = new ArrayList(); + if (packageName != null) { + for (String token : packageName.split("\\.")) { + packageTokens.add(token); + } + } + CodeObject rootObject = null; CodeObject previousObject = null; @@ -250,15 +270,19 @@ public static CodeObject createTree(String packageName) { */ public static CodeObject createTree(CtClass classType) { String packageName = classType.getPackageName(); + CodeObject classObj = new CodeObject(classType); CodeObject rootObject = CodeObject.createTree(packageName); - CodeObject pkgLeafObject = rootObject.get(packageName); - if (pkgLeafObject == null) { - Logger.println("failed to get leaf pkg object for package " + packageName); - return null; - } + if (rootObject == null) { + rootObject = classObj; + } else { + CodeObject pkgLeafObject = rootObject.get(packageName); + if (pkgLeafObject == null) { + Logger.println("failed to get leaf pkg object for package " + packageName); + return null; + } - CodeObject classObj = new CodeObject(classType); - pkgLeafObject.addChild(classObj); + pkgLeafObject.addChild(classObj); + } return rootObject; } diff --git a/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java b/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java index 1c6edbdb..5dcd2eab 100644 --- a/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java +++ b/src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java @@ -1,31 +1,91 @@ package com.appland.appmap.output.v1; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.appland.appmap.output.v1.CodeObject; +import com.appland.appmap.output.v1.Event; +import com.appland.appmap.test.util.ClassBuilder; + import javassist.ClassPool; import javassist.CtClass; import javassist.NotFoundException; -import org.junit.Test; -import static org.junit.Assert.*; +import java.util.List; public class CodeObjectTest { - @Test - public void getSourceFilePath_for_RegularClass() throws NotFoundException { - CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.ExampleClass"); - assertEquals("com/appland/appmap/ExampleClass.java", CodeObject.getSourceFilePath(testCtClass)); - } + /** + * Validate the objects in a CodeObject tree match the given names and types + * @param obj The root object + * @param names The names of the objects, beginning with the root + * @param types The types of the objects, beginning with the root + */ + private void validateCodeObjectTree(CodeObject obj, String[] names, String[] types) { + CodeObject currentObject = obj; + assertTrue(currentObject != null); - @Test - public void getSourceFilePath_for_InnerClass_ResultInBaseClass() throws NotFoundException { - CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.output.v1.testclasses.ExampleInnerClass$StaticFinalInnerClass"); - assertEquals("com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java", CodeObject.getSourceFilePath(testCtClass)); - } + for (int i = 0; currentObject != null; ++i) { + assertEquals(currentObject.name, names[i]); + assertEquals(currentObject.type, types[i]); - @Test - public void getSourceFilePath_for_AnonymousClass_ResultInBaseClass() throws NotFoundException { - CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.output.v1.testclasses.Anonymous$1"); - assertEquals("com/appland/appmap/output/v1/testclasses/Anonymous.java", CodeObject.getSourceFilePath(testCtClass)); + List children = currentObject.getChildren(); + if (children != null && children.size() > 0) { + currentObject = children.get(0); + } else { + currentObject = null; + } } + } + + @Test + public void testGetSourceFilePath() { + CtClass testClass = new ClassBuilder("testGetSourceFilePath1").ctClass(); + assertEquals(CodeObject.getSourceFilePath(testClass), "testGetSourceFilePath1.java"); + + testClass = new ClassBuilder("com.myorg.testGetSourceFilePath2").ctClass(); + assertEquals(CodeObject.getSourceFilePath(testClass), "com/myorg/testGetSourceFilePath2.java"); + + // It shouldn't be possible to hit this case in the wild, but make sure it + // doesn't raise an exception + testClass = new ClassBuilder("").ctClass(); + assertEquals(CodeObject.getSourceFilePath(testClass), ".java"); + } + + @Test + public void testCreateTree() { + CtClass testClass = new ClassBuilder("testCreateTree").ctClass(); + validateCodeObjectTree( + CodeObject.createTree(testClass), + new String[] { "testCreateTree" }, + new String[] { "class" } + ); + + testClass = new ClassBuilder("com.myorg.testCreateTree").ctClass(); + validateCodeObjectTree( + CodeObject.createTree(testClass), + new String[] { "com", "myorg", "testCreateTree" }, + new String[] { "package", "package", "class" } + ); + } + + @Test + public void getSourceFilePathForRegularClass() throws NotFoundException { + CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.ExampleClass"); + assertEquals("com/appland/appmap/ExampleClass.java", CodeObject.getSourceFilePath(testCtClass)); + } + @Test + public void getSourceFilePath_for_InnerClass_ResultInBaseClass() throws NotFoundException { + CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.output.v1.testclasses.ExampleInnerClass$StaticFinalInnerClass"); + assertEquals("com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java", CodeObject.getSourceFilePath(testCtClass)); + } -} \ No newline at end of file + @Test + public void getSourceFilePath_for_AnonymousClass_ResultInBaseClass() throws NotFoundException { + CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.output.v1.testclasses.Anonymous$1"); + assertEquals("com/appland/appmap/output/v1/testclasses/Anonymous.java", CodeObject.getSourceFilePath(testCtClass)); + } +}