Skip to content

Commit

Permalink
312 WIP Improve refactoring and tests
Browse files Browse the repository at this point in the history
- Fix some small bugs (which even existed in the Groovy implementation)
- Make central Configuration class a (lombok) Bean with only getters and a builder.
  All configuration items are now ordinary (typed) fields.
  It only contains very few setter like logic any longer (we should get rid
  of those methods as well and treat a configuration immutable in the future.
  Nevertheless, we could drop some tests regarding configuration contents,
  overriding etc.
- Fix some Groovy->Java translations, e.g., '==' for Strings -> 'equals'
- Fix some internal types, in particular,
  - using interfaces vs. implementations,
  - using Sets or Lists instead of Collections to gain more specific behavior.
- Fix warnings, e.g., remove unnecessary public modifier of test methods
- Align implementation and test packages
- Fix some typos/wording
- Align some names with Java conventions (e.g., for constants)
- Simplify by inlining some code fragments
- Handle IP address validation by Apache commons-validator
  • Loading branch information
ascheman committed Jan 31, 2024
1 parent bb37a50 commit 5bb15ff
Show file tree
Hide file tree
Showing 46 changed files with 667 additions and 1,018 deletions.
2 changes: 2 additions & 0 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ groovy-version = "3.0.17"
junit5-version = "5.10.1"

[libraries]
commons-validator = "commons-validator:commons-validator:1.8.0"
# Use Groovy which corresponds to Gradle version (of hsc build)
groovy-bom = { module = "org.codehaus.groovy:groovy-bom", version.ref = "groovy-version" }
jsoup = "org.jsoup:jsoup:1.17.2"
junit-vintage = { module = "org.junit.vintage:junit-vintage-engine", version.ref = "junit5-version" }
lombok = "org.projectlombok:lombok:1.18.30"
slf4j-api = "org.slf4j:slf4j-api:2.0.11"
slf4j-nop = "org.slf4j:slf4j-nop:2.0.11"
spock = "org.spockframework:spock-bom:2.3-groovy-3.0"
Expand Down
12 changes: 10 additions & 2 deletions htmlSanityCheck-core/build.gradle
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
import org.apache.tools.ant.filters.ReplaceTokens

dependencies {
implementation (libs.commons.validator) {
// Having a vulnerability here ...
exclude group: 'commons-collections', module: 'commons-collections'
}
implementation libs.slf4j.api
testImplementation libs.slf4j.nop
// jsoup is our awesome html parser, see jsoup.org
implementation libs.jsoup

compileOnly libs.lombok
annotationProcessor libs.lombok
testCompileOnly libs.lombok
testAnnotationProcessor libs.lombok

implementation 'org.codehaus.groovy:groovy-xml'
// Since Groovy 2.5 it is necessary to import dateutil separately
implementation 'org.codehaus.groovy:groovy-dateutil'
}

publishing {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import java.nio.file.Path
class FileUtil {


static File commonPath(Collection<File> files) {
static File commonPath(Set<File> files) {
if (!files) {
return null
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import org.slf4j.LoggerFactory
class AllChecksRunner {

// we check a collection of files:
private Collection<File> filesToCheck = new HashSet<File>()
private Set<File> filesToCheck = new HashSet<File>()

// where do we put our results
private File checkingResultsDir
Expand All @@ -45,7 +45,7 @@ class AllChecksRunner {
boolean consoleReport = true

// checker instances
private Set<Checker> checkers
private List<Checker> checkers

// keep all results
private PerRunResults resultsForAllPages
Expand All @@ -55,29 +55,29 @@ class AllChecksRunner {
private Configuration myConfig


private static final Logger logger = LoggerFactory.getLogger(AllChecksRunner.class);
private static final Logger logger = LoggerFactory.getLogger(AllChecksRunner.class)

/**
* runs all available checks
*
*/

public AllChecksRunner( Configuration pConfig ) {
AllChecksRunner( Configuration pConfig ) {
super()

myConfig = pConfig

this.filesToCheck = myConfig.getConfigItemByName(Configuration.ITEM_NAME_sourceDocuments)
this.filesToCheck = myConfig.getSourceDocuments()

// TODO: #185 (checker classes shall be detected automatically (aka CheckerFactory)
// CheckerFactory needs the configuration
Set<Class> checkerClasses = myConfig.getConfigItemByName(Configuration.ITEM_NAME_checksToExecute)
List<Class> checkerClasses = myConfig.getChecksToExecute()
this.checkers = CheckerCreator.createCheckerClassesFrom( checkerClasses, myConfig )

this.resultsForAllPages = new PerRunResults()

this.checkingResultsDir = myConfig.getConfigItemByName(Configuration.ITEM_NAME_checkingResultsDir)
this.junitResultsDir = myConfig.getConfigItemByName(Configuration.ITEM_NAME_junitResultsDir)
this.checkingResultsDir = myConfig.getCheckingResultsDir()
this.junitResultsDir = myConfig.getJunitResultsDir()

logger.debug("AllChecksRunner created with ${this.checkers.size()} checkers for ${filesToCheck.size()} files")
}
Expand All @@ -88,7 +88,7 @@ class AllChecksRunner {
* performs all available checks on pageToCheck
*
*/
public PerRunResults performAllChecks() {
PerRunResults performAllChecks() {

logger.debug "entered performAllChecks"

Expand Down Expand Up @@ -191,7 +191,7 @@ class AllChecksRunner {
* line with default settings...
* @param args
*/
public static void main(String[] args) {
static void main(String[] args) {
// TODO: read parameter from command line

// empty method is currently marked "prio-2" issue by CodeNarc... we'll ignore that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
package net.ricecode.similarity;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;

Expand Down Expand Up @@ -57,7 +56,7 @@ public StringSimilarityServiceImpl(SimilarityStrategy strategy) {
* @return A list of similarity scores.
*/
public List<SimilarityScore> scoreAll(List<String> features, String target) {
ArrayList<SimilarityScore> scores = new ArrayList<SimilarityScore>();
List<SimilarityScore> scores = new ArrayList<>();

for (String feature : features) {
double score = strategy.score(feature, target);
Expand Down Expand Up @@ -99,11 +98,11 @@ public SimilarityScore findTop(List<String> features, String target) {
* @return A SimilarityScore that has the top value amongst the features, according to the comparator.
*/
public SimilarityScore findTop(List<String> features, String target, Comparator<SimilarityScore> comparator) {
if (features.size() == 0) {
if (features.isEmpty()) {
return null;
}
List<SimilarityScore> scores = scoreAll(features, target);
Collections.sort(scores, comparator);
scores.sort(comparator);
return scores.get(0);
}

Expand All @@ -119,11 +118,11 @@ public SimilarityScore findTop(List<String> features, String target, Comparator<
* according to the comparator
*/
public List<SimilarityScore> findBestN(List<String> features, String target, int n) {
List<SimilarityScore> result = new ArrayList<SimilarityScore>();
List<SimilarityScore> result = new ArrayList<>();

if ((features.size() > 0) && (n >= 1)) {
if ((!features.isEmpty()) && (n >= 1)) {
List<SimilarityScore> scores = scoreAll(features, target);
Collections.sort(scores, new DescendingSimilarityScoreComparator());
scores.sort(new DescendingSimilarityScoreComparator());

// fails if n> scores.size(): result = scores.subList(0, n);
result = scores.subList(0, Math.min(scores.size(), n));
Expand Down
Loading

0 comments on commit 5bb15ff

Please sign in to comment.