Skip to content

Commit

Permalink
enforce DSpace checkstyle rules
Browse files Browse the repository at this point in the history
  • Loading branch information
nwoodward committed Oct 7, 2024
1 parent 6b652b4 commit 902bfca
Show file tree
Hide file tree
Showing 54 changed files with 1,132 additions and 1,328 deletions.
10 changes: 10 additions & 0 deletions checkstyle-suppressions.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE suppressions PUBLIC
"-//Puppy Crawl//DTD Suppressions 1.2//EN"
"http://checkstyle.sourceforge.net/dtds/suppressions_1_2.dtd">
<suppressions>
<!-- Temporarily suppress indentation checks for all Tests -->
<!-- TODO: We should have these turned on. But, currently there's a known bug with indentation checks
on JMockIt Expectations blocks and similar. See https://github.com/checkstyle/checkstyle/issues/3739 -->
<suppress checks="Indentation" files="src[/\\]test[/\\]java"/>
</suppressions>
140 changes: 140 additions & 0 deletions checkstyle.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC
"-//Puppy Crawl//DTD Check Configuration 1.3//EN"
"http://checkstyle.sourceforge.net/dtds/configuration_1_3.dtd">
<!--
DSpace CodeStyle Requirements
1. 4-space indents for Java, and 2-space indents for XML. NO TABS ALLOWED.
2. K&R style braces required. Braces required on all blocks.
3. Do not use wildcard imports (e.g. import java.util.*). Duplicated or unused imports also not allowed.
4. Javadocs should exist for all public classes and methods. (Methods rule is unenforced at this time.) Keep it short and to the point
5. Maximum line length is 120 characters (except for long URLs, packages or imports)
6. No trailing spaces allowed (except in comments)
7. Tokens should be surrounded by whitespace (see http://checkstyle.sourceforge.net/config_whitespace.html#WhitespaceAround)
8. Each source file must include our license header (validated separately by license-maven-plugin, see pom.xml)
For more information on CheckStyle configurations below, see: http://checkstyle.sourceforge.net/checks.html
-->
<module name="Checker">
<!-- Configure checker to use UTF-8 encoding -->
<property name="charset" value="UTF-8"/>
<!-- Configure checker to run on files with these extensions -->
<property name="fileExtensions" value="java, properties, cfg, xml"/>

<!-- Suppression configurations in checkstyle-suppressions.xml in same directory -->
<module name="SuppressionFilter">
<property name="file" value="${checkstyle.suppressions.file}" default="checkstyle-suppressions.xml"/>
</module>

<!-- No tab characters ('\t') allowed in the source code -->
<module name="FileTabCharacter">
<property name="eachLine" value="true"/>
<property name="fileExtensions" value="java, properties, cfg, css, js, xml"/>
</module>

<!-- No Trailing Whitespace, except on lines that only have an asterisk (e.g. Javadoc comments) -->
<module name="RegexpSingleline">
<property name="format" value="(?&lt;!\*)\s+$|\*\s\s+$"/>
<property name="message" value="Line has trailing whitespace"/>
<property name="fileExtensions" value="java, properties, cfg, css, js, xml"/>
</module>

<!-- Allow individual lines of code to be excluded from these rules, if they are annotated
with @SuppressWarnings. See also SuppressWarningsHolder below -->
<module name="SuppressWarningsFilter" />

<!-- Maximum line length is 120 characters -->
<module name="LineLength">
<property name="fileExtensions" value="java"/>
<property name="max" value="120"/>
<!-- Only exceptions for packages, imports, URLs, and JavaDoc {@link} tags -->
<property name="ignorePattern" value="^package.*|^import.*|http://|https://|@link"/>
</module>

<!-- Check individual Java source files for specific rules -->
<module name="TreeWalker">
<!-- Highlight any TODO or FIXME comments in info messages -->
<module name="TodoComment">
<property name="severity" value="info"/>
<property name="format" value="(TODO)|(FIXME)"/>
</module>

<!-- Do not report errors on any lines annotated with @SuppressWarnings -->
<module name="SuppressWarningsHolder"/>

<!-- ##### Import statement requirements ##### -->
<!-- Star imports (e.g. import java.util.*) are NOT ALLOWED -->
<module name="AvoidStarImport"/>
<!-- Redundant import statements are NOT ALLOWED -->
<module name="RedundantImport"/>
<!-- Unused import statements are NOT ALLOWED -->
<module name="UnusedImports"/>
<!-- Ensure imports appear alphabetically and grouped -->
<module name="CustomImportOrder">
<property name="sortImportsInGroupAlphabetically" value="true"/>
<property name="separateLineBetweenGroups" value="true"/>
<property name="customImportOrderRules" value="STATIC###STANDARD_JAVA_PACKAGE###THIRD_PARTY_PACKAGE"/>
</module>

<!-- ##### Javadocs requirements ##### -->
<!-- Requirements for Javadocs for classes/interfaces -->
<module name="JavadocType">
<!-- All public classes/interfaces MUST HAVE Javadocs -->
<property name="scope" value="public"/>
<!-- Add an exception for anonymous inner classes -->
<property name="excludeScope" value="anoninner"/>
<!-- Ignore errors related to unknown tags -->
<property name="allowUnknownTags" value="true"/>
<!-- Allow params tags to be optional -->
<property name="allowMissingParamTags" value="false"/>
</module>
<!-- Requirements for Javadocs for methods -->
<module name="JavadocMethod">
<!-- All public methods MUST HAVE Javadocs -->
<property name="scope" value="public"/>
<!-- Allow params, throws and return tags to be optional -->
<property name="allowMissingParamTags" value="true"/>
<property name="allowMissingReturnTag" value="true"/>
</module>

<!-- ##### Requirements for K&R Style braces ##### -->
<!-- Code blocks MUST HAVE braces, even single line statements (if, while, etc) -->
<module name="NeedBraces"/>
<!-- Left braces should be at the end of current line (default value)-->
<module name="LeftCurly"/>
<!-- Right braces should be on start of a new line (default value) -->
<module name="RightCurly"/>

<!-- ##### Indentation / Whitespace requirements ##### -->
<!-- Require 4-space indentation (default value) -->
<module name="Indentation"/>
<!-- Whitespace should exist around all major tokens -->
<module name="WhitespaceAround">
<!-- However, make an exception for empty constructors, methods, types, etc. -->
<property name="allowEmptyConstructors" value="true"/>
<property name="allowEmptyMethods" value="true"/>
<property name="allowEmptyTypes" value="true"/>
<property name="allowEmptyLoops" value="true"/>
</module>
<!-- Validate whitespace around Generics (angle brackets) per typical conventions
http://checkstyle.sourceforge.net/config_whitespace.html#GenericWhitespace -->
<module name="GenericWhitespace"/>

<!-- ##### Requirements for "switch" statements ##### -->
<!-- "switch" statements MUST have a "default" clause -->
<module name="MissingSwitchDefault"/>
<!-- "case" clauses in switch statements MUST include break, return, throw or continue -->
<module name="FallThrough"/>

<!-- ##### Other / Miscellaneous requirements ##### -->
<!-- Require utility classes do not have a public constructor -->
<module name="HideUtilityClassConstructor"/>
<!-- Require each variable declaration is its own statement on its own line -->
<module name="MultipleVariableDeclarations"/>
<!-- Each line of code can only include one statement -->
<module name="OneStatementPerLine"/>
<!-- Require that "catch" statements are not empty (must at least contain a comment) -->
<module name="EmptyCatchBlock"/>
</module>
</module>
38 changes: 38 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,44 @@
<artifactId>maven-resources-plugin</artifactId>
<version>3.3.1</version>
</plugin>
<!-- Used to validate all code style rules in source code via the Checkstyle config in checkstyle.xml -->
<!-- Can be skipped by passing -Dcheckstyle.skip=true to Maven. -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-checkstyle-plugin</artifactId>
<version>3.3.1</version>
<executions>
<execution>
<id>verify-style</id>
<!-- Bind to verify so it runs after package & unit tests, but before install -->
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
<configuration>
<sourceDirectories>
<sourceDirectory>src/main/java</sourceDirectory>
</sourceDirectories>
<configLocation>checkstyle.xml</configLocation>
<logViolationsToConsole>true</logViolationsToConsole>
<failOnViolation>true</failOnViolation>
<!-- Enable checks on all test source files -->
<includeTestSourceDirectory>true</includeTestSourceDirectory>
<!-- Define our suppressions file location, and the key used to pass it to checkstyle.xml-->
<suppressionsLocation>checkstyle-suppressions.xml</suppressionsLocation>
<suppressionsFileExpression>checkstyle.suppressions.file</suppressionsFileExpression>
</configuration>
<dependencies>
<!-- Override dependencies to use latest version of checkstyle -->
<dependency>
<groupId>com.puppycrawl.tools</groupId>
<artifactId>checkstyle</artifactId>
<version>8.38</version>
</dependency>
</dependencies>
</plugin>
<!-- Used to generate a new release via Sonatype (see release profile). -->
<plugin>
<groupId>org.sonatype.plugins</groupId>
Expand Down
49 changes: 19 additions & 30 deletions src/main/java/org/dspace/ctask/replicate/AbstractPackagerTask.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@
* @see org.dspace.content.packager.PackageDisseminator
* @see org.dspace.content.packager.PackageIngester
*/
public abstract class AbstractPackagerTask extends AbstractCurationTask
{
public abstract class AbstractPackagerTask extends AbstractCurationTask {
// Name of recursive mode option configurable in curation task configuration file
private final String recursiveMode = "recursiveMode";

Expand Down Expand Up @@ -66,48 +65,38 @@ public abstract class AbstractPackagerTask extends AbstractCurationTask
* @return configured PackageParameters (or null, if configurations not found)
* @see org.dspace.content.packager.PackageParameters
*/
protected PackageParameters loadPackagerParameters(String moduleName)
{
//Load up the replicate-mets.cfg file & all settings inside it
protected PackageParameters loadPackagerParameters(String moduleName) {
// Load up the replicate-mets.cfg file & all settings inside it
List<String> moduleProps = configurationService.getPropertyKeys(moduleName);

PackageParameters pkgParams = new PackageParameters();

//If our config file doesn't load properly, we'll return null
if(moduleProps!=null)
{
//loop through all properties in the config file
for(String property : moduleProps)
{
//Set propertyName, removing leading module name (if applicable)
// If our config file doesn't load properly, we'll return null
if (moduleProps != null) {
// loop through all properties in the config file
for (String property : moduleProps) {
// Set propertyName, removing leading module name (if applicable)
String propertyName = property;
if(propertyName.startsWith(moduleName + ".")) {
if (propertyName.startsWith(moduleName + ".")) {
propertyName = propertyName.replaceFirst(moduleName + ".", "");
}

//Only obey the setting(s) beginning with this task's ID/name,
if(propertyName.startsWith(this.taskId))
{
//Parse out the option name by removing the "[taskID]." from beginning of property
// Only obey the setting(s) beginning with this task's ID/name,
if (propertyName.startsWith(this.taskId)) {
// Parse out the option name by removing the "[taskID]." from beginning of property
String option = propertyName.replace(taskId + ".", "");
String value = configurationService.getProperty(property);

//Check which option is being set
if(option.equalsIgnoreCase(recursiveMode))
{
// Check which option is being set
if (option.equalsIgnoreCase(recursiveMode)) {
pkgParams.setRecursiveModeEnabled(Boolean.parseBoolean(value));
}
else if (option.equals(useWorkflow))
{
} else if (option.equals(useWorkflow)) {
pkgParams.setWorkflowEnabled(Boolean.parseBoolean(value));
}
else if (option.equals(useCollectionTemplate))
{
} else if (option.equals(useCollectionTemplate)) {
pkgParams.setUseCollectionTemplate(Boolean.parseBoolean(value));
}
else //otherwise, assume the Packager will understand what to do with this option
{
//just set it as a property in PackageParameters
} else {
// otherwise, assume the Packager will understand what to do with this option
// just set it as a property in PackageParameters
pkgParams.addProperty(option, value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void init(Curator curator, String taskId) throws IOException {
archFmt = configurationService.getProperty("replicate.packer.archfmt");
storeGroupName = configurationService.getProperty("replicate.group.aip.name");
}

/**
* Perform the 'Replace with AIP' task.
* <P>
Expand All @@ -70,7 +70,7 @@ public void init(Curator curator, String taskId) throws IOException {
@Override
public int perform(DSpaceObject dso) throws IOException {
final ReplicaManager repMan = ReplicaManager.instance();

// overwrite with AIP data
try {
Context context = Curator.curationContext();
Expand Down
Loading

0 comments on commit 902bfca

Please sign in to comment.