Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add enableLoggingFail option for failed introspect to FixtureMonkeyOptions #1069

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public final class ArbitraryGeneratorContext implements Traceable {
private final int generateUniqueMaxTries;
private final AtomicReference<CombinableArbitrary<?>> generated =
new AtomicReference<>(CombinableArbitrary.NOT_GENERATED);
private final boolean enableLoggingFail;
Copy link
Contributor

@seongahjo seongahjo Oct 29, 2024

Choose a reason for hiding this comment

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

To prevent the breaking change, could you make a class ArbitraryGeneratorLoggingContext that includes enableLoggingFail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

public final class ArbitraryGeneratorLoggingContext {
    private final boolean enableLoggingFail;
    ...
    public boolean isEnableLoggingFail(){
        return enableLoggingFail;
    }
}
public final class ArbitraryGeneratorContext implements Traceable {
    ...
    private final ArbitraryGeneratorLoggingContext loggingContext;
    ...
    public ArbitraryGeneratorLoggingContext getLoggingContext() {
        return this.loggingContext;
    }
...
}
public final class BeanArbitraryIntrospector implements ArbitraryIntrospector {
    ...
    @Override
    public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context) {
        ArbitraryGeneratorLoggingContext loggingCotext = context.getLoggingContext();
        ...
        if(loggingContext.isEnableLoggingFail()){
        ...
        }
    }
    ...
}

Like this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, indeed


public ArbitraryGeneratorContext(
Property resolvedProperty,
Expand All @@ -69,7 +70,8 @@ public ArbitraryGeneratorContext(
BiFunction<ArbitraryGeneratorContext, ArbitraryProperty, CombinableArbitrary<?>> resolveArbitrary,
LazyArbitrary<PropertyPath> lazyPropertyPath,
MonkeyGeneratorContext monkeyGeneratorContext,
int generateUniqueMaxTries
int generateUniqueMaxTries,
boolean enableLoggingFail
) {
this.resolvedProperty = resolvedProperty;
this.property = property;
Expand All @@ -79,6 +81,7 @@ public ArbitraryGeneratorContext(
this.lazyPropertyPath = lazyPropertyPath;
this.monkeyGeneratorContext = monkeyGeneratorContext;
this.generateUniqueMaxTries = generateUniqueMaxTries;
this.enableLoggingFail = enableLoggingFail;
}

public ArbitraryProperty getArbitraryProperty() {
Expand Down Expand Up @@ -149,6 +152,10 @@ public int getGenerateUniqueMaxTries() {
return generateUniqueMaxTries;
}

public boolean isEnableLoggingFail() {
return enableLoggingFail;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be better to replace it with ArbitraryGeneratorLoggingContext.


public CombinableArbitrary<?> getGenerated() {
return generated.get();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context)
try {
checkPrerequisite(type);
} catch (Exception ex) {
LOGGER.warn("Given type {} is failed to generate due to the exception. It may be null.", type, ex);
if (context.isEnableLoggingFail()) {
LOGGER.warn("Given type {} is failed to generate due to the exception. It may be null.", type, ex);
}
return ArbitraryIntrospectorResult.NOT_INTROSPECTED;
}
generated = CombinableArbitrary.from(() -> Reflections.newInstance(type));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context)
}
);
} catch (Exception ex) {
LOGGER.warn("Given type {} is failed to generate due to the exception. It may be null.", type, ex);
if (context.isEnableLoggingFail()) {
LOGGER.warn("Given type {} is failed to generate due to the exception. It may be null.", type, ex);
}
return ArbitraryIntrospectorResult.NOT_INTROSPECTED;
}
Method builderMethod = BUILDER_CACHE.get(type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,13 @@ public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context)

