From 170b4c81a2eed770c2f192953ae7966cdd6a84c5 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Wed, 27 Dec 2023 12:49:31 +1100 Subject: [PATCH] This removes SLF4J from the code base --- build.gradle | 5 +-- src/main/java/graphql/GraphQL.java | 37 +------------------ .../MaxQueryComplexityInstrumentation.java | 7 ---- .../java/graphql/execution/Execution.java | 7 ---- .../graphql/execution/ExecutionStrategy.java | 19 ---------- .../SimpleDataFetcherExceptionHandler.java | 5 --- .../DataLoaderDispatcherInstrumentation.java | 9 +---- ...aLoaderDispatcherInstrumentationState.java | 5 +-- .../FieldLevelTrackingApproach.java | 8 +--- .../graphql/schema/PropertyFetchingImpl.java | 4 -- .../idl/ArgValueOfAllowedTypeChecker.java | 7 ---- src/main/java/graphql/util/LogKit.java | 22 ----------- .../groovy/graphql/util/LogKitTest.groovy | 14 ------- 13 files changed, 6 insertions(+), 143 deletions(-) delete mode 100644 src/main/java/graphql/util/LogKit.java delete mode 100644 src/test/groovy/graphql/util/LogKitTest.groovy diff --git a/build.gradle b/build.gradle index e474eee399..8c17b0a887 100644 --- a/build.gradle +++ b/build.gradle @@ -61,9 +61,8 @@ def getDevelopmentVersion() { return makeDevelopmentVersion(["0.0.0", branchName, "SNAPSHOT"]) } -def reactiveStreamsVersion = '1.0.3' -def slf4jVersion = '2.0.7' def releaseVersion = System.env.RELEASE_VERSION +def reactiveStreamsVersion = '1.0.3' def antlrVersion = '4.11.1' // https://mvnrepository.com/artifact/org.antlr/antlr4-runtime def guavaVersion = '32.1.2-jre' version = releaseVersion ? releaseVersion : getDevelopmentVersion() @@ -101,7 +100,6 @@ jar { dependencies { compileOnly 'org.jetbrains:annotations:24.1.0' implementation 'org.antlr:antlr4-runtime:' + antlrVersion - implementation 'org.slf4j:slf4j-api:' + slf4jVersion api 'com.graphql-java:java-dataloader:3.2.2' api 'org.reactivestreams:reactive-streams:' + reactiveStreamsVersion antlr 'org.antlr:antlr4:' + antlrVersion @@ -113,7 +111,6 @@ dependencies { testImplementation 'com.google.code.gson:gson:2.10.1' testImplementation 'org.eclipse.jetty:jetty-server:11.0.15' testImplementation 'com.fasterxml.jackson.core:jackson-databind:2.16.0' - testImplementation 'org.slf4j:slf4j-simple:' + slf4jVersion testImplementation 'org.awaitility:awaitility-groovy:4.2.0' testImplementation 'com.github.javafaker:javafaker:1.0.2' diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 2ccb54b955..535accfdbe 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -28,10 +28,7 @@ import graphql.execution.preparsed.PreparsedDocumentProvider; import graphql.language.Document; import graphql.schema.GraphQLSchema; -import graphql.util.LogKit; import graphql.validation.ValidationError; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.List; @@ -91,9 +88,6 @@ @PublicApi public class GraphQL { - private static final Logger log = LoggerFactory.getLogger(GraphQL.class); - private static final Logger logNotSafe = LogKit.getNotPrivacySafeLogger(GraphQL.class); - private final GraphQLSchema graphQLSchema; private final ExecutionStrategy queryStrategy; private final ExecutionStrategy mutationStrategy; @@ -419,9 +413,6 @@ public CompletableFuture executeAsync(UnaryOperator executeAsync(ExecutionInput executionInput) { - if (logNotSafe.isDebugEnabled()) { - logNotSafe.debug("Executing request. operation name: '{}'. query: '{}'. variables '{}'", executionInput.getOperationName(), executionInput.getQuery(), executionInput.getVariables()); - } ExecutionInput executionInputWithId = ensureInputHasId(executionInput); CompletableFuture instrumentationStateCF = instrumentation.createStateAsync(new InstrumentationCreateStateParameters(this.graphQLSchema, executionInput)); @@ -496,12 +487,8 @@ private PreparsedDocumentEntry parseAndValidate(AtomicReference ExecutionInput executionInput = executionInputRef.get(); String query = executionInput.getQuery(); - if (logNotSafe.isDebugEnabled()) { - logNotSafe.debug("Parsing query: '{}'...", query); - } ParseAndValidateResult parseResult = parse(executionInput, graphQLSchema, instrumentationState); if (parseResult.isFailure()) { - logNotSafe.warn("Query did not parse : '{}'", executionInput.getQuery()); return new PreparsedDocumentEntry(parseResult.getSyntaxException().toInvalidSyntaxError()); } else { final Document document = parseResult.getDocument(); @@ -509,12 +496,8 @@ private PreparsedDocumentEntry parseAndValidate(AtomicReference executionInput = executionInput.transform(builder -> builder.variables(parseResult.getVariables())); executionInputRef.set(executionInput); - if (logNotSafe.isDebugEnabled()) { - logNotSafe.debug("Validating query: '{}'", query); - } final List errors = validate(executionInput, document, graphQLSchema, instrumentationState); if (!errors.isEmpty()) { - logNotSafe.warn("Query did not validate : '{}'", query); return new PreparsedDocumentEntry(document, errors); } @@ -562,25 +545,7 @@ private CompletableFuture execute(ExecutionInput executionInput Execution execution = new Execution(queryStrategy, mutationStrategy, subscriptionStrategy, instrumentation, valueUnboxer); ExecutionId executionId = executionInput.getExecutionId(); - if (logNotSafe.isDebugEnabled()) { - logNotSafe.debug("Executing '{}'. operation name: '{}'. query: '{}'. variables '{}'", executionId, executionInput.getOperationName(), executionInput.getQuery(), executionInput.getVariables()); - } - CompletableFuture future = execution.execute(document, graphQLSchema, executionId, executionInput, instrumentationState); - future = future.whenComplete((result, throwable) -> { - if (throwable != null) { - logNotSafe.error(String.format("Execution '%s' threw exception when executing : query : '%s'. variables '%s'", executionId, executionInput.getQuery(), executionInput.getVariables()), throwable); - } else { - if (log.isDebugEnabled()) { - int errorCount = result.getErrors().size(); - if (errorCount > 0) { - log.debug("Execution '{}' completed with '{}' errors", executionId, errorCount); - } else { - log.debug("Execution '{}' completed with zero errors", executionId); - } - } - } - }); - return future; + return execution.execute(document, graphQLSchema, executionId, executionInput, instrumentationState); } private static Instrumentation checkInstrumentationDefaultState(Instrumentation instrumentation, boolean doNotAddDefaultInstrumentations) { diff --git a/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java b/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java index 87a00e976a..b952214948 100644 --- a/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java +++ b/src/main/java/graphql/analysis/MaxQueryComplexityInstrumentation.java @@ -12,8 +12,6 @@ import graphql.execution.instrumentation.parameters.InstrumentationValidationParameters; import graphql.validation.ValidationError; import org.jetbrains.annotations.Nullable; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.List; import java.util.concurrent.atomic.AtomicReference; @@ -32,8 +30,6 @@ @PublicApi public class MaxQueryComplexityInstrumentation extends SimplePerformantInstrumentation { - private static final Logger log = LoggerFactory.getLogger(MaxQueryComplexityInstrumentation.class); - private final int maxComplexity; private final FieldComplexityCalculator fieldComplexityCalculator; private final Function maxQueryComplexityExceededFunction; @@ -100,9 +96,6 @@ public InstrumentationState createState(InstrumentationCreateStateParameters par State state = ofState(rawState); QueryComplexityCalculator queryComplexityCalculator = newQueryComplexityCalculator(instrumentationExecuteOperationParameters.getExecutionContext()); int totalComplexity = queryComplexityCalculator.calculate(); - if (log.isDebugEnabled()) { - log.debug("Query complexity: {}", totalComplexity); - } if (totalComplexity > maxComplexity) { QueryComplexityInfo queryComplexityInfo = QueryComplexityInfo.newQueryComplexityInfo() .complexity(totalComplexity) diff --git a/src/main/java/graphql/execution/Execution.java b/src/main/java/graphql/execution/Execution.java index 916bc64659..133ef2cda9 100644 --- a/src/main/java/graphql/execution/Execution.java +++ b/src/main/java/graphql/execution/Execution.java @@ -21,8 +21,6 @@ import graphql.schema.GraphQLObjectType; import graphql.schema.GraphQLSchema; import graphql.schema.impl.SchemaUtil; -import graphql.util.LogKit; -import org.slf4j.Logger; import java.util.Collections; import java.util.List; @@ -37,8 +35,6 @@ @Internal public class Execution { - private static final Logger logNotSafe = LogKit.getNotPrivacySafeLogger(Execution.class); - private final FieldCollector fieldCollector = new FieldCollector(); private final ExecutionStrategy queryStrategy; private final ExecutionStrategy mutationStrategy; @@ -155,9 +151,6 @@ private CompletableFuture executeOperation(ExecutionContext exe CompletableFuture result; try { ExecutionStrategy executionStrategy = executionContext.getStrategy(operation); - if (logNotSafe.isDebugEnabled()) { - logNotSafe.debug("Executing '{}' query operation: '{}' using '{}' execution strategy", executionContext.getExecutionId(), operation, executionStrategy.getClass().getName()); - } result = executionStrategy.execute(executionContext, parameters); } catch (NonNullableFieldWasNullException e) { // this means it was non-null types all the way from an offending non-null type diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java index 4ed0f1e644..fd22112b45 100644 --- a/src/main/java/graphql/execution/ExecutionStrategy.java +++ b/src/main/java/graphql/execution/ExecutionStrategy.java @@ -40,9 +40,6 @@ import graphql.schema.GraphQLType; import graphql.schema.LightDataFetcher; import graphql.util.FpKit; -import graphql.util.LogKit; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.Collections; @@ -126,9 +123,6 @@ @SuppressWarnings("FutureReturnValueIgnored") public abstract class ExecutionStrategy { - private static final Logger log = LoggerFactory.getLogger(ExecutionStrategy.class); - private static final Logger logNotSafe = LogKit.getNotPrivacySafeLogger(ExecutionStrategy.class); - protected final FieldCollector fieldCollector = new FieldCollector(); protected final ExecutionStepInfoFactory executionStepInfoFactory = new ExecutionStepInfoFactory(); private final ResolveType resolvedType = new ResolveType(); @@ -312,9 +306,6 @@ private CompletableFuture invokeDataFetcher(ExecutionContext executionCo } fetchedValue = Async.toCompletableFuture(fetchedValueRaw); } catch (Exception e) { - if (logNotSafe.isDebugEnabled()) { - logNotSafe.debug(String.format("'%s', field '%s' fetch threw exception", executionContext.getExecutionId(), parameters.getPath()), e); - } fetchedValue = Async.exceptionallyCompletedFuture(e); } return fetchedValue; @@ -428,10 +419,6 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut .nonNullFieldValidator(nonNullableFieldValidator) ); - if (log.isDebugEnabled()) { - log.debug("'{}' completing field '{}'...", executionContext.getExecutionId(), executionStepInfo.getPath()); - } - FieldValueInfo fieldValueInfo = completeValue(executionContext, newParameters); CompletableFuture executionResultFuture = fieldValueInfo.getFieldValue(); @@ -493,9 +480,7 @@ protected FieldValueInfo completeValue(ExecutionContext executionContext, Execut private void handleUnresolvedTypeProblem(ExecutionContext context, ExecutionStrategyParameters parameters, UnresolvedTypeException e) { UnresolvedTypeError error = new UnresolvedTypeError(parameters.getPath(), parameters.getExecutionStepInfo(), e); - logNotSafe.warn(error.getMessage(), e); context.addError(error); - } /** @@ -705,10 +690,7 @@ protected CompletableFuture completeValueForObject(ExecutionCon @SuppressWarnings("SameReturnValue") private Object handleCoercionProblem(ExecutionContext context, ExecutionStrategyParameters parameters, CoercingSerializeException e) { SerializationError error = new SerializationError(parameters.getPath(), e); - logNotSafe.warn(error.getMessage(), e); context.addError(error); - - return null; } @@ -733,7 +715,6 @@ protected Iterable toIterable(ExecutionContext context, ExecutionStrateg private void handleTypeMismatchProblem(ExecutionContext context, ExecutionStrategyParameters parameters, Object result) { TypeMismatchError error = new TypeMismatchError(parameters.getPath(), parameters.getExecutionStepInfo().getUnwrappedNonNullType()); - logNotSafe.warn("{} got {}", error.getMessage(), result.getClass()); context.addError(error); } diff --git a/src/main/java/graphql/execution/SimpleDataFetcherExceptionHandler.java b/src/main/java/graphql/execution/SimpleDataFetcherExceptionHandler.java index 606de0f8a9..79e201a333 100644 --- a/src/main/java/graphql/execution/SimpleDataFetcherExceptionHandler.java +++ b/src/main/java/graphql/execution/SimpleDataFetcherExceptionHandler.java @@ -3,8 +3,6 @@ import graphql.ExceptionWhileDataFetching; import graphql.PublicApi; import graphql.language.SourceLocation; -import graphql.util.LogKit; -import org.slf4j.Logger; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -16,8 +14,6 @@ @PublicApi public class SimpleDataFetcherExceptionHandler implements DataFetcherExceptionHandler { - private static final Logger logNotSafe = LogKit.getNotPrivacySafeLogger(SimpleDataFetcherExceptionHandler.class); - static final SimpleDataFetcherExceptionHandler defaultImpl = new SimpleDataFetcherExceptionHandler(); private DataFetcherExceptionHandlerResult handleExceptionImpl(DataFetcherExceptionHandlerParameters handlerParameters) { @@ -43,7 +39,6 @@ public CompletableFuture handleException(Data * @param exception the exception that happened */ protected void logException(ExceptionWhileDataFetching error, Throwable exception) { - logNotSafe.warn(error.getMessage(), exception); } /** diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java b/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java index 87c137e303..8fc8accfde 100644 --- a/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java +++ b/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java @@ -49,9 +49,6 @@ */ @PublicApi public class DataLoaderDispatcherInstrumentation extends SimplePerformantInstrumentation { - - private static final Logger log = LoggerFactory.getLogger(DataLoaderDispatcherInstrumentation.class); - private final DataLoaderDispatcherInstrumentationOptions options; /** @@ -73,7 +70,7 @@ public DataLoaderDispatcherInstrumentation(DataLoaderDispatcherInstrumentationOp @Override public InstrumentationState createState(InstrumentationCreateStateParameters parameters) { - return new DataLoaderDispatcherInstrumentationState(log, parameters.getExecutionInput().getDataLoaderRegistry()); + return new DataLoaderDispatcherInstrumentationState(parameters.getExecutionInput().getDataLoaderRegistry()); } @Override @@ -158,10 +155,6 @@ private boolean isDataLoaderCompatibleExecution(ExecutionContext executionContex Map dataLoaderStats = buildStatsMap(state); statsMap.put("dataloader", dataLoaderStats); - if (log.isDebugEnabled()) { - log.debug("Data loader stats : {}", dataLoaderStats); - } - return CompletableFuture.completedFuture(new ExecutionResultImpl(executionResult.getData(), executionResult.getErrors(), statsMap)); } diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentationState.java b/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentationState.java index 1d6a697fa1..31d1db0499 100644 --- a/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentationState.java +++ b/src/main/java/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentationState.java @@ -6,7 +6,6 @@ import graphql.execution.instrumentation.InstrumentationState; import org.dataloader.DataLoader; import org.dataloader.DataLoaderRegistry; -import org.slf4j.Logger; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; @@ -45,9 +44,9 @@ public DataLoaderRegistry unregister(String key) { private volatile boolean aggressivelyBatching = true; private volatile boolean hasNoDataLoaders; - public DataLoaderDispatcherInstrumentationState(Logger log, DataLoaderRegistry dataLoaderRegistry) { + public DataLoaderDispatcherInstrumentationState(DataLoaderRegistry dataLoaderRegistry) { this.dataLoaderRegistry = new AtomicReference<>(dataLoaderRegistry); - this.approach = new FieldLevelTrackingApproach(log, this::getDataLoaderRegistry); + this.approach = new FieldLevelTrackingApproach(this::getDataLoaderRegistry); this.state = approach.createState(); hasNoDataLoaders = checkForNoDataLoader(dataLoaderRegistry); } diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java b/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java index 7da689db9a..a98fac3ae0 100644 --- a/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java +++ b/src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java @@ -12,7 +12,6 @@ import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters; import graphql.util.LockKit; import org.dataloader.DataLoaderRegistry; -import org.slf4j.Logger; import java.util.LinkedHashSet; import java.util.List; @@ -26,7 +25,6 @@ @Internal public class FieldLevelTrackingApproach { private final Supplier dataLoaderRegistrySupplier; - private final Logger log; private static class CallStack implements InstrumentationState { @@ -112,9 +110,8 @@ public void clearAndMarkCurrentLevelAsReady(int level) { } } - public FieldLevelTrackingApproach(Logger log, Supplier dataLoaderRegistrySupplier) { + public FieldLevelTrackingApproach(Supplier dataLoaderRegistrySupplier) { this.dataLoaderRegistrySupplier = dataLoaderRegistrySupplier; - this.log = log; } public InstrumentationState createState() { @@ -236,9 +233,6 @@ private boolean levelReady(CallStack callStack, int level) { void dispatch() { DataLoaderRegistry dataLoaderRegistry = getDataLoaderRegistry(); - if (log.isDebugEnabled()) { - log.debug("Dispatching data loaders ({})", dataLoaderRegistry.getKeys()); - } dataLoaderRegistry.dispatchAll(); } diff --git a/src/main/java/graphql/schema/PropertyFetchingImpl.java b/src/main/java/graphql/schema/PropertyFetchingImpl.java index da8b153e3d..c5f3b95656 100644 --- a/src/main/java/graphql/schema/PropertyFetchingImpl.java +++ b/src/main/java/graphql/schema/PropertyFetchingImpl.java @@ -32,8 +32,6 @@ */ @Internal public class PropertyFetchingImpl { - private static final Logger log = LoggerFactory.getLogger(PropertyFetchingImpl.class); - private final AtomicBoolean USE_SET_ACCESSIBLE = new AtomicBoolean(true); private final AtomicBoolean USE_LAMBDA_FACTORY = new AtomicBoolean(true); private final AtomicBoolean USE_NEGATIVE_CACHE = new AtomicBoolean(true); @@ -123,8 +121,6 @@ public Object getPropertyValue(String propertyName, Object object, GraphQLType g // are preventing the Meta Lambda from working. So let's continue with // old skool reflection and if it's all broken there then it will eventually // end up negatively cached - log.debug("Unable to invoke fast Meta Lambda for `{}` - Falling back to reflection", object.getClass().getName(), ignored); - } } diff --git a/src/main/java/graphql/schema/idl/ArgValueOfAllowedTypeChecker.java b/src/main/java/graphql/schema/idl/ArgValueOfAllowedTypeChecker.java index c9d2498abd..9dcd16b768 100644 --- a/src/main/java/graphql/schema/idl/ArgValueOfAllowedTypeChecker.java +++ b/src/main/java/graphql/schema/idl/ArgValueOfAllowedTypeChecker.java @@ -30,8 +30,6 @@ import graphql.schema.CoercingParseLiteralException; import graphql.schema.GraphQLScalarType; import graphql.schema.idl.errors.DirectiveIllegalArgumentTypeError; -import graphql.util.LogKit; -import org.slf4j.Logger; import java.util.List; import java.util.Locale; @@ -64,8 +62,6 @@ @Internal class ArgValueOfAllowedTypeChecker { - private static final Logger logNotSafe = LogKit.getNotPrivacySafeLogger(ArgValueOfAllowedTypeChecker.class); - private final Directive directive; private final Node element; private final String elementName; @@ -291,9 +287,6 @@ private boolean isArgumentValueScalarLiteral(GraphQLScalarType scalarType, Value scalarType.getCoercing().parseLiteral(instanceValue, CoercedVariables.emptyVariables(), GraphQLContext.getDefault(), Locale.getDefault()); return true; } catch (CoercingParseLiteralException ex) { - if (logNotSafe.isDebugEnabled()) { - logNotSafe.debug("Attempted parsing literal into '{}' but got the following error: ", scalarType.getName(), ex); - } return false; } } diff --git a/src/main/java/graphql/util/LogKit.java b/src/main/java/graphql/util/LogKit.java deleted file mode 100644 index df65060e6b..0000000000 --- a/src/main/java/graphql/util/LogKit.java +++ /dev/null @@ -1,22 +0,0 @@ -package graphql.util; - -import graphql.Internal; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -@Internal -public class LogKit { - - /** - * Creates a logger with a name indicating that the content might not be privacy safe - * eg it could contain user generated content or privacy information. - * - * @param clazz the class to make a logger for - * - * @return a new Logger - */ - public static Logger getNotPrivacySafeLogger(Class clazz) { - return LoggerFactory.getLogger(String.format("notprivacysafe.%s", clazz.getName())); - } - -} diff --git a/src/test/groovy/graphql/util/LogKitTest.groovy b/src/test/groovy/graphql/util/LogKitTest.groovy deleted file mode 100644 index eeb1582baa..0000000000 --- a/src/test/groovy/graphql/util/LogKitTest.groovy +++ /dev/null @@ -1,14 +0,0 @@ -package graphql.util - - -import spock.lang.Specification - -class LogKitTest extends Specification { - - def "logger has a prefixed name"() { - when: - def logger = LogKit.getNotPrivacySafeLogger(LogKitTest.class) - then: - logger.getName() == "notprivacysafe.graphql.util.LogKitTest" - } -}