diff --git a/sfge/src/main/java/com/salesforce/graph/Schema.java b/sfge/src/main/java/com/salesforce/graph/Schema.java index 26cd02b95..9cdaa9a65 100644 --- a/sfge/src/main/java/com/salesforce/graph/Schema.java +++ b/sfge/src/main/java/com/salesforce/graph/Schema.java @@ -28,6 +28,7 @@ public class Schema { public static final String IDENTIFIER = "Identifier"; public static final String IMPLEMENTATION_OF = "ImplementationOf"; public static final String IMPLEMENTED_BY = "ImplementedBy"; + public static final String INTERFACE_DEFINING_TYPES = "InterfaceDefiningTypes"; public static final String INTERFACE_NAMES = "InterfaceNames"; /** True if this vertex is part of the Apex Standard Library */ public static final String IS_STANDARD = "IsStandard"; diff --git a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java index 2cd405346..ddf737c4b 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/AbstractApexVertexBuilder.java @@ -82,9 +82,11 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) { final Vertex vChild = g.addV(child.getLabel()).next(); addProperties(g, child, vChild); - /** Handle static block if we are looking at a method that has a block statement. - * See {@linkplain StaticBlockUtil} on why this is needed - * and how we handle it. */ + /** + * Handle static block if we are looking at a method that has a block + * statement. See {@linkplain StaticBlockUtil} on why this is needed and how we handle + * it. + */ if (StaticBlockUtil.isStaticBlockStatement(node, child)) { final Vertex parentVertexForChild = StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, i); diff --git a/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java b/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java index e0439ee71..3d068a49c 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java @@ -39,12 +39,13 @@ public class CaseSafePropertyUtil { // The following properties are (with rare exception) assumed to be case-sensitive. private static final ImmutableSet CASE_SENSITIVE_PROPERTIES = ImmutableSet.of( - Schema.NAME, Schema.DEFINING_TYPE, Schema.FULL_METHOD_NAME, + Schema.INTERFACE_DEFINING_TYPES, Schema.INTERFACE_NAMES, Schema.METHOD_NAME, Schema.NAME, + Schema.RETURN_TYPE, Schema.SUPER_CLASS_NAME, Schema.SUPER_INTERFACE_NAME); @@ -80,16 +81,16 @@ private static String toCaseSafeValue(String value) { return value.toLowerCase(Locale.ROOT); } - private static ArrayList toCaseSafeValue(ArrayList value) { + private static Object[] toCaseSafeValue(ArrayList value) { if (value.isEmpty() || !(value.get(0) instanceof String)) { // An empty array or non-string array can't be made case-safe. - return value; + return value.toArray(); } else { ArrayList stringList = new ArrayList<>(); for (Object o : value) { stringList.add(((String) o).toLowerCase(Locale.ROOT)); } - return stringList; + return stringList.toArray(); } } @@ -191,5 +192,19 @@ public static GraphTraversal hasEndingWith( return __.has(nodeType, property, TextP.endingWith(value)); } } + + public static GraphTraversal hasArrayContaining( + String nodeType, String property, String value) { + if (property.equalsIgnoreCase(Schema.DEFINING_TYPE)) { + throw new UnexpectedException(property); + } + String caseSafeVariant = toCaseSafeProperty(property).orElse(null); + if (caseSafeVariant != null) { + return __.hasLabel(nodeType) + .has(caseSafeVariant, __.unfold().is(toCaseSafeValue(value))); + } else { + return __.hasLabel(nodeType).has(property, __.unfold().is(value)); + } + } } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java index 7847dac40..686d48eef 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java +++ b/sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java @@ -10,7 +10,6 @@ import com.salesforce.graph.vertex.InheritableSFVertex; import com.salesforce.graph.vertex.SFVertexFactory; import com.salesforce.graph.vertex.UserClassVertex; -import com.salesforce.graph.vertex.UserInterfaceVertex; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -18,10 +17,11 @@ import java.util.Map; import java.util.Optional; import java.util.TreeMap; -import java.util.stream.Collectors; +import java.util.TreeSet; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.structure.Vertex; @@ -73,7 +73,8 @@ private void buildInheritanceMaps() { private void processVertexInheritance(InheritableSFVertex vertex) { InheritableSFVertex extendedVertex = findExtendedVertex(vertex).orElse(null); - List implementedVertices = findImplementedVertices(vertex); + Map implementedVerticesByName = + findImplementedVerticesByName(vertex); // It's possible for the extendedVertex to be null, if the extended class is declared in a // file that wasn't scanned. @@ -85,17 +86,30 @@ private void processVertexInheritance(InheritableSFVertex vertex) { Schema.EXTENSION_OF); } - if (implementedVertices != null) { - for (InheritableSFVertex implementedVertex : implementedVertices) { - // It's possible for the implementedVertex to be null if the interface being - // implemented is declared in a file - // that wasn't scanned. + if (!implementedVerticesByName.isEmpty()) { + List implementedVertexDefiningTypes = new ArrayList<>(); + Long myId = vertex.getId(); + for (String implementedVertexName : implementedVerticesByName.keySet()) { + InheritableSFVertex implementedVertex = + implementedVerticesByName.get(implementedVertexName); + // If we actually have a vertex for this interface, we need to create edges + // connecting it to the implementor, + // and we can use its defining type for our list. if (implementedVertex != null) { Long implId = implementedVertex.getId(); - Long myId = vertex.getId(); addEdges(implId, myId, Schema.IMPLEMENTED_BY, Schema.IMPLEMENTATION_OF); + implementedVertexDefiningTypes.add(implementedVertex.getDefiningType()); + } else { + // If there's no vertex for this interface, we can assume it's defined in some + // other codebase, and therefore + // whatever name was used by the implementor to reference it can be assumed to + // be the defining type. + implementedVertexDefiningTypes.add(implementedVertexName); } } + // Give the implementor new properties indicating the full names of the implemented + // interfaces. + addProperties(myId, implementedVertexDefiningTypes); } } @@ -108,18 +122,20 @@ private Optional findExtendedVertex(InheritableSFVertex ver } } - private List findImplementedVertices(InheritableSFVertex vertex) { - if (vertex instanceof UserInterfaceVertex) { - return new ArrayList<>(); - } else { + private Map findImplementedVerticesByName( + InheritableSFVertex vertex) { + Map results = new HashMap<>(); + // Only classes can implement interfaces. + if (vertex instanceof UserClassVertex) { String inheritorType = vertex.getDefiningType(); - return ((UserClassVertex) vertex) - .getInterfaceNames().stream() - .map(i -> findInheritedVertex(i, inheritorType)) - .filter(v -> v.isPresent()) - .map(v -> v.get()) - .collect(Collectors.toList()); + List interfaceNames = ((UserClassVertex) vertex).getInterfaceNames(); + for (String interfaceName : interfaceNames) { + InheritableSFVertex inheritedVertex = + findInheritedVertex(interfaceName, inheritorType).orElse(null); + results.put(interfaceName, inheritedVertex); + } } + return results; } private Optional findInheritedVertex( @@ -127,17 +143,19 @@ private Optional findInheritedVertex( if (inheritableType == null) { throw new UnexpectedException("inheritableType can't be null"); } - // If the inheritor type contains a period, it's an inner type. If the inheritable type does - // not contain a period, - // it could be either an outer type, or an inner type declared in the same outer type. The - // latter overrides the - // former, so we'll derive the full name of such an inner type so we can check if it exists. - if (inheritorType.contains(".") && !inheritableType.contains(".")) { - String[] parts = inheritorType.split("\\."); - - String possibleInnerType = parts[0] + "." + inheritableType; - if (verticesByDefiningType.containsKey(possibleInnerType)) { - return Optional.ofNullable(verticesByDefiningType.get(possibleInnerType)); + // If the inheritable type does NOT contain a period, then it's either an outer type, or an + // inner type declared + // in the same outer type as the inheritor type. + // The latter takes priority over the former, so we should check if it's the case. + if (!inheritableType.contains(".")) { + // If the inheritor type contains a period, then it's an inner type, and we should use + // its outer type. + // Otherwise, we should use the inheritor type as a potential outer type. + String potentialOuterType = + inheritorType.contains(".") ? inheritorType.split("\\.")[0] : inheritorType; + String potentialInnerType = potentialOuterType + "." + inheritableType; + if (verticesByDefiningType.containsKey(potentialInnerType)) { + return Optional.ofNullable(verticesByDefiningType.get(potentialInnerType)); } } @@ -163,4 +181,19 @@ private void addEdges(Long inheritableId, Long inheritorId, String fromEdge, Str g.addE(fromEdge).from(inheritable).to(inheritor).iterate(); g.addE(toEdge).from(inheritor).to(inheritable).iterate(); } + + private void addProperties(Long inheritorId, List implementedVertexDefiningTypes) { + GraphTraversal traversal = g.V(inheritorId); + // It's probably not best practice to be using an empty treeset here, but we no longer have + // access to the treeset + // that was used when adding the base properties, and we're setting a property that + // definitely isn't set elsewhere, + // so it should be fine. + GremlinVertexUtil.addProperty( + new TreeSet<>(), + traversal, + Schema.INTERFACE_DEFINING_TYPES, + implementedVertexDefiningTypes); + traversal.iterate(); + } } diff --git a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java index 3a2a8756f..339cce69f 100644 --- a/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/build/StaticBlockUtil.java @@ -19,9 +19,7 @@ /** * Handles creation of synthetic methods and vertices to gracefully invoke static code blocks. * - *

Consider this example: - * - * + *

Consider this example: * class StaticBlockClass { * static { * System.debug("inside static block 1"); @@ -30,10 +28,7 @@ * System.debug("inside static block 2"); * } * } - * - * - * In Jorje's compilation structure, static blocks are represented like this: - * + * In Jorje's compilation structure, static blocks are represented like this: * class StaticBlockClass { * private static void () { * { @@ -46,17 +41,17 @@ * } * * - *

Having multiple block statements inside a method breaks SFGE's normal code flow logic. - * This makes handling code blocks in () impossible. - *

As an alternative, we are creating synthetic vertices in the Graph to - * represent the static blocks as individual methods. - *

We also create one top-level synthetic method ("StaticBlockInvoker") that invokes - * individual static block methods. While creating static scope - * for this class, we should invoke the method call expressions inside the top-level synthetic - * method. + *

Having multiple block statements inside a method breaks SFGE's normal code flow logic. This + * makes handling code blocks in () impossible. + * + *

As an alternative, we are creating synthetic vertices in the Graph to represent the static + * blocks as individual methods. + * + *

We also create one top-level synthetic method ("StaticBlockInvoker") that invokes individual + * static block methods. While creating static scope for this class, we should invoke the method + * call expressions inside the top-level synthetic method. * - *

New structure looks like this: - * + *

New structure looks like this: * class StaticBlockClass { * private static void SyntheticStaticBlock_1() { * System.debug("inside static block 1"); @@ -90,12 +85,11 @@ private StaticBlockUtil() {} * parent for blockStatementVertex, and a sibling of () */ public static Vertex createSyntheticStaticBlockMethod( - GraphTraversalSource g, - Vertex clinitVertex, - int staticBlockIndex) { + GraphTraversalSource g, Vertex clinitVertex, int staticBlockIndex) { final Vertex syntheticMethodVertex = g.addV(ASTConstants.NodeType.METHOD).next(); final String definingType = clinitVertex.value(Schema.DEFINING_TYPE); - final List siblings = GremlinUtil.getChildren(g, GremlinVertexUtil.getParentVertex(g, clinitVertex)); + final List siblings = + GremlinUtil.getChildren(g, GremlinVertexUtil.getParentVertex(g, clinitVertex)); final int nextSiblingIndex = siblings.size(); addSyntheticStaticBlockMethodProperties( @@ -286,7 +280,10 @@ private static void addStaticModifierProperties( } private static void addStaticBlockInvokerProperties( - GraphTraversalSource g, String definingType, Vertex staticBlockInvokerVertex, int childIndex) { + GraphTraversalSource g, + String definingType, + Vertex staticBlockInvokerVertex, + int childIndex) { verifyType(staticBlockInvokerVertex, ASTConstants.NodeType.METHOD); final Map properties = new HashMap<>(); properties.put(Schema.NAME, STATIC_BLOCK_INVOKER_METHOD); @@ -296,11 +293,11 @@ private static void addStaticBlockInvokerProperties( } private static void addSyntheticStaticBlockMethodProperties( - GraphTraversalSource g, - String definingType, - Vertex syntheticMethodVertex, - int staticBlockIndex, - int childIndex) { + GraphTraversalSource g, + String definingType, + Vertex syntheticMethodVertex, + int staticBlockIndex, + int childIndex) { verifyType(syntheticMethodVertex, ASTConstants.NodeType.METHOD); final Map properties = new HashMap<>(); properties.put( diff --git a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java index 9632bb1d6..acc1e7fc9 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java @@ -81,6 +81,9 @@ public final class MethodUtil { private static final Logger LOGGER = LogManager.getLogger(MethodUtil.class); private static final String PAGE_REFERENCE = "PageReference"; + private static final String INBOUND_EMAIL_HANDLER = "Messaging.InboundEmailHandler"; + private static final String HANDLE_INBOUND_EMAIL = "handleInboundEmail"; + private static final String INBOUND_EMAIL_RESULT = "Messaging.InboundEmailResult"; public static final String INSTANCE_CONSTRUCTOR_CANONICAL_NAME = ""; public static final String STATIC_CONSTRUCTOR_CANONICAL_NAME = ""; @@ -257,6 +260,21 @@ public static List getGlobalMethods( __.not(__.has(Schema.IS_STANDARD, true))))); } + public static List getInboundEmailHandlerMethods( + GraphTraversalSource g, List targetFiles) { + return SFVertexFactory.loadVertices( + g, + // Get any target class that implements the email handler interface. + TraversalUtil.traverseImplementationsOf(g, targetFiles, INBOUND_EMAIL_HANDLER) + // Get every implementation of the handle email method. + .out(Schema.CHILD) + .where(H.has(NodeType.METHOD, Schema.NAME, HANDLE_INBOUND_EMAIL)) + // Filter the results by return type and arity to limit the possibility of + // getting unnecessary results. + .where(H.has(NodeType.METHOD, Schema.RETURN_TYPE, INBOUND_EMAIL_RESULT)) + .has(Schema.ARITY, 2)); + } + /** * Returns all non-test public- and global-scoped methods in controllers referenced by * VisualForce pages, filtered by target file list. An empty list implicitly includes all files. diff --git a/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java index bdc0e831b..8c98aa4fa 100644 --- a/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java +++ b/sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java @@ -1,7 +1,9 @@ package com.salesforce.graph.ops; import com.salesforce.apex.jorje.ASTConstants; +import com.salesforce.apex.jorje.ASTConstants.NodeType; import com.salesforce.graph.Schema; +import com.salesforce.graph.build.CaseSafePropertyUtil.H; import com.salesforce.rules.AbstractRuleRunner.RuleRunnerTarget; import java.util.List; import java.util.stream.Collectors; @@ -68,5 +70,70 @@ public static GraphTraversal ruleTargetTraversal( .toArray(GraphTraversal[]::new)); } + /** + * Returns a traversal containing every class in the target files that implements the specified + * interface, either directly or indirectly (i.e., by extending a class that implements it, + * implementing an interface that extends it, or extending a class that does either of those + * things). An empty target array implicitly targets the whole graph. + */ + public static GraphTraversal traverseImplementationsOf( + GraphTraversalSource g, List targetFiles, String interfaceName) { + // For our traversal, we want to start with every class that implements either the target + // interface or one of its + // subtypes. Do that with a union of these two subtraversals. + // Also, start with the hasLabel() call, because doing an initial filter along an indexed + // field saves us a ton of time. + GraphTraversal traversal = + g.V().hasLabel(NodeType.USER_CLASS, NodeType.USER_INTERFACE) + .union( + // Subtraversal 1: Get every class that implements the target + // interface, via a helper method. + __.where( + H.hasArrayContaining( + NodeType.USER_CLASS, + Schema.INTERFACE_DEFINING_TYPES, + interfaceName)), + // Subtraversal 2: Get every class that implements a subtype of the + // target interface, by starting with all + // the direct subtypes of the interface... + __.where( + H.has( + NodeType.USER_INTERFACE, + Schema.SUPER_INTERFACE_NAME, + interfaceName)) + .union( + __.identity(), + // ...recursively adding subtypes of those + // interfaces... + __.repeat(__.out(Schema.EXTENDED_BY)).emit()) + // ... and getting every class that implements one of those + // interfaces. + .out(Schema.IMPLEMENTED_BY) + // Now, we add every subclass of any of these classes, recursively. + ) + .union(__.identity(), __.repeat(__.out(Schema.EXTENDED_BY)).emit()); + + // If there are no target files, we're clear to return this traversal since it includes all + // relevant classes in + // the graph. + if (targetFiles.isEmpty()) { + return traversal; + } else { + // Otherwise, we need to filter so we're only returning the classes in a target file. To + // do that, do a traversal + // to get all the IDs of classes defined in the target files. + Object[] targetIds = + fileRootTraversal(g, targetFiles) + .union(__.identity(), __.repeat(__.out(Schema.CHILD)).emit()) + .hasLabel(NodeType.USER_CLASS) + .id() + .toList() + .toArray(); + // Note, there's no .hasId(Object...) overload, so we're using the .hasId(Object, + // ...Object) overload instead. + return traversal.hasId(targetIds); + } + } + private TraversalUtil() {} } diff --git a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java index 5e088f338..f5a4858c5 100644 --- a/sfge/src/main/java/com/salesforce/rules/RuleUtil.java +++ b/sfge/src/main/java/com/salesforce/rules/RuleUtil.java @@ -67,6 +67,8 @@ public static List getPathEntryPoints( methods.addAll(MethodUtil.getPageReferenceMethods(g, fileLevelTargets)); // ...and global-exposed methods... methods.addAll(MethodUtil.getGlobalMethods(g, fileLevelTargets)); + // ...and implementations of Messaging.InboundEmailHandler#handleInboundEmail... + methods.addAll(MethodUtil.getInboundEmailHandlerMethods(g, fileLevelTargets)); // ...and exposed methods on VF controllers. methods.addAll(MethodUtil.getExposedControllerMethods(g, fileLevelTargets)); } diff --git a/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java index 5c63f768a..8716abf0e 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/CustomerApexVertexBuilderTest.java @@ -117,11 +117,10 @@ public void testCaseSafeProperties() { Schema.SUPER_CLASS_NAME + CaseSafePropertyUtil.CASE_SAFE_SUFFIX)); assertEquals( "someinterface", - ((ArrayList) + ((Object[]) userClassVertex.get( Schema.INTERFACE_NAMES - + CaseSafePropertyUtil.CASE_SAFE_SUFFIX)) - .get(0)); + + CaseSafePropertyUtil.CASE_SAFE_SUFFIX))[0]); Map userInterfaceVertex = g.V().hasLabel(NodeType.USER_INTERFACE).elementMap().next(); diff --git a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java index 2f9bcc137..bceff013f 100644 --- a/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java +++ b/sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java @@ -1,7 +1,20 @@ package com.salesforce.graph.build; +import static org.hamcrest.Matchers.arrayContaining; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.core.IsEqual.equalTo; + import com.salesforce.TestUtil; +import com.salesforce.apex.jorje.ASTConstants.NodeType; +import com.salesforce.graph.Schema; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.apache.tinkerpop.gremlin.process.traversal.Order; +import org.apache.tinkerpop.gremlin.process.traversal.Scope; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -14,22 +27,319 @@ public void setup() { } @Test - public void testSimpleExtension() { - // TODO + public void testNoInheritance() { + String[] sourceCodes = {"public class c1 {}", "public interface i1 {}"}; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that nothing has any inheritance edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testSimpleClassExtension() { + String[] sourceCodes = { + "public class c1 {}", "public class c2 extends c1{}", + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the extension looks right. + verifyExtension(g, "c1", "c2"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); } @Test - public void testInnerTypeExtension() { - // TODO + public void testSimpleInterfaceExtension() { + String[] sourceCodes = { + "public interface i1 {}", "public interface i2 extends i1{}", + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the extension looks right. + verifyExtension(g, "i1", "i2"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); } @Test public void testSimpleImplementation() { - // TODO + String[] sourceCodes = { + "public interface i1 {}", "public class c1 implements i1{}", + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the implementations look right. + verifyImplementation(g, "i1", "c1"); + + // Verify that there are no extension edges. + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testImplementationOfExternalInterface() { + String[] sourceCodes = {"public class c1 implements ExternallyDeclaredInterface {}"}; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right properties were set. + verifyInterfaceDefiningTypeProperty( + g, "ExternallyDeclaredInterface", Collections.singletonList("c1")); + // Verify that no edges were created. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testExtensionOfInnerClass() { + String[] sourceCodes = { + // Extensions in this class don't use the outer class name. + "public class OuterClass1 extends InnerClass1 {\n" + + " public class InnerClass1 {}\n" + + " public class InnerClass2 extends InnerClass1 {}\n" + + "}", + // Extensions in this class use the outer class name. + "public class OuterClass2 extends OuterClass2.InnerClass1 {\n" + + " public class InnerClass1 {}\n" + + " public class InnerClass2 extends InnerClass1 {}\n" + + "}", + // Third class extends one of the inner classes in another class. + "public class OuterClass3 extends OuterClass2.InnerClass1 {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Make sure OuterClass1.InnerClass1 is extended by the right classes. + verifyExtension( + g, + "OuterClass1.InnerClass1", + Arrays.asList("OuterClass1", "OuterClass1.InnerClass2")); + + // Make sure OuterClass2.InnerClass1 is extended by the right classes. + verifyExtension( + g, + "OuterClass2.InnerClass1", + Arrays.asList("OuterClass2", "OuterClass2.InnerClass2", "OuterClass3")); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + } + + @Test + public void testImplementationOfInnerInterface() { + String[] sourceCodes = { + // Implementations in this class don't use the outer class name. + "public class OuterClass1 implements InnerInterface1 {\n" + + " public class InnerInterface1 {}\n" + + " public class InnerClass1 implements InnerInterface1 {}\n" + + "}", + // Implementations in this class use the outer class name. + "public class OuterClass2 implements OuterClass2.InnerInterface1 {\n" + + " public class InnerInterface1 {}\n" + + " public class InnerClass1 implements InnerInterface1 {}\n" + + "}", + // Third class extends one of the inner classes in another class. + "public class OuterClass3 implements OuterClass2.InnerInterface1 {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Make sure OuterClass1.InnerInterface1 is implemented by the right classes. + verifyImplementation( + g, + "OuterClass1.InnerInterface1", + Arrays.asList("OuterClass1", "OuterClass1.InnerClass1")); + + // Make sure OuterClass2.InnerInterface1 is implemented by the right classes. + verifyImplementation( + g, + "OuterClass2.InnerInterface1", + Arrays.asList("OuterClass2", "OuterClass2.InnerClass1", "OuterClass3")); + + // Verify that there are no extension edges. + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + @Test + public void testExtensionOfOwnOuterClass() { + String[] sourceCodes = { + "public class OuterClass1 {\n" + + " public class InnerClass1 extends OuterClass1 {}\n" + + "}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right extension edges exist. + verifyExtension(g, "OuterClass1", "OuterClass1.InnerClass1"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); } @Test - public void testInnerTypeImplementation() { - // TODO + public void testClassNameOverlap() { + String[] sourceCodes = { + // This class extends its own inner class without using the outer class name. + "public class OuterClass1 extends NameOverlappedClass {\n" + + " public class NameOverlappedClass {}\n" + + "}", + // This class has the same name as the inner class the other class extends. + "public class NameOverlappedClass {}", + // This class extends the outer class with the conflict name. + "public class OuterClass2 extends NameOverlappedClass {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right extension edges exist. + verifyExtension(g, "NameOverlappedClass", "OuterClass2"); + verifyExtension(g, "OuterClass1.NameOverlappedClass", "OuterClass1"); + + // Verify that there are no implementation edges. + verifyNoEdges(g, Schema.IMPLEMENTED_BY); + verifyNoEdges(g, Schema.IMPLEMENTATION_OF); + } + + @Test + public void testInterfaceNameOverlap() { + String[] sourceCodes = { + // This class implements its own inner interface without using the outer class name. + "public class OuterClass1 implements NameOverlappedInterface {\n" + + " public class NameOverlappedInterface {}\n" + + "}", + // This interface has the same name as the inner interface the other class extends. + "public interface NameOverlappedInterface {}", + // This class implements the outer class with the conflict name. + "public class OuterClass2 implements NameOverlappedInterface {}" + }; + + // Build the graph. + // TODO: Ideally, this would call the edge builder directly against an incomplete graph. + TestUtil.buildGraph(g, sourceCodes); + + // Verify that the right implementation edges exist. + verifyImplementation(g, "NameOverlappedInterface", "OuterClass2"); + verifyImplementation(g, "OuterClass1.NameOverlappedInterface", "OuterClass1"); + + // Verify that there are no extension edges. + verifyNoEdges(g, Schema.EXTENDED_BY); + verifyNoEdges(g, Schema.EXTENSION_OF); + } + + private void verifyNoEdges(GraphTraversalSource g, String edgeName) { + List vertices = g.V().out(edgeName).toList(); + MatcherAssert.assertThat(vertices, hasSize(equalTo(0))); + } + + private void verifyExtension( + GraphTraversalSource g, String parentDefiningType, String childDefiningType) { + verifyExtension(g, parentDefiningType, Collections.singletonList(childDefiningType)); + } + + private void verifyExtension( + GraphTraversalSource g, String parentDefiningType, List childDefiningTypes) { + verifyEdges( + g, Schema.EXTENDED_BY, Schema.EXTENSION_OF, parentDefiningType, childDefiningTypes); + } + + private void verifyImplementation( + GraphTraversalSource g, String parentDefiningType, String childDefiningType) { + verifyImplementation(g, parentDefiningType, Collections.singletonList(childDefiningType)); + } + + private void verifyImplementation( + GraphTraversalSource g, String parentDefiningType, List childDefiningTypes) { + verifyEdges( + g, + Schema.IMPLEMENTED_BY, + Schema.IMPLEMENTATION_OF, + parentDefiningType, + childDefiningTypes); + verifyInterfaceDefiningTypeProperty(g, parentDefiningType, childDefiningTypes); + } + + private void verifyEdges( + GraphTraversalSource g, + String outgoingEdge, + String incomingEdge, + String parentDefiningType, + List childDefiningTypes) { + List outgoingVertices = + g.V().has(Schema.DEFINING_TYPE, parentDefiningType) + .out(outgoingEdge) + .values(Schema.DEFINING_TYPE) + .order(Scope.global) + .by(Order.asc) + .toList(); + List incomingVertices = + g.V().has(Schema.DEFINING_TYPE, parentDefiningType) + .in(incomingEdge) + .values(Schema.DEFINING_TYPE) + .order(Scope.global) + .by(Order.asc) + .toList(); + + // Both lists should have the same vertices in the same order. + MatcherAssert.assertThat(outgoingVertices, hasSize(equalTo(childDefiningTypes.size()))); + MatcherAssert.assertThat(incomingVertices, hasSize(equalTo(childDefiningTypes.size()))); + for (int i = 0; i < childDefiningTypes.size(); i++) { + MatcherAssert.assertThat( + outgoingVertices.get(i).toString(), equalTo(childDefiningTypes.get(i))); + MatcherAssert.assertThat( + incomingVertices.get(i).toString(), equalTo(childDefiningTypes.get(i))); + } + } + + private void verifyInterfaceDefiningTypeProperty( + GraphTraversalSource g, String parentDefiningType, List childDefiningTypes) { + // For each child type, get the property on that vertex. + List propValues = + g.V().where( + CaseSafePropertyUtil.H.hasWithin( + NodeType.USER_CLASS, + Schema.DEFINING_TYPE, + childDefiningTypes)) + .values(Schema.INTERFACE_DEFINING_TYPES) + .toList(); + for (Object propValue : propValues) { + MatcherAssert.assertThat( + (Object[])propValue, + arrayContaining(parentDefiningType)); + } } } diff --git a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java index 52336ec4f..8997082ea 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/MethodUtilTest.java @@ -428,4 +428,34 @@ public void testGetGlobalMethods() { MatcherAssert.assertThat(excludedName, excludedMethod.isTest(), equalTo(true)); } } + + @Test + public void testGetInboundEmailHandlerMethods() { + String[] sourceCode = { + "public class MyClass implements Messaging.InboundEmailHandler {\n" + + " public Messaging.InboundEmailResult handleInboundEmail(Messaging.InboundEmail email, Messaging.InboundEnvelope envelope) {\n" + + " return null;\n" + + " }\n" + + " public Messaging.InboundEmailHandler someSecondaryMethod() {\n" + + " return null;\n" + + " }\n" + + "}\n", + "public class MyClass2 {\n" + + " public Messaging.InboundEmailResult handleInboundEmail(Messaging.InboundEmail email, Messaging.InboundEnvelope envelope) {\n" + + " return null;\n" + + " }\n" + + " public Messaging.InboundEmailHandler someSecondaryMethod() {\n" + + " return null;\n" + + " }\n" + + "}\n" + }; + TestUtil.buildGraph(g, sourceCode); + + List methods = MethodUtil.getInboundEmailHandlerMethods(g, new ArrayList<>()); + // The `MyClass#handleInboundEmail` method should be included because it's an implementation + // of the desired interface. + MatcherAssert.assertThat(methods, hasSize(equalTo(1))); + MatcherAssert.assertThat(methods.get(0).getName(), equalTo("handleInboundEmail")); + MatcherAssert.assertThat(methods.get(0).getDefiningType(), equalTo("MyClass")); + } } diff --git a/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java b/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java index 434bb67ce..35e946355 100644 --- a/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java +++ b/sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java @@ -2,6 +2,7 @@ import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasSize; +import static org.junit.jupiter.api.Assertions.assertTrue; import com.salesforce.TestUtil; import com.salesforce.apex.jorje.ASTConstants; @@ -11,6 +12,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; @@ -18,6 +21,8 @@ import org.hamcrest.MatcherAssert; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; public class TraversalUtilTest { private GraphTraversalSource g; @@ -63,6 +68,47 @@ public class TraversalUtilTest { + " }\n" + "}\n"; + private String[] interfaceTestSourceCodes = { + // An interface + "public interface MyInterface1 {}", + // An interface that extends another interface. + "public interface MyInterface2 extends MyInterface1 {}", + // A class that implements the first interface. + "public class MyClass1 implements MyInterface1 {}", + // A class that extends a class that implements an interface. + "public class MyClass2 extends MyClass1 {}", + // A class that implements an interface that extends another interface. + "public class MyClass3 implements MyInterface2 {}", + // A class that implements its own inner interface without using the outer class syntax. + "public class MyClass4 implements InnerInterface1 {\n" + + " public interface InnerInterface1 {}\n" + // A class that implements an inner interface in the same outer class, without using + // the outer class syntax. + + " public class InnerClass1 implements InnerInterface1 {}\n" + // A class that implements an inner interface in the same outer class, using the + // outer class syntax. + + " public class InnerClass2 implements MyClass4.InnerInterface1 {}\n" + // A class that does nothing in particular. + + " public class InnerClass3 {}\n" + + "}", + // A class that implements its own inner interface using the outer class syntax. + "public class MyClass5 implements MyClass5.InnerInterface1 {\n" + + " public interface InnerInterface1 {}\n" + + "}", + // A class that implements another class's inner interface. + "public class MyClass6 implements MyClass4.InnerInterface1 {}", + // An interface whose name will be shared by an inner interface in another class. + "public interface SharedInterfaceName1 {}", + // A class implementing an inner interface whose name is shared by another outer interface. + "public class MyClass7 implements SharedInterfaceName1 {\n" + + " public interface SharedInterfaceName1 {}\n" + + "}", + // A class implementing the outer interface with the duplicate name. + "public class MyClass8 implements SharedInterfaceName1 {}", + // A class that does nothing in particular. + "public class MyClass9 {}" + }; + @BeforeEach public void setup() { this.g = TestUtil.getGraph(); @@ -213,4 +259,53 @@ public void ruleTargetTraversal_MethodAndFile() { MatcherAssert.assertThat( targetResults, hasSize(expectedWholeFile.size() + expectedSingleMethod.size())); } + + @ParameterizedTest(name = "{displayName}: {0}") + @CsvSource({ + // MyInterface1 is directly implemented by MyClass1, and indirectly implemented by MyClass2 + // and MyClass3. + "MyInterface1, 'MyClass1,MyClass2,MyClass3'", + // MyInterface2 is directly implemented by MyClass3 + "MyInterface2, 'MyClass3'", + // MyClass4.InnerInterface1 is directly implemented by MyClass4, two of its inner classes, + // and MyClass6. + "MyClass4.InnerInterface1, 'MyClass4,MyClass4.InnerClass1,MyClass4.InnerClass2,MyClass6'", + // MyClass5.InnerInterface1 is implemented only by its outer class, MyClass5. + "MyClass5.InnerInterface1, 'MyClass5'", + // SharedInterfaceName1 is implemented by MyClass8. + "SharedInterfaceName1, 'MyClass8'", + // MyClass7.SharedInterfaceName1 is implemented by MyClass7. + "MyClass7.SharedInterfaceName1, 'MyClass7'" + }) + public void traverseImplementationsOf_testWithoutTargetFiles( + String interfaceName, String expectedClassNameString) { + String[] expectedClassNames = expectedClassNameString.split(","); + TestUtil.buildGraph(g, interfaceTestSourceCodes); + + // RUN THE TESTED METHOD. + List traversedVertices = + TraversalUtil.traverseImplementationsOf(g, new ArrayList<>(), interfaceName) + // For testing purposes, get the name of each vertex instead of returning + // the whole vertex. + .values(Schema.DEFINING_TYPE) + .toList(); + // Turn the object list into a string set for comparison purposes. + Set traversedVertexNames = + traversedVertices.stream().map(Object::toString).collect(Collectors.toSet()); + + // Create an array of booleans indicating whether we returned each expected class. + boolean[] expectedClassPresent = new boolean[expectedClassNames.length]; + Arrays.fill(expectedClassPresent, false); + + // Iterate over the expected classes and set each class's boolean based on whether it's in + // the name set. + for (int i = 0; i < expectedClassPresent.length; i++) { + expectedClassPresent[i] = traversedVertexNames.contains(expectedClassNames[i]); + } + + // If any of the booleans are still false, the test fails. + for (int i = 0; i < expectedClassPresent.length; i++) { + assertTrue(expectedClassPresent[i], "Class " + expectedClassNames[i] + " is absent"); + } + } } diff --git a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java index e55890da4..1e1e03d60 100644 --- a/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java +++ b/sfge/src/test/java/com/salesforce/rules/RuleUtilTest.java @@ -65,14 +65,14 @@ public void getPathEntryPoints_includesAnnotatedMethods(String annotation) { @Test public void getPathEntryPoints_includesGlobalMethods() { String sourceCode = - "public class Foo {\n" - + " global static void globalStaticMethod() {\n" - + " }\n" - + " global void globalInstanceMethod() {\n" - + " }\n" - + " public static void publicStaticMethod() {\n" - + " }\n" - + "}\n"; + "public class Foo {\n" + + " global static void globalStaticMethod() {\n" + + " }\n" + + " global void globalInstanceMethod() {\n" + + " }\n" + + " public static void publicStaticMethod() {\n" + + " }\n" + + "}\n"; TestUtil.buildGraph(g, sourceCode, true); List entryPoints = RuleUtil.getPathEntryPoints(g); @@ -117,6 +117,26 @@ public void getPathEntryPoints_includesPageReferenceMethods() { assertEquals("pageRefMethod", firstVertex.getName()); } + @Test + public void getPathEntryPoints_includesInboundEmailHandlerMethods() { + String sourceCode = + "public class MyClass implements Messaging.InboundEmailHandler {\n" + + " public Messaging.InboundEmailResult handleInboundEmail(Messaging.InboundEmail email, Messaging.InboundEnvelope envelope) {\n" + + " return null;\n" + + " }\n" + + " public Messaging.InboundEmailHandler someSecondaryMethod() {\n" + + " return null;\n" + + " }\n" + + "}\n"; + TestUtil.buildGraph(g, sourceCode, true); + + List entryPoints = RuleUtil.getPathEntryPoints(g); + + MatcherAssert.assertThat(entryPoints, hasSize(equalTo(1))); + MethodVertex firstVertex = entryPoints.get(0); + assertEquals("handleInboundEmail", firstVertex.getName()); + } + @Test public void getPathEntryPoints_includesExposedControllerMethods() { try {