Skip to content

Commit bbad8b1

Browse files
JonathingJonathing
committed
Make EnhancedProblems serializable and no longer extend Problems
Addresses known issues with custom objects implementing the `Problems` interface. Also allows for cleanup in ToolExecBase where the enhanced problems isntance would need to be recalculated at execution time, it can instead now serialize the enhanced problems instance. This *does not* affect existing plugins as all plugins must shadow the shared base. No changes need to be made when updating the shared base, just a re-compile.
1 parent 45306d4 commit bbad8b1

File tree

7 files changed

+76
-98
lines changed

7 files changed

+76
-98
lines changed

‎gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/EnhancedPlugin.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ private DirectoryProperty makeGlobalCaches() {
133133
} catch (Exception e) {
134134
throw this.problemsInternal.illegalPluginTarget(
135135
new IllegalArgumentException(String.format("Failed to get %s global caches directory for target: %s", this.displayName, this.target), e),
136-
"projects or settings"
136+
"types with access to Gradle via `#getGradle()`"
137137
);
138138
}
139139
}
@@ -162,7 +162,7 @@ private DirectoryProperty makeLocalCaches() {
162162
} catch (Exception e) {
163163
throw this.problemsInternal.illegalPluginTarget(
164164
new IllegalArgumentException(String.format("Failed to get %s local caches directory for target: %s", this.displayName, this.getTarget()), e),
165-
"projects or settings"
165+
Project.class, Settings.class
166166
);
167167
}
168168
}
@@ -187,7 +187,7 @@ private DirectoryProperty makeWorkingProjectDirectory() {
187187
} catch (Exception e) {
188188
throw this.problemsInternal.illegalPluginTarget(
189189
new IllegalArgumentException(String.format("Failed to get %s working project directory for target: %s", this.displayName, this.getTarget()), e),
190-
"projects or settings"
190+
Project.class, Settings.class
191191
);
192192
}
193193
}

‎gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/EnhancedProblems.java‎

Lines changed: 63 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
package net.minecraftforge.gradleutils.shared;
66

77
import groovy.lang.GroovyObjectSupport;
8-
import org.codehaus.groovy.runtime.InvokerHelper;
98
import org.gradle.api.Action;
109
import org.gradle.api.Task;
1110
import org.gradle.api.Transformer;
1211
import org.gradle.api.file.Directory;
1312
import org.gradle.api.file.FileSystemLocation;
13+
import org.gradle.api.logging.Logger;
14+
import org.gradle.api.logging.Logging;
1415
import org.gradle.api.problems.Problem;
1516
import org.gradle.api.problems.ProblemGroup;
1617
import org.gradle.api.problems.ProblemId;
@@ -27,30 +28,60 @@
2728
import javax.inject.Inject;
2829
import java.io.File;
2930
import java.io.IOException;
31+
import java.io.Serializable;
3032
import java.nio.file.Files;
3133
import java.util.Collection;
32-
import java.util.Objects;
3334
import java.util.function.Predicate;
3435
import java.util.stream.Collectors;
3536
import java.util.stream.Stream;
3637

