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

RAT-190: Fixes for minor bugs that blocked problem detection #250

Merged
merged 3 commits into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
20 changes: 12 additions & 8 deletions apache-rat-core/src/main/java/org/apache/rat/Report.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@
*/
public class Report {

/*
If there are changes to Options the example output should be regenerated and placed in
apache-rat/src/site/apt/index.apt.vm
Be careful of formatting as some editors get confused.
*/
private static final String[] NOTES = {
"Rat highlights possible issues.",
"Rat reports require interpretation.",
Expand Down Expand Up @@ -160,7 +165,7 @@ public String desc() {
* Name of File to exclude from report consideration.
*/
static final Option EXCLUDE_CLI = Option.builder("e").longOpt("exclude").hasArgs().argName("Expression")
.desc("Excludes files matching wildcard <expression>. May be followed by multiple arguments. "
.desc("Excludes files matching wildcard <Expression>. May be followed by multiple arguments. "
Copy link
Contributor

Choose a reason for hiding this comment

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

Pls update the site template to keep its text in sync with the stuff in the code. Should we add a javadoc in this class on where to keep it in sync with in case of changes?

+ "Note that '--' or a following option is required when using this parameter.")
.build();
/**
Expand Down Expand Up @@ -188,7 +193,8 @@ public String desc() {
* @since 0.16.0
*/
static final Option LICENSES = Option.builder().longOpt("licenses").hasArgs().argName("FileOrURI")
.desc("File names or URLs for license definitions")
.desc("File names or URLs for license definitions. May be followed by multiple arguments. " +
"Note that '--' or a following option is required when using this parameter.")
.build();
/**
* Do not use the default files.
Expand Down Expand Up @@ -224,7 +230,7 @@ public String desc() {
* @since 0.16.0
*/
static final Option LOG_LEVEL = Option.builder().longOpt("log-level")
.hasArgs().argName("LogLevel")
.hasArg().argName("LogLevel")
.desc("sets the log level.")
.converter(s -> Log.Level.valueOf(s.toUpperCase()))
.build();
Expand Down Expand Up @@ -347,7 +353,7 @@ static ReportConfiguration createConfiguration(String baseDirectory, CommandLine

if (cl.hasOption(LIST_LICENSES)) {
try {
configuration.listFamilies(cl.getParsedOptionValue(LIST_LICENSES));
configuration.listLicenses(cl.getParsedOptionValue(LIST_LICENSES));
} catch (ParseException e) {
logParseException(e, LIST_LICENSES, cl, Defaults.LIST_LICENSES);
}
Expand Down Expand Up @@ -424,7 +430,7 @@ static ReportConfiguration createConfiguration(String baseDirectory, CommandLine
}
Defaults defaults = defaultBuilder.build(DefaultLog.INSTANCE);
configuration.setFrom(defaults);
if (baseDirectory != null) {
if (StringUtils.isNotBlank(baseDirectory)) {
configuration.setReportable(getDirectory(baseDirectory, configuration));
}
return configuration;
Expand Down Expand Up @@ -553,9 +559,7 @@ private static IReportable getDirectory(String baseDirectory, ReportConfiguratio
* This class implements the {@code Comparator} interface for comparing Options.
*/
public static class OptionComparator implements Comparator<Option>, Serializable {
/**
* The serial version UID.
*/
/** The serial version UID. */
private static final long serialVersionUID = 5305467873966684014L;

private String getKey(Option opt) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
import org.apache.rat.report.xml.writer.IXmlWriter;

public class SimpleXmlClaimReporter extends AbstractReport {
private static final String RAT_REPORT = "rat-report";
private static final String TIMESTAMP = "timestamp";
private static final String RESOURCE = "resource";
private static final String LICENSE = "license";
private static final String APPROVAL = "approval";
Expand Down Expand Up @@ -89,20 +87,9 @@ private void writeHeaderSample(final MetaData metaData) throws IOException {

@Override
public void startReport() throws RatException {
try {
writer.openElement(RAT_REPORT).attribute(TIMESTAMP,
DateFormatUtils.ISO_8601_EXTENDED_DATETIME_TIME_ZONE_FORMAT.format(Calendar.getInstance()));
} catch (IOException e) {
throw new RatException("Cannot open start element", e);
}
}

@Override
public void endReport() throws RatException {
try {
writer.closeElement();
} catch (IOException e) {
throw new RatException("Cannot close start element: "+RAT_REPORT, e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.apache.rat.document.IDocumentAnalyser;
import org.apache.rat.document.RatDocumentAnalysisException;
import org.apache.rat.report.RatReport;
import org.apache.rat.report.xml.XmlReportFactory;
import org.apache.rat.report.xml.writer.IXmlWriter;

/**
* Executes a RatReport that multiplexes the running of multiple RatReports
Expand All @@ -35,17 +37,20 @@ public class ClaimReporterMultiplexer implements RatReport {
private final List<? extends RatReport> reporters;
private final boolean dryRun;

private final IXmlWriter writer;

/**
* A multiplexer to run multiple claim reports.
* @param dryRun true if this is a dry run.
* @param analyser the analyser to use.
* @param reporters the reports to execute.
*/
public ClaimReporterMultiplexer(final boolean dryRun, final IDocumentAnalyser analyser,
final List<? extends RatReport> reporters) {
public ClaimReporterMultiplexer(final IXmlWriter writer, final boolean dryRun, final IDocumentAnalyser analyser,
final List<? extends RatReport> reporters) {
this.analyser = analyser;
this.reporters = reporters;
this.dryRun = dryRun;
this.writer = writer;
}

@Override
Expand All @@ -66,6 +71,7 @@ public void report(Document document) throws RatException {

@Override
public void startReport() throws RatException {
XmlReportFactory.startReport(writer);
for (RatReport report : reporters) {
report.startReport();
}
Expand All @@ -76,5 +82,6 @@ public void endReport() throws RatException {
for (RatReport report : reporters) {
report.endReport();
}
XmlReportFactory.endReport(writer);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
* KIND, either express or implied. See the License for the *
* specific language governing permissions and limitations *
* under the License. *
*/
*/
package org.apache.rat.report.xml;

import org.apache.commons.lang3.time.DateFormatUtils;
import org.apache.rat.ReportConfiguration;
import org.apache.rat.analysis.DefaultAnalyserFactory;
import org.apache.rat.api.RatException;
import org.apache.rat.document.IDocumentAnalyser;
import org.apache.rat.document.impl.util.DocumentAnalyserMultiplexer;
import org.apache.rat.license.LicenseSetFactory.LicenseFilter;
Expand All @@ -33,45 +35,80 @@
import org.apache.rat.report.claim.util.LicenseAddingReport;
import org.apache.rat.report.xml.writer.IXmlWriter;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Calendar;
import java.util.List;

/**
* A factory to create reports from a writer and a configuration.
*
*/
public class XmlReportFactory {
public final class XmlReportFactory {
private static final String RAT_REPORT = "rat-report";
private static final String TIMESTAMP = "timestamp";

private XmlReportFactory() {
// Do not instantiate
}

/**
* Creates a RatReport from the arguments.
* The {@code statistic} is used to create a ClaimAggregator.
* If the {@code configuration} indicates that licenses should be added a LicenseAddingReport is added.
* @param writer The XML writer to send output to.
* @param statistic the ClaimStatistics for the report. may be null.
*
* @param writer The XML writer to send output to.
* @param statistic the ClaimStatistics for the report. may be null.
* @param configuration The report configuration.
* @return a RatReport instance.
*/
public static RatReport createStandardReport(IXmlWriter writer,
final ClaimStatistic statistic, ReportConfiguration configuration) {
public static RatReport createStandardReport(final IXmlWriter writer, final ClaimStatistic statistic, final ReportConfiguration configuration) {
final List<RatReport> reporters = new ArrayList<>();
if (statistic != null) {
reporters.add(new ClaimAggregator(statistic));
}
if (configuration.isAddingLicenses() && !configuration.isDryRun()) {
reporters.add(new LicenseAddingReport(configuration.getLog(), configuration.getCopyrightMessage(), configuration.isAddingLicensesForced()));
}

if (configuration.listFamilies() != LicenseFilter.NONE || configuration.listLicenses() != LicenseFilter.NONE) {

reporters.add(new ConfigurationReport(writer, configuration));
}

reporters.add(new SimpleXmlClaimReporter(writer));

final IDocumentAnalyser analyser = DefaultAnalyserFactory.createDefaultAnalyser(configuration);
final DefaultPolicy policy = new DefaultPolicy(configuration.getLicenseFamilies(LicenseFilter.APPROVED));

final IDocumentAnalyser[] analysers = {analyser, policy};
DocumentAnalyserMultiplexer analysisMultiplexer = new DocumentAnalyserMultiplexer(analysers);
return new ClaimReporterMultiplexer(configuration.isDryRun(), analysisMultiplexer, reporters);
return new ClaimReporterMultiplexer(writer, configuration.isDryRun(), analysisMultiplexer, reporters);
}

Copy link
Contributor

@ottlinger ottlinger May 11, 2024

Choose a reason for hiding this comment

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

LHF: Pls reformat this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LHF? what does that mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sry, Low Hanging Fruit (LHF) - I use this abbreviation in commits or reviews in case of minor details or simple things.

/**
* Starts the XML report by writing the standard header into the writer.
* @param writer The writer to write into
* @throws RatException on error
*/
public static void startReport(final IXmlWriter writer) throws RatException {
try {
writer.openElement(RAT_REPORT).attribute(TIMESTAMP, DateFormatUtils.ISO_8601_EXTENDED_DATETIME_TIME_ZONE_FORMAT.format(Calendar.getInstance()));
} catch (IOException e) {
throw new RatException("Cannot open start element", e);
}
}

/**
* Ends the XML reprot by closing the element that startReport opened.
* @param writer the write to write into.
* @throws RatException on error
* @see #startReport(IXmlWriter)
*/
public static void endReport(final IXmlWriter writer) throws RatException {
try {
writer.closeElement();
} catch (IOException e) {
throw new RatException("Cannot close start element: " + RAT_REPORT, e);
}
}
}
23 changes: 23 additions & 0 deletions apache-rat-core/src/test/java/org/apache/rat/ReportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
package org.apache.rat;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand All @@ -29,13 +30,16 @@
import java.io.StringWriter;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.SortedSet;

import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.DefaultParser;
import org.apache.commons.cli.Option;
import org.apache.commons.cli.Options;
import org.apache.commons.cli.ParseException;
import org.apache.commons.io.FileUtils;
import org.apache.rat.license.ILicense;
import org.apache.rat.license.LicenseSetFactory;
import org.apache.rat.testhelpers.TextUtils;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
Expand Down Expand Up @@ -126,4 +130,23 @@ public void testXMLOutput() throws Exception {
config = createConfig(longOpt(Report.XML));
assertFalse(config.isStyleReport());
}

@Test
public void LicensesOptionTest() throws Exception {
CommandLine cl = new DefaultParser().parse(Report.buildOptions(), new String[] {"-licenses", "target/test-classes/report/LicenseOne.xml"});
ReportConfiguration config = Report.createConfiguration("", cl);
SortedSet<ILicense> set = config.getLicenses(LicenseSetFactory.LicenseFilter.ALL);
ILicense found = LicenseSetFactory.search("LiOne", set);
assertNotNull(found);
}

@Test
public void LicensesOptionNoDefaultsTest() throws Exception {
CommandLine cl = new DefaultParser().parse(Report.buildOptions(), new String[] {"--no-default", "--licenses", "target/test-classes/report/LicenseOne.xml", "--licenses", "target/test-classes/report/LicenseTwo.xml"});
ReportConfiguration config = Report.createConfiguration("", cl);
SortedSet<ILicense> set = config.getLicenses(LicenseSetFactory.LicenseFilter.ALL);
assertEquals(2, set.size());
assertNotNull(LicenseSetFactory.search("LiOne", set), "LiOne");
assertNotNull(LicenseSetFactory.search("LiTwo", set), "LiTwo");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void roundTrip() throws RatException {
StringWriter writer = new StringWriter();
underTest.write(writer);
writer.flush();
System.out.println(writer.toString());
System.out.println(writer);
XMLConfigurationReader reader = new XMLConfigurationReader();
StringReader strReader = new StringReader(writer.toString());
reader.read(strReader);
Expand All @@ -81,9 +81,9 @@ public void testGen() throws Exception {
XPath xPath = XPathFactory.newInstance().newXPath();
Document doc = XmlUtils.toDom(new ByteArrayInputStream(result.getBytes()));

Node any = (Node) xPath.compile(String.format("/license[@id='GEN']/any")).evaluate(doc, XPathConstants.NODE);
assertNotNull(any, () -> "GEN/any node missing");
Node any = (Node) xPath.compile("/license[@id='GEN']/any").evaluate(doc, XPathConstants.NODE);
assertNotNull(any, "GEN/any node missing");
assertEquals(0, any.getChildNodes().getLength());
assertNotNull(any.getAttributes().getNamedItem("resource"), () -> "'resource' attribute missing");
assertNotNull(any.getAttributes().getNamedItem("resource"), "'resource' attribute missing");
}
}
11 changes: 11 additions & 0 deletions apache-rat-core/src/test/resources/report/LicenseOne.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<rat-config>
<families>
<family id="LiOne" name="License One" />
</families>
<licenses>
<license family="LiOne">
<text>License One</text>
</license>
</licenses>
</rat-config>
11 changes: 11 additions & 0 deletions apache-rat-core/src/test/resources/report/LicenseTwo.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<rat-config>
<families>
<family id="LiTwo" name="License Two" />
</families>
<licenses>
<license family="LiTwo">
<text>License Two</text>
</license>
</licenses>
</rat-config>