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.

Sign up for GitHub

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

Formatter refactoring #3919

Merged
merged 4 commits into from
Apr 4, 2018
Merged

Formatter refactoring #3919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
84da698
Refactor RegEx Formatter
tobiasdiez Apr 3, 2018
5014dcf
Fix formatting
tobiasdiez Apr 3, 2018
7eafad2
Reorder return statements in `getFormatterForModifier`
tobiasdiez Apr 3, 2018
f541d05
Fix order in RegexFormatter
tobiasdiez Apr 3, 2018
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
104 changes: 40 additions & 64 deletions src/main/java/org/jabref/logic/formatter/Formatters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,91 +31,67 @@

public class Formatters {

private static final List<Formatter> CONVERTERS = Arrays.asList(
new HtmlToLatexFormatter(),
new HtmlToUnicodeFormatter(),
new LatexToUnicodeFormatter(),
new UnicodeToLatexFormatter()
);

private static final List<Formatter> CASE_CHANGERS = Arrays.asList(
new CapitalizeFormatter(),
new LowerCaseFormatter(),
new SentenceCaseFormatter(),
new TitleCaseFormatter(),
new UpperCaseFormatter()
);

private static final List<Formatter> OTHERS = Arrays.asList(
new ClearFormatter(),
new LatexCleanupFormatter(),
new MinifyNameListFormatter(),
new NormalizeDateFormatter(),
new NormalizeMonthFormatter(),
new NormalizeNamesFormatter(),
new NormalizePagesFormatter(),
new OrdinalsToSuperscriptFormatter(),
new RegexFormatter(),
new RemoveBracesFormatter(),
new UnitsToLatexFormatter(),
new EscapeUnderscoresFormatter()
);

private static final String REGEX = "regex";

private static final int LENGTH_OF_REGEX_PREFIX = REGEX.length();

private Formatters() {
}

public static final List<Formatter> getConverters() {
List<Formatter> converters = new ArrayList<>();
converters.addAll(CONVERTERS);
return converters;
public static List<Formatter> getConverters() {
return Arrays.asList(
new HtmlToLatexFormatter(),
new HtmlToUnicodeFormatter(),
new LatexToUnicodeFormatter(),
new UnicodeToLatexFormatter()
);
}

public static final List<Formatter> getCaseChangers() {
List<Formatter> caseChangers = new ArrayList<>();
caseChangers.addAll(CASE_CHANGERS);
return caseChangers;
public static List<Formatter> getCaseChangers() {
return Arrays.asList(
new CapitalizeFormatter(),
new LowerCaseFormatter(),
new SentenceCaseFormatter(),
new TitleCaseFormatter(),
new UpperCaseFormatter()
);
}

public static final List<Formatter> getOthers() {
List<Formatter> others = new ArrayList<>();
others.addAll(OTHERS);
return others;
public static List<Formatter> getOthers() {
return Arrays.asList(
new ClearFormatter(),
new LatexCleanupFormatter(),
new MinifyNameListFormatter(),
new NormalizeDateFormatter(),
new NormalizeMonthFormatter(),
new NormalizeNamesFormatter(),
new NormalizePagesFormatter(),
new OrdinalsToSuperscriptFormatter(),
new RemoveBracesFormatter(),
new UnitsToLatexFormatter(),
new EscapeUnderscoresFormatter()
);
}

public static final List<Formatter> getAll() {
public static List<Formatter> getAll() {
List<Formatter> all = new ArrayList<>();
all.addAll(CONVERTERS);
all.addAll(CASE_CHANGERS);
all.addAll(OTHERS);
all.addAll(getConverters());
all.addAll(getCaseChangers());
all.addAll(getOthers());
return all;
}

public static Optional<Formatter> getFormatterForModifier(String modifier) {
Objects.requireNonNull(modifier);
Optional<Formatter> formatter;
List<Formatter> all = getAll();

if (modifier.matches("regex.*")) {
String regex = modifier.substring(LENGTH_OF_REGEX_PREFIX);
RegexFormatter.setRegex(regex);
formatter = all.stream().filter(f -> f.getKey().equals("regex")).findAny();
} else {
formatter = all.stream().filter(f -> f.getKey().equals(modifier)).findAny();
}
if (formatter.isPresent()) {
return formatter;
}
switch (modifier) {
case "lower":
return Optional.of(new LowerCaseFormatter());
case "upper":
return Optional.of(new UpperCaseFormatter());
default:
return Optional.empty();
}

if (modifier.startsWith(RegexFormatter.KEY)) {
String regex = modifier.substring(RegexFormatter.KEY.length());
return Optional.of(new RegexFormatter(regex));
} else {
return getAll().stream().filter(f -> f.getKey().equals(modifier)).findAny();
}
}

Expand Down
41 changes: 18 additions & 23 deletions src/main/java/org/jabref/logic/formatter/bibtexfields/RegexFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,36 @@

public class RegexFormatter implements Formatter {

public static final String KEY = "regex";
private static final Pattern PATTERN_ESCAPED_OPENING_CURLY_BRACE = Pattern.compile("\\\\\\{");

private static final Pattern PATTERN_ESCAPED_CLOSING_CURLY_BRACE = Pattern.compile("\\\\\\}");

// RegEx to match {...}
// \\ is required to have the { interpreted as character
// ? is required to disable the aggressive match
private static final Pattern PATTERN_ENCLOSED_IN_CURLY_BRACES = Pattern.compile("(\\{.*?})");

// Magic arbitrary unicode char, which will never appear in bibtex files
private static final String PLACEHOLDER_FOR_PROTECTED_GROUP = Character.toString('\u0A14');

private static final String PLACEHOLDER_FOR_OPENING_CURLY_BRACE = Character.toString('\u0A15');

private static final String PLACEHOLDER_FOR_CLOSING_CURLY_BRACE = Character.toString('\u0A16');

private static final String QUOTE_AND_OPENING_BRACE = "\"(";

private static final int LENGTH_OF_QUOTE_AND_OPENING_BRACE = QUOTE_AND_OPENING_BRACE.length();

private static final String CLOSING_BRACE_AND_QUOTE = ")\"";

private static final int LENGTH_OF_CLOSING_BRACE_AND_QUOTE = CLOSING_BRACE_AND_QUOTE.length();

// stores the regex set by setRegex
private static String[] regex;
private String regex;
private String replacement;

/**
* Constructs a new regular expression-based formatter with the given RegEx.
*
* @param input the regular expressions for matching and replacing given in the form {@code (<regex>, <replace>)}.
*/
public RegexFormatter(String input) {
// formatting is like ("exp1","exp2"), we want to first remove (" and ")
String rexToSet = input.substring(LENGTH_OF_QUOTE_AND_OPENING_BRACE, input.length() - LENGTH_OF_CLOSING_BRACE_AND_QUOTE);
String[] parts = rexToSet.split("\",\"");
regex = parts[0];
replacement = parts[1];
}

@Override
public String getName() {
Expand All @@ -45,7 +49,7 @@ public String getName() {

@Override
public String getKey() {
return "regex";
return KEY;
}

private String replaceHonoringProtectedGroups(final String input) {
Expand All @@ -56,7 +60,7 @@ private String replaceHonoringProtectedGroups(final String input) {
replaced.add(matcher.group(1));
}
String workingString = matcher.replaceAll(PLACEHOLDER_FOR_PROTECTED_GROUP);
workingString = workingString.replaceAll(RegexFormatter.regex[0], RegexFormatter.regex[1]);
workingString = workingString.replaceAll(regex, replacement);

for (String r : replaced) {
workingString = workingString.replaceFirst(PLACEHOLDER_FOR_PROTECTED_GROUP, r);
Expand Down Expand Up @@ -93,13 +97,4 @@ public String getDescription() {
public String getExampleInput() {
return "Please replace the spaces";
}

public static void setRegex(String rex) {
// formatting is like ("exp1","exp2"), we want to remove (" and ")
String rexToSet = rex;
rexToSet = rexToSet.substring(LENGTH_OF_QUOTE_AND_OPENING_BRACE, rexToSet.length() - LENGTH_OF_CLOSING_BRACE_AND_QUOTE);
String[] parts = rexToSet.split("\",\"");
regex = parts;
}

}
83 changes: 40 additions & 43 deletions src/test/java/org/jabref/logic/formatter/FormatterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,102 +38,99 @@
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class FormatterTest {
class FormatterTest {

private static ProtectedTermsLoader protectedTermsLoader;

@BeforeAll
public static void setUp() {
static void setUp() {
protectedTermsLoader = new ProtectedTermsLoader(
new ProtectedTermsPreferences(ProtectedTermsLoader.getInternalLists(), Collections.emptyList(),
Collections.emptyList(), Collections.emptyList()));

}

private static Stream<Formatter> getFormatters() {
// all classes implementing {@link net.sf.jabref.model.cleanup.Formatter}
// sorted alphabetically
// Alternative: Use reflection - https://github.com/ronmamo/reflections
return Stream.of(
new CapitalizeFormatter(),
new ClearFormatter(),
new HtmlToLatexFormatter(),
new HtmlToUnicodeFormatter(),
new IdentityFormatter(),
new LatexCleanupFormatter(),
new LatexToUnicodeFormatter(),
new LowerCaseFormatter(),
new MinifyNameListFormatter(),
new NormalizeDateFormatter(),
new NormalizeMonthFormatter(),
new NormalizeNamesFormatter(),
new NormalizePagesFormatter(),
new OrdinalsToSuperscriptFormatter(),
new ProtectTermsFormatter(protectedTermsLoader),
new RegexFormatter("(\" \",\"-\")"),
new RemoveBracesFormatter(),
new SentenceCaseFormatter(),
new TitleCaseFormatter(),
new UnicodeToLatexFormatter(),
new UnitsToLatexFormatter(),
new UpperCaseFormatter()
);
}

@ParameterizedTest
@MethodSource("getFormatters")
public void getNameReturnsNotNull(Formatter formatter) {
void getNameReturnsNotNull(Formatter formatter) {
Copy link
Member

Choose a reason for hiding this comment

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

Why do you remove the public?
Codacy or whatever will complain that you should always use explicit scoping modifiers. I see no harm in using public here. I don't know if this even has to be public to be accessible from gradle

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the new convention with JUnit 5 that test classes and their methods are package private. IntellJ even complains if they are marked with public.

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay, that was new to me

assertNotNull(formatter.getName());
}

@ParameterizedTest
@MethodSource("getFormatters")
public void getNameReturnsNotEmpty(Formatter formatter) {
void getNameReturnsNotEmpty(Formatter formatter) {
assertNotEquals("", formatter.getName());
}

@ParameterizedTest
@MethodSource("getFormatters")
public void getKeyReturnsNotNull(Formatter formatter) {
void getKeyReturnsNotNull(Formatter formatter) {
assertNotNull(formatter.getKey());
}

@ParameterizedTest
@MethodSource("getFormatters")
public void getKeyReturnsNotEmpty(Formatter formatter) {
void getKeyReturnsNotEmpty(Formatter formatter) {
assertNotEquals("", formatter.getKey());
}

@ParameterizedTest
@MethodSource("getFormatters")
public void formatOfNullThrowsException(Formatter formatter) {
void formatOfNullThrowsException(Formatter formatter) {
assertThrows(NullPointerException.class, () -> formatter.format(null));
}

@ParameterizedTest
@MethodSource("getFormatters")
public void formatOfEmptyStringReturnsEmpty(Formatter formatter) {
void formatOfEmptyStringReturnsEmpty(Formatter formatter) {
assertEquals("", formatter.format(""));
}

@ParameterizedTest
@MethodSource("getFormatters")
public void formatNotReturnsNull(Formatter formatter) {
void formatNotReturnsNull(Formatter formatter) {
assertNotNull(formatter.format("string"));
}

@ParameterizedTest
@MethodSource("getFormatters")
public void getDescriptionAlwaysNonEmpty(Formatter formatter) {
void getDescriptionAlwaysNonEmpty(Formatter formatter) {
assertFalse(formatter.getDescription().isEmpty());
}

@ParameterizedTest
@MethodSource("getFormatters")
public void getExampleInputAlwaysNonEmpty(Formatter formatter) {
void getExampleInputAlwaysNonEmpty(Formatter formatter) {
assertFalse(formatter.getExampleInput().isEmpty());
}

public static Stream<Formatter> getFormatters() {
// all classes implementing {@link net.sf.jabref.model.cleanup.Formatter}
// sorted alphabetically
// Alternative: Use reflection - https://github.com/ronmamo/reflections
// @formatter:off
return Stream.of(
new CapitalizeFormatter(),
new ClearFormatter(),
new HtmlToLatexFormatter(),
new HtmlToUnicodeFormatter(),
new IdentityFormatter(),
new LatexCleanupFormatter(),
new LatexToUnicodeFormatter(),
new LowerCaseFormatter(),
new MinifyNameListFormatter(),
new NormalizeDateFormatter(),
new NormalizeMonthFormatter(),
new NormalizeNamesFormatter(),
new NormalizePagesFormatter(),
new OrdinalsToSuperscriptFormatter(),
new ProtectTermsFormatter(protectedTermsLoader),
new RegexFormatter(),
new RemoveBracesFormatter(),
new SentenceCaseFormatter(),
new TitleCaseFormatter(),
new UnicodeToLatexFormatter(),
new UnitsToLatexFormatter(),
new UpperCaseFormatter());

// @formatter:on
}
}
Loading