3738
/// The enhanced problems contain several base helper members to help reduce duplicate code between Gradle plugins.
38-
public abstract class EnhancedProblems extends GroovyObjectSupport implements Problems, Predicate<String> {
39+
public abstract class EnhancedProblems implements Serializable, Predicate<String> {
40+
private static final long serialVersionUID = 2037193772993696096L;
41+
3942
/// The common message to send in [ProblemSpec#solution(String)] when reporting problems.
4043
protected static final String HELP_MESSAGE = "Consult the documentation or ask for help on the Forge Forums, GitHub, or Discord server.";
4144

45+
/// The display name used in reported problems to describe the plugin using this.
4246
private final String displayName;
47+
/// The problem group used when reporting problems using this.
4348
private final ProblemGroup problemGroup;
4449

45-
private final Problems delegate;
46-
private final Predicate<String> properties;
50+
/// The problems instance provided by Gradle services.
51+
///
52+
/// @return The problems instance
53+
/// @see <a href="https://docs.gradle.org/current/userguide/reporting_problems.html">Reporting Problems</a>
54+
/// @deprecated Does not handle if Problems API cannot be accessed. Use [#getDelegate()].
55+
@Deprecated
56+
@SuppressWarnings("DeprecatedIsStillUsed") // Used by #getDelegate
57+
protected abstract @Inject Problems getProblems();
4758

48-
@Override
49-
public ProblemReporter getReporter() {
50-
return this.delegate.getReporter();
59+
/// The provider factory provided by Gradle services.
60+
///
61+
/// @return The provider factory
62+
/// @see <a href="https://docs.gradle.org/current/userguide/service_injection.html#providerfactory">ProviderFactory
63+
/// Service Injection</a>
64+
protected abstract @Inject ProviderFactory getProviders();
65+
66+
/// Gets the problems instance used by this enhanced problems.
67+
///
68+
/// @return The delegate problems instance
69+
public final Problems getDelegate() {
70+
try {
71+
return this.getProblems();
72+
} catch (Exception e) {
73+
return EmptyReporter.AS_PROBLEMS;
74+
}
5175
}
5276

53-
/// Gets the problem group used by this problems instance. It is unique for the plugin.
77+
/// Gets the problem reporter used by the [delegate][#getDelegate()] problems instance.
78+
///
79+
/// @return The problem reporter
80+
protected final ProblemReporter getReporter() {
81+
return this.getDelegate().getReporter();
82+
}
83+
84+
/// Gets the problem group used by this enhanced problems. It is unique for the plugin.
5485
///
5586
/// @return The problem group
5687
public final ProblemGroup getProblemGroup() {
@@ -66,9 +97,13 @@ public final ProblemGroup getProblemGroup() {
6697
/// [Inject], and have no parameters, passing in static strings to this base constructor.
6798
protected EnhancedProblems(String name, String displayName) {
6899
this.problemGroup = ProblemGroup.create(name, this.displayName = displayName);
100+
}
69101

70-
this.delegate = this.unwrapProblems();
71-
this.properties = this.unwrapProperties();
102+
/// Gets the logger to be used by this enhanced problems.
103+
///
104+
/// @return The logger
105+
protected final Logger getLogger() {
106+
return Logging.getLogger(this.getClass());
72107
}
73108

74109
/// Creates a problem ID to be used when reporting problems. The name must be unique so as to not potentially
@@ -88,8 +123,13 @@ protected final ProblemId id(String name, String displayName) {
88123
///
89124
/// @param property The property to test
90125
/// @return If the property exists and is `true`
126+
@Override
91127
public final boolean test(String property) {
92-
return this.properties.test(property);
128+
try {
129+
return isTrue(this.getProviders(), property);
130+
} catch (Exception e) {
131+
return Boolean.getBoolean(property);
132+
}
93133
}
94134

95135

@@ -108,8 +148,8 @@ public final RuntimeException illegalPluginTarget(Exception e, Class<?> firstAll
108148
return this.getReporter().throwing(e, id("invalid-plugin-target", "Invalid plugin target"), spec -> spec
109149
.details(String.format(
110150
"Attempted to apply the %s plugin to an invalid target.\n" +
111-
"This plugin can only be applied on the following types:\n" +
112-
"%s", this.displayName, Stream.concat(Stream.of(firstAllowedTarget), Stream.of(allowedTargets)).map(Class::getName).collect(Collectors.joining(", ", "[", "]"))))
151+
"This plugin can only be applied on the following types:\n" +
152+
"%s", this.displayName, Stream.concat(Stream.of(firstAllowedTarget), Stream.of(allowedTargets)).map(Class::getName).collect(Collectors.joining(", ", "[", "]"))))
113153
.severity(Severity.ERROR)
114154
.stackLocation()
115155
.solution("Use a valid plugin target.")
@@ -125,7 +165,7 @@ public final RuntimeException illegalPluginTarget(Exception e, String allowedTar
125165
return this.getReporter().throwing(e, id("invalid-plugin-target", "Invalid plugin target"), spec -> spec
126166
.details(String.format(
127167
"Attempted to apply the %s plugin to an invalid target.\n" +
128-
"This plugin can only be applied on %s", this.displayName, allowedTargets))
168+
"This plugin can only be applied on %s.", this.displayName, allowedTargets))
129169
.severity(Severity.ERROR)
130170
.stackLocation()
131171
.solution("Use a valid plugin target.")
@@ -158,8 +198,8 @@ public final void reportToolExecNotEnhanced(Task task) {
158198
this.getReporter().report(id("tool-exec-not-enhanced", "ToolExec subclass doesn't implement EnhancedTask"), spec -> spec
159199
.details(String.format(
160200
"Implementing subclass of ToolExecBase should also implement (a subclass of) EnhancedTask.\n" +
161-
"Not doing so will result in global caches being ignored. Please check your implementations.\n" +
162-
"Affected task: %s (%s)", task, task.getClass()))
201+
"Not doing so will result in global caches being ignored. Please check your implementations.\n" +
202+
"Affected task: %s (%s)", task, task.getClass()))
163203
.severity(Severity.WARNING)
164204
.stackLocation()
165205
.solution("Double check your task implementation."));
@@ -172,7 +212,7 @@ public final void reportToolExecNotEnhanced(Class<?> task) {
172212
this.getReporter().report(id("enhanced-task-no-plugin", "EnhancedTask doesn't implement #pluginType"), spec -> spec
173213
.details(String.format(
174214
"Implementation of EnhancedTask must implement #pluginType\n" +
175-
"Affected task type: %s", task))
215+
"Affected task type: %s", task))
176216
.severity(Severity.ERROR)
177217
.stackLocation()
178218
.solution("Double check your task implementation."));
@@ -185,8 +225,8 @@ public final void reportToolExecEagerArgs(Task task) {
185225
this.getReporter().report(id("tool-exec-eager-args", "ToolExecBase implementation adds arguments without using addArguments()"), spec -> spec
186226
.details(String.format(
187227
"A ToolExecBase task is eagerly adding arguments using JavaExec#args without using ToolExecBase#addArguments.\n" +
188-
"This may cause implementations or superclasses to have their arguments ignored or missing.\n" +
189-
"Affected task: %s (%s)", task, task.getClass()))
228+
"This may cause implementations or superclasses to have their arguments ignored or missing.\n" +
229+
"Affected task: %s (%s)", task, task.getClass()))
190230
.severity(Severity.WARNING)
191231
.stackLocation()
192232
.solution("Use ToolExecBase#addArguments"));
@@ -209,7 +249,7 @@ public final <T extends FileSystemLocation> Transformer<T, T> ensureFileLocation
209249
throw this.getReporter().throwing(e, id("cannot-ensure-directory", "Failed to create directory"), spec -> spec
210250
.details(String.format(
211251
"Failed to create a directory required for %s to function.\n" +
212-
"Directory: %s",
252+
"Directory: %s",
213253
this.displayName, dir.getAbsolutePath()))
214254
.severity(Severity.ERROR)
215255
.stackLocation()
@@ -272,6 +312,8 @@ default RuntimeException toRTE(Throwable exception) {
272312
/* MINIMAL */
273313

274314
static abstract class Minimal extends EnhancedProblems implements HasPublicType {
315+
private static final long serialVersionUID = -6804792858587052477L;
316+
275317
@Inject
276318
public Minimal(String name, String displayName) {
277319
super(name, displayName);
@@ -284,43 +326,6 @@ public TypeOf<?> getPublicType() {
284326
}
285327

286328

287-
/* IMPL INSTANTIATION */
288-
289-
/// The problems instance provided by Gradle services.
290-
///
291-
/// @return The problems instance
292-
/// @see <a href="https://docs.gradle.org/current/userguide/reporting_problems.html">Reporting Problems</a>
293-
protected @Inject Problems getProblems() {
294-
throw new IllegalStateException();
295-
}
296-
297-
/// The provider factory provided by Gradle services.
298-
///
299-
/// @return The provider factory
300-
/// @see <a href="https://docs.gradle.org/current/userguide/service_injection.html#providerfactory">ProviderFactory
301-
/// Service Injection</a>
302-
protected @Inject ProviderFactory getProviders() {
303-
throw new IllegalStateException();
304-
}
305-
306-
private Problems unwrapProblems() {
307-
try {
308-
return this.getProblems();
309-
} catch (Exception e) {
310-
return EmptyReporter.AS_PROBLEMS;
311-
}
312-
}
313-
314-
private Predicate<String> unwrapProperties() {
315-
try {
316-
ProviderFactory providers = Objects.requireNonNull(this.getProviders());
317-
return property -> isTrue(providers, property);
318-
} catch (Exception e) {
319-
return Boolean::getBoolean;
320-
}
321-
}
322-
323-
324329
/* IMPL UTILS */
325330

326331
private static @Nullable Boolean getBoolean(Provider<? extends String> provider) {
@@ -344,35 +349,4 @@ private static boolean isFalse(ProviderFactory providers, String property) {
344349
private static boolean isFalse(Provider<? extends String> provider) {
345350
return Boolean.FALSE.equals(getBoolean(provider));
346351
}
347-
348-
349-
/* META CLASS */
350-
351-
@Override
352-
public Object invokeMethod(String name, Object args) {
353-
try {
354-
return super.invokeMethod(name, args);
355-
} catch (Exception suppressed) {
356-
try {
357-
return InvokerHelper.getMetaClass(this.delegate).invokeMethod(this.delegate, name, args);
358-
} catch (Exception e) {
359-
e.addSuppressed(suppressed);
360-
throw e;
361-
}
362-
}
363-
}
364-
365-
@Override
366-
public Object getProperty(String propertyName) {
367-
try {
368-
return super.getProperty(propertyName);
369-
} catch (Exception suppressed) {
370-
try {
371-
return InvokerHelper.getProperty(this.delegate, propertyName);
372-
} catch (Exception e) {
373-
e.addSuppressed(suppressed);
374-
throw e;
375-
}
376-
}
377-
}
378352
}

‎gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/Tool.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@
1111
import org.jetbrains.annotations.Nullable;
1212

1313
import java.io.File;
14+
import java.io.Serializable;
1415

1516
/// Tools are definitions of Java libraries (may or may not be executable) that are managed by Gradle using a
1617
/// [org.gradle.api.provider.ValueSource]. This means that while the downloading and local caching of this file are done
1718
/// in house, the Gradle-specific caching and file tracking are done by Gradle. This enables the usage of downloading
1819
/// external files quickly without breaking caches.
19-
public interface Tool extends Named {
20+
public interface Tool extends Named, Serializable {
2021
/// Creates a new tool with the given information.
2122
///
2223
/// @param name The name for this tool (will be used in the file name)

‎gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/ToolExecBase.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@
3030
/// @see JavaExec
3131
/// @see Tool
3232
public abstract class ToolExecBase<P extends EnhancedProblems> extends JavaExec {
33-
private final Class<P> problemsType;
34-
private transient @Nullable P problems;
33+
private final P problems;
3534
/// The default tool directory (usage is not required).
3635
protected final DirectoryProperty defaultToolDir;
3736

@@ -52,7 +51,7 @@ public abstract class ToolExecBase<P extends EnhancedProblems> extends JavaExec
5251
/// best practice is to make a single `ToolExec` class for the implementing plugin to use, which other tasks can
5352
/// extend off of.
5453
protected ToolExecBase(Class<P> problemsType, Tool tool) {
55-
this.problemsType = problemsType;
54+
this.problems = this.getObjectFactory().newInstance(problemsType);
5655

5756
if (this instanceof EnhancedTask) {
5857
this.defaultToolDir = this.getObjectFactory().directoryProperty().value(
@@ -82,7 +81,7 @@ protected ToolExecBase(Class<P> problemsType, Tool tool) {
8281
///
8382
/// @return The enhanced problems
8483
protected final @Internal P getProblems() {
85-
return this.problems == null ? this.problems = this.getObjectFactory().newInstance(this.problemsType) : this.problems;
84+
return this.problems;
8685
}
8786

8887
private <T extends FileSystemLocation> Transformer<T, T> ensureFileLocationInternal() {

‎gradleutils-shared/src/main/java/net/minecraftforge/gradleutils/shared/ToolImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.io.IOException;
2323

2424
class ToolImpl implements Tool {
25+
private static final long serialVersionUID = -862411638019629688L;
2526
private static final Logger LOGGER = Logging.getLogger(Tool.class);
2627

2728
private final String name;

‎settings.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ plugins {
1313
// NOTE: We need to load this into the classpath before GradleUtils for the service to load correctly
1414
id 'io.freefair.javadoc-links' version '8.14' apply false // https://plugins.gradle.org/plugin/io.freefair.javadoc-links
1515

16-
id 'net.minecraftforge.gradleutils' version '3.1.4' // https://plugins.gradle.org/plugin/net.minecraftforge.gradleutils
16+
id 'net.minecraftforge.gradleutils' version '3.2.0' // https://plugins.gradle.org/plugin/net.minecraftforge.gradleutils
1717
id 'net.minecraftforge.gitversion' version '3.0.2' // https://plugins.gradle.org/plugin/net.minecraftforge.gitversion
1818
}
1919

‎src/main/groovy/net/minecraftforge/gradleutils/GradleUtilsProblems.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,11 @@
88
import org.gradle.api.problems.Severity;
99

1010
import javax.inject.Inject;
11+
import java.io.Serial;
1112

1213
abstract class GradleUtilsProblems extends EnhancedProblems {
14+
private static final @Serial long serialVersionUID = 3278085642147772954L;
15+
1316
@Inject
1417
public GradleUtilsProblems() {
1518
super(GradleUtilsPlugin.NAME, GradleUtilsPlugin.DISPLAY_NAME);

0 commit comments

Comments
 (0)