Entry<Constructor<?>, String[]> parameterNamesByConstructor = TypeCache.getParameterNamesByConstructor(type);
if (parameterNamesByConstructor == null) {
LOGGER.warn(
"Given type {} is failed to generate due to the exception. It may be null.",
type,
new IllegalArgumentException("Primary Constructor does not exist. type " + type.getSimpleName())
);
if (context.isEnableLoggingFail()) {
LOGGER.warn(
"Given type {} is failed to generate due to the exception. It may be null.",
type,
new IllegalArgumentException("Primary Constructor does not exist. type " + type.getSimpleName())
);
}
return ArbitraryIntrospectorResult.NOT_INTROSPECTED;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context)
results.add(new FailoverIntrospectorResult(introspector, result));
}
} catch (Exception ex) {
if (enableLoggingFail) {
if (context.isEnableLoggingFail() || enableLoggingFail) {
LOGGER.warn(
String.format(
"\"%s\" is failed to introspect \"%s\" type.",
Expand Down Expand Up @@ -85,7 +85,7 @@ public Object combined() {
result = iterator.next();
return result.getResult().getValue().combined();
} catch (Exception ex) {
if (enableLoggingFail) {
if (context.isEnableLoggingFail() || enableLoggingFail) {
LOGGER.warn(
String.format(
"\"%s\" is failed to introspect \"%s\" type.",
Expand Down Expand Up @@ -115,7 +115,7 @@ public Object rawValue() {
result = iterator.next();
return result.getResult().getValue().rawValue();
} catch (Exception ex) {
if (enableLoggingFail) {
if (context.isEnableLoggingFail() || enableLoggingFail) {
LOGGER.warn(
String.format(
"\"%s\" is failed to introspect type \"%s\"",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ public ArbitraryIntrospectorResult introspect(ArbitraryGeneratorContext context)
try {
checkPrerequisite(type);
} catch (Exception ex) {
LOGGER.warn("Given type {} is failed to generate due to the exception. It may be null.", type, ex);
if (context.isEnableLoggingFail()) {
LOGGER.warn("Given type {} is failed to generate due to the exception. It may be null.", type, ex);
}
return ArbitraryIntrospectorResult.NOT_INTROSPECTED;
}
generated = CombinableArbitrary.from(() -> Reflections.newInstance(type));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ public final class FixtureMonkeyOptions {
private final JavaConstraintGenerator javaConstraintGenerator;
private final InstantiatorProcessor instantiatorProcessor;
private final List<MatcherOperator<CandidateConcretePropertyResolver>> candidateConcretePropertyResolvers;
private final boolean enableLoggingFail;

public FixtureMonkeyOptions(
List<MatcherOperator<PropertyGenerator>> propertyGenerators,
Expand All @@ -189,7 +190,8 @@ public FixtureMonkeyOptions(
int generateUniqueMaxTries,
JavaConstraintGenerator javaConstraintGenerator,
InstantiatorProcessor instantiatorProcessor,
List<MatcherOperator<CandidateConcretePropertyResolver>> candidateConcretePropertyResolvers
List<MatcherOperator<CandidateConcretePropertyResolver>> candidateConcretePropertyResolvers,
boolean enableLoggingFail
) {
this.propertyGenerators = propertyGenerators;
this.defaultPropertyGenerator = defaultPropertyGenerator;
Expand All @@ -210,6 +212,7 @@ public FixtureMonkeyOptions(
this.javaConstraintGenerator = javaConstraintGenerator;
this.instantiatorProcessor = instantiatorProcessor;
this.candidateConcretePropertyResolvers = candidateConcretePropertyResolvers;
this.enableLoggingFail = enableLoggingFail;
}

public static FixtureMonkeyOptionsBuilder builder() {
Expand Down Expand Up @@ -351,6 +354,10 @@ public InstantiatorProcessor getInstantiatorProcessor() {
return instantiatorProcessor;
}

public boolean isEnableLoggingFail() {
return enableLoggingFail;
}

@Nullable
public CandidateConcretePropertyResolver getCandidateConcretePropertyResolver(Property property) {
List<CandidateConcretePropertyResolver> candidateConcretePropertyResolverList =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ public final class FixtureMonkeyOptionsBuilder {
private boolean defaultNotNull = false;
private boolean nullableContainer = false;
private boolean nullableElement = false;
private boolean enableLoggingFail = true;
private UnaryOperator<NullInjectGenerator> defaultNullInjectGeneratorOperator = it -> it;
private ArbitraryValidator defaultArbitraryValidator = (obj) -> {
};
Expand Down Expand Up @@ -453,6 +454,11 @@ public FixtureMonkeyOptionsBuilder nullableElement(boolean nullableElement) {
return this;
}

public FixtureMonkeyOptionsBuilder enableLoggingFail(boolean enableLoggingFail) {
this.enableLoggingFail = enableLoggingFail;
return this;
}

public FixtureMonkeyOptionsBuilder defaultArbitraryValidator(ArbitraryValidator arbitraryValidator) {
this.defaultArbitraryValidator = arbitraryValidator;
return this;
Expand Down Expand Up @@ -664,7 +670,8 @@ public FixtureMonkeyOptions build() {
this.generateUniqueMaxTries,
resolvedJavaConstraintGenerator,
this.instantiatorProcessor,
this.candidateConcretePropertyResolvers
this.candidateConcretePropertyResolvers,
this.enableLoggingFail
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,11 @@ public FixtureMonkeyBuilder nullableElement(boolean nullableElement) {
return this;
}

public FixtureMonkeyBuilder enableLoggingFail(boolean enableLoggingFail) {
this.fixtureMonkeyOptionsBuilder.enableLoggingFail(enableLoggingFail);
return this;
}

/**
* It is deprecated. Please use {@link InterfacePlugin#interfaceImplements(Matcher, List)} instead.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ private ArbitraryGeneratorContext generateContext(
},
objectNode.getLazyPropertyPath(),
monkeyGeneratorContext,
fixtureMonkeyOptions.getGenerateUniqueMaxTries()
fixtureMonkeyOptions.getGenerateUniqueMaxTries(),
fixtureMonkeyOptions.isEnableLoggingFail()
);
}

Expand Down
Loading