Fix invalid ClassCastException in graphql 16/17

* Unsafe cast to GraphQLObjectType * #1064 * https://issues.newrelic.com/browse/NEWRELIC-4936 (internal)
newrelic · Nov 16, 2022 · b28e835 · b28e835
1 parent c6bd497
commit b28e835
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 7 deletions.
diff --git a/instrumentation/graphql-java-16.2/build.gradle b/instrumentation/graphql-java-16.2/build.gradle
@@ -5,9 +5,12 @@ dependencies {
 
     testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.2'
     testImplementation 'org.junit.jupiter:junit-jupiter-params:5.7.2'
+    testImplementation 'org.mockito:mockito-core:4.6.1'
+    testImplementation 'org.mockito:mockito-junit-jupiter:4.6.1'
 
     testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.2'
-    testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.7.2'}
+    testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.7.2'
+}
 
 repositories {
     mavenCentral()

diff --git a/...ation/graphql-java-16.2/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java b/...ation/graphql-java-16.2/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java
@@ -11,7 +11,8 @@
 import graphql.execution.ExecutionStrategyParameters;
 import graphql.language.Document;
 import graphql.language.OperationDefinition;
-import graphql.schema.GraphQLObjectType;
+import graphql.schema.GraphQLNamedSchemaElement;
+import graphql.schema.GraphQLOutputType;
 
 import static com.nr.instrumentation.graphql.GraphQLObfuscator.obfuscate;
 import static com.nr.instrumentation.graphql.GraphQLOperationDefinition.getOperationTypeFrom;
@@ -38,9 +39,18 @@ public static void setOperationAttributes(final Document document, final String
 
     public static void setResolverAttributes(ExecutionStrategyParameters parameters) {
         AgentBridge.privateApi.addTracerParameter("graphql.field.path", parameters.getPath().getSegmentName());
-        GraphQLObjectType type = (GraphQLObjectType) parameters.getExecutionStepInfo().getType();
-        AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", type.getName());
         AgentBridge.privateApi.addTracerParameter("graphql.field.name", parameters.getField().getName());
+        // ExecutionStepInfo is not nullable according to documentation
+        GraphQLOutputType graphQLOutputType = parameters.getExecutionStepInfo().getType();
+        setGraphQLFieldParentTypeIfPossible(graphQLOutputType);
+    }
+
+    private static void setGraphQLFieldParentTypeIfPossible(GraphQLOutputType graphQLOutputType) {
+        // graphql.field.parentType is NOT required according to the Agent Spec
+        if (graphQLOutputType instanceof GraphQLNamedSchemaElement) {
+            GraphQLNamedSchemaElement named = (GraphQLNamedSchemaElement) graphQLOutputType;
+            AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", named.getName());
+        }
     }
 
     private static void setOperationAttributes(String type, String name, String query) {

diff --git a/...n/graphql-java-16.2/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java b/...n/graphql-java-16.2/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java
@@ -11,21 +11,34 @@
 import com.newrelic.agent.bridge.PrivateApi;
 import com.nr.instrumentation.graphql.helper.GraphQLTestHelper;
 import com.nr.instrumentation.graphql.helper.PrivateApiStub;
+import graphql.execution.ExecutionStepInfo;
+import graphql.execution.ExecutionStrategyParameters;
+import graphql.execution.MergedField;
+import graphql.execution.ResultPath;
 import graphql.language.Definition;
 import graphql.language.Document;
+import graphql.schema.GraphQLNonNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoExtension;
 
 import java.util.Collections;
 import java.util.List;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
+@ExtendWith(MockitoExtension.class)
 public class GraphQLSpanUtilTest {
 
     private static final List<Definition> NO_DEFINITIONS = Collections.emptyList();
@@ -80,4 +93,42 @@ public void testSetOperationAttributesEdgeCases(Document document, String query,
         assertEquals(expectedName, privateApiStub.getTracerParameterFor("graphql.operation.name"));
         assertEquals(expectedQuery, privateApiStub.getTracerParameterFor("graphql.operation.query"));
     }
+
+    // This test verifies https://issues.newrelic.com/browse/NEWRELIC-4936 no longer exists
+    @Test
+    public void testInvalidClassExceptionFix() {
+        // given execution parameters with execution step info type that is NOT GraphQLNamedSchemaElement
+        String expectedFieldPath = "fieldPath";
+        String expectedFieldName = "fieldName";
+        ExecutionStrategyParameters parameters =
+                mockExecutionStrategyParametersForInvalidClassCastExceptionTest(expectedFieldPath, expectedFieldName);
+
+        // when (this had ClassCastException)
+        GraphQLSpanUtil.setResolverAttributes(parameters);
+
+        // then tracer parameters set correctly without exception
+        assertEquals(expectedFieldPath, privateApiStub.getTracerParameterFor("graphql.field.path"));
+        assertEquals(expectedFieldName, privateApiStub.getTracerParameterFor("graphql.field.name"));
+
+        // and 'graphql.field.parentType' is not set
+        assertNull(privateApiStub.getTracerParameterFor("graphql.field.parentType"),
+                "'graphql.field.parentType' is not required and should be null here");
+    }
+
+    private static ExecutionStrategyParameters mockExecutionStrategyParametersForInvalidClassCastExceptionTest(String graphqlFieldPath, String graphqlFieldName) {
+        ExecutionStrategyParameters parameters = mock(ExecutionStrategyParameters.class);
+        ResultPath resultPath = mock(ResultPath.class);
+        MergedField mergedField = mock(MergedField.class);
+        ExecutionStepInfo executionStepInfo = mock(ExecutionStepInfo.class);
+        GraphQLNonNull notGraphQLNamedSchemaElement = mock(GraphQLNonNull.class);
+
+        when(resultPath.getSegmentName()).thenReturn(graphqlFieldPath);
+        when(mergedField.getName()).thenReturn(graphqlFieldName);
+        when(parameters.getPath()).thenReturn(resultPath);
+        when(parameters.getField()).thenReturn(mergedField);
+        when(parameters.getExecutionStepInfo()).thenReturn(executionStepInfo);
+        when(executionStepInfo.getType()).thenReturn(notGraphQLNamedSchemaElement);
+
+        return parameters;
+    }
 }
diff --git a/instrumentation/graphql-java-17.0/build.gradle b/instrumentation/graphql-java-17.0/build.gradle
@@ -5,6 +5,8 @@ dependencies {
 
     testImplementation 'org.junit.jupiter:junit-jupiter-api:5.7.2'
     testImplementation 'org.junit.jupiter:junit-jupiter-params:5.7.2'
+    testImplementation 'org.mockito:mockito-core:4.6.1'
+    testImplementation 'org.mockito:mockito-junit-jupiter:4.6.1'
 
     testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.7.2'
     testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.7.2'}

diff --git a/...ation/graphql-java-17.0/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java b/...ation/graphql-java-17.0/src/main/java/com/nr/instrumentation/graphql/GraphQLSpanUtil.java
@@ -11,7 +11,8 @@
 import graphql.execution.ExecutionStrategyParameters;
 import graphql.language.Document;
 import graphql.language.OperationDefinition;
-import graphql.schema.GraphQLObjectType;
+import graphql.schema.GraphQLNamedSchemaElement;
+import graphql.schema.GraphQLOutputType;
 
 import static com.nr.instrumentation.graphql.GraphQLObfuscator.obfuscate;
 import static com.nr.instrumentation.graphql.GraphQLOperationDefinition.getOperationTypeFrom;
@@ -38,9 +39,18 @@ public static void setOperationAttributes(final Document document, final String
 
     public static void setResolverAttributes(ExecutionStrategyParameters parameters) {
         AgentBridge.privateApi.addTracerParameter("graphql.field.path", parameters.getPath().getSegmentName());
-        GraphQLObjectType type = (GraphQLObjectType) parameters.getExecutionStepInfo().getType();
-        AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", type.getName());
         AgentBridge.privateApi.addTracerParameter("graphql.field.name", parameters.getField().getName());
+        // ExecutionStepInfo is not nullable according to documentation
+        GraphQLOutputType graphQLOutputType = parameters.getExecutionStepInfo().getType();
+        setGraphQLFieldParentTypeIfPossible(graphQLOutputType);
+    }
+
+    private static void setGraphQLFieldParentTypeIfPossible(GraphQLOutputType graphQLOutputType) {
+        // graphql.field.parentType is NOT required according to the Agent Spec
+        if (graphQLOutputType instanceof GraphQLNamedSchemaElement) {
+            GraphQLNamedSchemaElement named = (GraphQLNamedSchemaElement) graphQLOutputType;
+            AgentBridge.privateApi.addTracerParameter("graphql.field.parentType", named.getName());
+        }
     }
 
     private static void setOperationAttributes(String type, String name, String query) {

diff --git a/...n/graphql-java-17.0/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java b/...n/graphql-java-17.0/src/test/java/com/nr/instrumentation/graphql/GraphQLSpanUtilTest.java
@@ -11,10 +11,16 @@
 import com.newrelic.agent.bridge.PrivateApi;
 import com.nr.instrumentation.graphql.helper.GraphQLTestHelper;
 import com.nr.instrumentation.graphql.helper.PrivateApiStub;
+import graphql.execution.ExecutionStepInfo;
+import graphql.execution.ExecutionStrategyParameters;
+import graphql.execution.MergedField;
+import graphql.execution.ResultPath;
 import graphql.language.Definition;
 import graphql.language.Document;
+import graphql.schema.GraphQLNonNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.CsvSource;
@@ -25,6 +31,9 @@
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 public class GraphQLSpanUtilTest {
 
@@ -80,4 +89,42 @@ public void testSetOperationAttributesEdgeCases(Document document, String query,
         assertEquals(expectedName, privateApiStub.getTracerParameterFor("graphql.operation.name"));
         assertEquals(expectedQuery, privateApiStub.getTracerParameterFor("graphql.operation.query"));
     }
+
+    // This test verifies https://issues.newrelic.com/browse/NEWRELIC-4936 no longer exists
+    @Test
+    public void testInvalidClassExceptionFix() {
+        // given execution parameters with execution step info type that is NOT GraphQLNamedSchemaElement
+        String expectedFieldPath = "fieldPath";
+        String expectedFieldName = "fieldName";
+        ExecutionStrategyParameters parameters =
+                mockExecutionStrategyParametersForInvalidClassCastExceptionTest(expectedFieldPath, expectedFieldName);
+
+        // when (this had ClassCastException)
+        GraphQLSpanUtil.setResolverAttributes(parameters);
+
+        // then tracer parameters set correctly without exception
+        assertEquals(expectedFieldPath, privateApiStub.getTracerParameterFor("graphql.field.path"));
+        assertEquals(expectedFieldName, privateApiStub.getTracerParameterFor("graphql.field.name"));
+
+        // and 'graphql.field.parentType' is not set
+        assertNull(privateApiStub.getTracerParameterFor("graphql.field.parentType"),
+                "'graphql.field.parentType' is not required and should be null here");
+    }
+
+    private static ExecutionStrategyParameters mockExecutionStrategyParametersForInvalidClassCastExceptionTest(String graphqlFieldPath, String graphqlFieldName) {
+        ExecutionStrategyParameters parameters = mock(ExecutionStrategyParameters.class);
+        ResultPath resultPath = mock(ResultPath.class);
+        MergedField mergedField = mock(MergedField.class);
+        ExecutionStepInfo executionStepInfo = mock(ExecutionStepInfo.class);
+        GraphQLNonNull notGraphQLNamedSchemaElement = mock(GraphQLNonNull.class);
+
+        when(resultPath.getSegmentName()).thenReturn(graphqlFieldPath);
+        when(mergedField.getName()).thenReturn(graphqlFieldName);
+        when(parameters.getPath()).thenReturn(resultPath);
+        when(parameters.getField()).thenReturn(mergedField);
+        when(parameters.getExecutionStepInfo()).thenReturn(executionStepInfo);
+        when(executionStepInfo.getType()).thenReturn(notGraphQLNamedSchemaElement);
+
+        return parameters;
+    }
 }