@W-10459675@: Part 4 of several. Added Messaging.InboundEmailHandler implementations as sources. #742

jfeingold35 · 2022-07-05T17:50:02Z

This PR does the following:

Adds a new vertex property, InterfaceDefiningTypes, which captures the full defining type of each interface implemented by a class, and a case-safe equivalent for this property.
These properties are populated during the execution of InheritanceEdgeBuilder.
Adds robust test coverage for the new and existing functionality of InheritanceEdgeBuilder.
Adds a case-safe equivalent for the existing property, ReturnType.
Adds a new case-safe traversal method, hasArrayContaining(), which returns vertices whose value of a specified property is an array containing a specified value.
Adds a new TraversalUtil method, traverseImplementationsOf(), which traverses every class that implements a given interface both directly and indirectly (i.e., extends a class that implements it, or implements an interface that extends it), along with robust test coverage for this new method.
Adds the Messaging.InboundEmailResult handleInboundEmail() method on Messaging.InboundEmailHandler implementations (both direct and indirect) as entrypoints for ApexFlsViolationRule.

Change to be added in supplemental PR:

Rename InheritanceEdgeBuilder to something like InheritanceInformationCreator, since it now does more than just building edges.

jfeingold35 · 2022-07-05T17:50:58Z

sfge/src/main/java/com/salesforce/graph/Schema.java

    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";


New property added because the existing INTERFACE_NAMES property isn't guaranteed to contain the full defining types of interfaces, which was making queries very complicated.

jfeingold35 · 2022-07-05T17:54:29Z

sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java

-                    Schema.NAME,
                    Schema.DEFINING_TYPE,
                    Schema.FULL_METHOD_NAME,
+                    Schema.INTERFACE_DEFINING_TYPES,


Case-safe equivalent for new property.

jfeingold35 · 2022-07-05T17:54:45Z

sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java

                    Schema.INTERFACE_NAMES,
                    Schema.METHOD_NAME,
                    Schema.NAME,
+                    Schema.RETURN_TYPE,


The lack of a case-safe return type was a problem.

jfeingold35 · 2022-07-05T17:56:00Z

sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java

            }
        }
+
+        public static GraphTraversal<Object, Object> hasArrayContaining(


This is the opposite of H.hasWithin(). Instead of checking whether a property's value is contained by the provided array, it checks whether a property represents an array containing the provided value.

Would be helpful to have this information as javadoc.

jfeingold35 · 2022-07-05T17:59:40Z

sfge/src/main/java/com/salesforce/graph/build/InheritanceEdgeBuilder.java

-                // 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()) {


Switched to isEmpty check, since the map is always non-null.

jfeingold35 · 2022-07-05T18:11:18Z

sfge/src/test/java/com/salesforce/graph/build/InheritanceEdgeBuilderTest.java

@@ -1,7 +1,19 @@
 package com.salesforce.graph.build;

+import static org.hamcrest.Matchers.hasSize;


Added aggressive test coverage for every scenario I could think of, since there was previously none.

jfeingold35 · 2022-07-05T18:17:23Z

sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java

+                                __.where(
+                                        H.hasArrayContaining(
+                                                NodeType.USER_CLASS,
+                                                Schema.INTERFACE_DEFINING_TYPES,


The new property makes this query significantly less complicated.

jfeingold35 · 2022-07-05T18:17:45Z

sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java

+        // 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<Vertex, Vertex> traversal =


This query is complicated, but I tried to document it as best I could. Please let me know if anything is unclear.

jfeingold35 · 2022-07-05T18:21:20Z

sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java

+        // Also, start with the hasLabel() call, because doing an initial filter along an indexed
+        // field saves us a ton of time.
+        GraphTraversal<Vertex, Vertex> traversal =
+                g.V().hasLabel(NodeType.USER_CLASS, NodeType.USER_INTERFACE)


This hasLabel call isn't technically necessary, but adding it improved the performance of the query substantially.

jfeingold35 · 2022-07-05T18:22:37Z

sfge/src/test/java/com/salesforce/graph/ops/TraversalUtilTest.java

                    + "	}\n"
                    + "}\n";

+    private String[] interfaceTestSourceCodes = {


Added aggressive test coverage for the new traversal, with every test case I could think of.

jfeingold35 · 2022-07-05T18:31:43Z

sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java

+                        // 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));


At this time, we're only checking the arity, instead of checking the types of arguments. We could add that in the future, but that feels like an excessive level of complication unless we know we'll need it. (He said, having already overcomplicated the PR...)

rmohan20

Looks great!

…implementations as sources.

jfeingold35 commented Jul 5, 2022

View reviewed changes

jfeingold35 force-pushed the d/W-10459675-4 branch from 219d8f8 to 212c25b Compare July 5, 2022 18:36

rmohan20 approved these changes Jul 5, 2022

View reviewed changes

jfeingold35 added 3 commits July 6, 2022 10:54

@W-10459675@: Part 4 of several. Added Messaging.InboundEmailHandler …

e8832fb

…implementations as sources.

@W-10459675@: Added documentation for methods.

0ae8880

@W-10459675@: Integrated changes from rebase into PR.

92e2a45

jfeingold35 force-pushed the d/W-10459675-4 branch from 212c25b to 92e2a45 Compare July 6, 2022 16:12

@W-10459675@: Fixed failing tests.

462eca9

jfeingold35 force-pushed the d/W-10459675-4 branch from a58bb49 to 462eca9 Compare July 6, 2022 17:33

jfeingold35 merged commit 36f2c23 into dev-3 Jul 6, 2022

jfeingold35 deleted the d/W-10459675-4 branch September 14, 2022 18:17

		@@ -1,7 +1,19 @@
		package com.salesforce.graph.build;

		import static org.hamcrest.Matchers.hasSize;

@W-10459675@: Part 4 of several. Added Messaging.InboundEmailHandler implementations as sources. #742

@W-10459675@: Part 4 of several. Added Messaging.InboundEmailHandler implementations as sources. #742

Uh oh!

Conversation

jfeingold35 commented Jul 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmohan20 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jfeingold35 commented Jul 5, 2022 •

edited

Loading