Claudenw commented on code in PR #704:
URL: https://github.com/apache/creadur-rat/pull/704#discussion_r3667368302
##########
apache-rat-core/src/main/java/org/apache/rat/analysis/matchers/SPDXMatcherFactory.java:
##########
@@ -101,10 +125,10 @@ public Match create(final String spdxId) {
if (StringUtils.isBlank(spdxId)) {
throw new ConfigurationException("'SPDX' type matcher requires a
name");
}
- Match matcher = MATCHER_MAP.get(spdxId);
+ Match matcher = matcherMap.get(spdxId);
if (matcher == null) {
matcher = new Match(spdxId);
- MATCHER_MAP.put(spdxId, matcher);
+ matcherMap.put(spdxId, matcher);
Review Comment:
We can replace this block with `matcherMap.computIfAbsent()`
##########
src/changes/changes.xml:
##########
@@ -68,6 +68,9 @@ in order to be properly linked in site reports.
</release>
-->
<release version="1.0.0-SNAPSHOT" date="xxxx-yy-zz" description="Current
SNAPSHOT - release to be done">
+ <action issue="RAT-573" type="add" dev="pottlinger" due-to="Guillaume
Nodet">
+ Make DefaultLog and DeprecationReporter thread-safe in order to allow
parallel Maven builds.
Review Comment:
Let's change this to "Make RAT safe for parallel builds" as it should work
for Maven and any other parallel system.
##########
apache-rat-core/src/main/java/org/apache/rat/analysis/matchers/SPDXMatcherFactory.java:
##########
@@ -40,18 +40,29 @@
* SPDX identifiers are specified by the Software Package Data Exchange(R) also
* known as SPDX(R) project from the Linux foundation.
* </p>
+ * <p>
+ * Each factory instance maintains its own matcher map and per-document match
+ * state ({@code lastMatch}, {@code checked}). In multi-threaded environments
+ * (e.g. parallel Maven builds), use {@link #newInstance()} or a
+ * {@code ThreadLocal<SPDXMatcherFactory>} to obtain a per-thread factory
+ * instead of the shared {@link #INSTANCE}.
+ * </p>
*
* @see <a href="https://spdx.dev/ids/";>List of Ids at spdx.dev</a>
*/
public final class SPDXMatcherFactory {
/**
- * The collection of all matchers produced by this factory.
+ * The collection of all matchers produced by this factory instance.
*/
- private static final Map<String, SPDXMatcherFactory.Match> MATCHER_MAP =
new HashMap<>();
+ private final Map<String, SPDXMatcherFactory.Match> matcherMap = new
HashMap<>();
/**
- * The instance of this factory.
+ * The shared instance of this factory.
+ * <p>
+ * <b>Not thread-safe</b> — in multi-threaded environments use
+ * {@link #newInstance()} to create per-thread instances instead.
+ * </p>
*/
public static final SPDXMatcherFactory INSTANCE = new SPDXMatcherFactory();
Review Comment:
Should we deprecate this? We will be releasing 1.0.0 at which time we will
remove all deprecated bits. So we can deprecate this now,still be able to
release another 0.x.0 version and be able to remove it for release of 1.0.0
##########
apache-rat-core/src/main/java/org/apache/rat/analysis/matchers/SPDXMatcherFactory.java:
##########
@@ -77,12 +88,25 @@ public final class SPDXMatcherFactory {
private boolean checked;
/**
- * Constructor.
+ * Constructor. Creates a new factory with its own matcher map and match
state.
*/
- private SPDXMatcherFactory() {
+ SPDXMatcherFactory() {
Review Comment:
Why is this not private?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
