Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions sfge/src/main/java/com/salesforce/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.salesforce.cli.OutputFormatter;
import com.salesforce.exception.SfgeException;
import com.salesforce.exception.SfgeRuntimeException;
import com.salesforce.exception.UnexpectedException;
import com.salesforce.graph.ops.GraphUtil;
import com.salesforce.messaging.CliMessager;
import com.salesforce.metainfo.MetaInfoCollector;
Expand Down Expand Up @@ -129,6 +130,11 @@ private int execute(String... args) {
LOGGER.error("Error while loading graph", ex);
System.err.println(formatError(ex));
return INTERNAL_ERROR;
} catch (UnexpectedException ex) {
LOGGER.error("Unexpected exception while loading graph", ex);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New catch-all for Unexpected exceptions so that the error message is less cryptic.

System.err.println(
"Unexpected exception while loading graph. See logs for more information.");
return INTERNAL_ERROR;
}

// Run all of the rules.
Expand Down
5 changes: 4 additions & 1 deletion sfge/src/main/java/com/salesforce/graph/ApexPath.java
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,10 @@ public void addVertices(List<BaseSFVertex> vertices) {
!(vertices.get(0) instanceof BlockStatementVertex)
&&
// Class Instantiation Path
!(vertices.get(0) instanceof FieldVertex)) {
!(vertices.get(0) instanceof FieldVertex)
&&
// Static blocks
!(vertices.get(0) instanceof MethodCallExpressionVertex)) {
throw new UnexpectedException(vertices);
}
this.vertices.addAll(vertices);
Expand Down
12 changes: 12 additions & 0 deletions sfge/src/main/java/com/salesforce/graph/Schema.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,16 @@ public static final class JorjeNodeType {
public static final String CHILD = "Child";
public static final String PARENT = "Parent";
public static final String NEXT_SIBLING = "NextSibling";

/** Mark a vertex as synthetic */
public static final String IS_SYNTHETIC = "IsSynthetic";
/** Indicates if a method is a synthetic static block method */
public static final String IS_STATIC_BLOCK_METHOD = "IsStaticBlockMethod";
/** Indicates if a method is a synthetic static block invoker method */
public static final String IS_STATIC_BLOCK_INVOKER_METHOD = "IsStaticBlockInvokerMethod";
/**
* Indicates if a MethodCallExpression is a synthetic invocation of static block from invoker
* method
*/
public static final String IS_STATIC_BLOCK_INVOCATION = "IsStaticBlockInvocation";
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.salesforce.graph.Schema;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -72,23 +73,39 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) {
}

Vertex vPreviousSibling = null;
for (JorjeNode child : node.getChildren()) {
Vertex vChild = g.addV(child.getLabel()).next();
final List<JorjeNode> children = node.getChildren();
final Set<Vertex> verticesAddressed = new HashSet<>();
verticesAddressed.add(vNode);

for (int i = 0; i < children.size(); i++) {
final JorjeNode child = children.get(i);
final Vertex vChild = g.addV(child.getLabel()).next();
addProperties(g, child, vChild);
// We are currently adding PARENT and CHILD, in theory the same edge could be navigated
// both ways, however
// the code looks messy when that is done.
// TODO: Determine if this causes performance or resource issues. Consider a single edge
g.addE(Schema.PARENT).from(vChild).to(vNode).iterate();
g.addE(Schema.CHILD).from(vNode).to(vChild).iterate();

/** Handle static block if we are looking at a <clinit> 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 =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like what's happening is that we detect that the child is a static block, and if so we create our synthetic method block to hold it, make that a child of the current parent, and make the block statement a child of that. Am I reading that right?
I'd love some commenting about what is being done here, and why it was necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added more comments. Let me know if it explains sufficiently or I'll try expanding it some more.

StaticBlockUtil.createSyntheticStaticBlockMethod(g, vNode, i);
GremlinVertexUtil.addParentChildRelationship(g, parentVertexForChild, vChild);
verticesAddressed.add(parentVertexForChild);
} else {
GremlinVertexUtil.addParentChildRelationship(g, vNode, vChild);
}

if (vPreviousSibling != null) {
g.addE(Schema.NEXT_SIBLING).from(vPreviousSibling).to(vChild).iterate();
}
vPreviousSibling = vChild;

// To save memory in the graph, don't pass the source name into recursive calls.
buildVertices(child, vChild, null);
}
afterInsert(g, node, vNode);
// Execute afterInsert() on each vertex we addressed
for (Vertex vertex : verticesAddressed) {
afterInsert(g, node, vertex);
}
if (rootVNode != null) {
// Only call this for the root node
afterFileInsert(g, rootVNode);
Expand All @@ -101,6 +118,8 @@ private void buildVertices(JorjeNode node, Vertex vNodeParam, String fileName) {
*/
private final void afterInsert(GraphTraversalSource g, JorjeNode node, Vertex vNode) {
if (node.getLabel().equals(ASTConstants.NodeType.METHOD)) {
// If we just added a method, create forward and
// backward code flow for the contents of the method
MethodPathBuilderVisitor.apply(g, vNode);
}
}
Expand All @@ -111,7 +130,9 @@ private final void afterInsert(GraphTraversalSource g, JorjeNode node, Vertex vN
* @param vNode root node that corresponds to the file
*/
protected void afterFileInsert(GraphTraversalSource g, Vertex vNode) {
// Intentionally left blank
// If the root (class/trigger/etc) contained any static blocks,
// create an invoker method to invoke the static blocks
StaticBlockUtil.createSyntheticStaticBlockInvocation(g, vNode);
}

protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNode) {
Expand All @@ -122,11 +143,12 @@ protected void addProperties(GraphTraversalSource g, JorjeNode node, Vertex vNod
String key = entry.getKey();
Object value = entry.getValue();
value = adjustPropertyValue(node, key, value);
addProperty(previouslyInsertedKeys, traversal, key, value);
GremlinVertexUtil.addProperty(previouslyInsertedKeys, traversal, key, value);
}

for (Map.Entry<String, Object> entry : getAdditionalProperties(node).entrySet()) {
addProperty(previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue());
GremlinVertexUtil.addProperty(
previouslyInsertedKeys, traversal, entry.getKey(), entry.getValue());
}

// Commit the changes.
Expand All @@ -142,49 +164,4 @@ protected Object adjustPropertyValue(JorjeNode node, String key, Object value) {
protected Map<String, Object> getAdditionalProperties(JorjeNode node) {
return new HashMap<>();
}

/** Add a property to the traversal, throwing an exception if any keys are duplicated. */
protected void addProperty(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted to GremlinVertexUtil for reuse.

TreeSet<String> previouslyInsertedKeys,
GraphTraversal<Vertex, Vertex> traversal,
String keyParam,
Object value) {
final String key = keyParam.intern();

if (!previouslyInsertedKeys.add(key)) {
throw new UnexpectedException(key);
}

if (value instanceof List) {
List list = (List) value;
// Convert ArrayList to an Array. There seems to be a Tinkerpop bug where a singleton
// ArrayList
// isn't properly stored. Remote graphs also have issues with empty lists, so don't add
// an empty list.
if (!list.isEmpty()) {
traversal.property(key, list.toArray());
} else {
// return so that we don't store a case insensitive version
return;
}
} else if (value instanceof Boolean
|| value instanceof Double
|| value instanceof Integer
|| value instanceof Long
|| value instanceof String) {
traversal.property(key, value);
} else {
if (value != null) {
if (!(value instanceof Enum)) {
if (LOGGER.isWarnEnabled()) {
LOGGER.warn(
"Using string for value. type=" + value.getClass().getSimpleName());
}
}
final String strValue = String.valueOf(value).intern();
traversal.property(key, strValue);
}
}
CaseSafePropertyUtil.addCaseSafeProperty(traversal, key, value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public void build() {

@Override
protected void afterFileInsert(GraphTraversalSource g, Vertex vNode) {
super.afterFileInsert(g, vNode);
ApexPropertyAnnotator.apply(g, vNode);
}
}
20 changes: 19 additions & 1 deletion sfge/src/main/java/com/salesforce/graph/build/GremlinUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ public static Optional<Vertex> getOnlyChild(
if (children.isEmpty()) {
return Optional.empty();
} else if (children.size() > 1) {
throw new UnexpectedException(children);
throw new UnexpectedException(
"Did not expect more than one child node of type "
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding more information to the exception.

+ childLabel
+ ". Actual count: "
+ children.size());
} else {
return Optional.of(children.get(0));
}
Expand All @@ -56,6 +60,11 @@ public static List<Vertex> getChildren(GraphTraversalSource g, Vertex vertex) {
.toList();
}

public static List<Vertex> getChildren(
GraphTraversalSource g, Vertex vertex, String childLabel) {
return g.V(vertex).out(Schema.CHILD).hasLabel(childLabel).toList();
}

public static Optional<Vertex> getPreviousSibling(GraphTraversalSource g, Vertex vertex) {
Iterator<Vertex> it = g.V(vertex).in(Schema.NEXT_SIBLING);
if (it.hasNext()) {
Expand Down Expand Up @@ -87,5 +96,14 @@ public static Optional<Vertex> getFirstChild(GraphTraversalSource g, Vertex vert
}
}

public static Optional<Vertex> getParent(GraphTraversalSource g, Vertex vertex) {
Iterator<Vertex> it = g.V(vertex).out(Schema.PARENT);
if (it.hasNext()) {
return Optional.of(it.next());
} else {
return Optional.empty();
}
}

private GremlinUtil() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package com.salesforce.graph.build;

import com.salesforce.exception.UnexpectedException;
import com.salesforce.graph.Schema;
import java.util.List;
import java.util.Optional;
import java.util.TreeSet;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
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;

/** Handles common operations performed while creating vertices on Graph */
public final class GremlinVertexUtil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored existing code into this util for core reuse.

private static final Logger LOGGER = LogManager.getLogger(GremlinVertexUtil.class);

private GremlinVertexUtil() {}

/** Create parent-child relationship between vertices on graph */
static void addParentChildRelationship(
GraphTraversalSource g, Vertex parentVertex, Vertex childVertex) {
// We are currently adding PARENT and CHILD, in theory the same edge could be navigated
// both ways, however
// the code looks messy when that is done.
// TODO: Determine if this causes performance or resource issues. Consider a single edge
g.addE(Schema.PARENT).from(childVertex).to(parentVertex).iterate();
g.addE(Schema.CHILD).from(parentVertex).to(childVertex).iterate();
}

/** Make a synthetic vertex a sibling of an existing vertex on graph */
static void makeSiblings(GraphTraversalSource g, Vertex vertex, Vertex syntheticVertex) {
final Vertex rootVertex = getParentVertex(g, vertex);

addParentChildRelationship(g, rootVertex, syntheticVertex);
}

static Vertex getParentVertex(GraphTraversalSource g, Vertex vertex) {
// Get parent node of vertex
final Optional<Vertex> rootVertex = GremlinUtil.getParent(g, vertex);
if (!rootVertex.isPresent()) {
throw new UnexpectedException(
"Did not expect vertex to not have a parent vertex. vertex=" + vertex);
}
return rootVertex.get();
}

/** Add a property to the traversal, throwing an exception if any keys are duplicated. */
protected static void addProperty(
TreeSet<String> previouslyInsertedKeys,
GraphTraversal<Vertex, Vertex> traversal,
String keyParam,
Object value) {
final String key = keyParam.intern();

if (!previouslyInsertedKeys.add(key)) {
throw new UnexpectedException(key);
}

if (value instanceof List) {
List list = (List) value;
// Convert ArrayList to an Array. There seems to be a Tinkerpop bug where a singleton
// ArrayList
// isn't properly stored. Remote graphs also have issues with empty lists, so don't add
// an empty list.
if (!list.isEmpty()) {
traversal.property(key, list.toArray());
} else {
// return so that we don't store a case insensitive version
return;
}
} else if (value instanceof Boolean
|| value instanceof Double
|| value instanceof Integer
|| value instanceof Long
|| value instanceof String) {
traversal.property(key, value);
} else {
if (value != null) {
if (!(value instanceof Enum)) {
if (LOGGER.isWarnEnabled()) {
LOGGER.warn(
"Using string for value. type=" + value.getClass().getSimpleName());
}
}
final String strValue = String.valueOf(value).intern();
traversal.property(key, strValue);
}
}
CaseSafePropertyUtil.addCaseSafeProperty(traversal, key, value);
}
}
Loading