Skip to content

Commit

Permalink
Feat/improve tag validation (#69)
Browse files Browse the repository at this point in the history 
* Do not look for problems in unsupported tags

* Clean up code in TagValueValidators

* Rename TagValueValidators::defaultValidator to requiredValueValidator

* Do not mark empty values in AddSongInfoDialog as invalid
  • Loading branch information
@Nianna
Nianna authored Oct 13, 2024
1 parent 896decf commit bcfb8cc
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 109 deletions.
40 changes: 17 additions & 23 deletions src/main/java/com/github/nianna/karedi/dialog/AddSongInfoDialog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,15 @@
import javafx.fxml.FXML;
import javafx.scene.control.ButtonType;
import javafx.scene.control.Control;
import javafx.scene.control.Dialog;
import javafx.scene.control.TextField;
import org.controlsfx.validation.ValidationMessage;
import org.controlsfx.validation.ValidationSupport;
import org.controlsfx.validation.ValidationResult;
import org.controlsfx.validation.Validator;

import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

public class AddSongInfoDialog extends Dialog<List<Tag>> {
public class AddSongInfoDialog extends ValidatedDialog<List<Tag>> {

@FXML
private ManageableGridPane gridPane;
Expand All @@ -43,8 +41,6 @@ public class AddSongInfoDialog extends Dialog<List<Tag>> {

private final FormatSpecification formatSpecification;

private ValidationSupport validationSupport = new ValidationSupport();

public AddSongInfoDialog(FormatSpecification formatSpecification) {
this.formatSpecification = formatSpecification;
setTitle(I18N.get("dialog.creator.add_info.title"));
Expand Down Expand Up @@ -105,29 +101,27 @@ private void registerValidators() {
}

private void registerValidator(Control control, TagKey key) {
validationSupport.registerValidator(control, TagValueValidators.forKey(key));
Validator<String> tagValidator = TagValueValidators.forKey(key);
Validator<String> allowEmptyInputWrapper = (c, value) ->
value.isEmpty() ? new ValidationResult() : tagValidator.apply(c, value);
validationSupport.registerValidator(control, allowEmptyInputWrapper);
}

private List<Tag> generateListOfValidTags() {
List<Tag> list = new ArrayList<>();
List<Control> invalidFields = validationSupport.getValidationResult().getErrors().stream()
.map(ValidationMessage::getTarget)
.distinct()
.collect(Collectors.toList());
addTag(TagKey.GAP, gapField, invalidFields).ifPresent(list::add);
addTag(TagKey.YEAR, yearField, invalidFields).ifPresent(list::add);
addTag(TagKey.LANGUAGE, languageField, invalidFields).ifPresent(list::add);
addTag(TagKey.CREATOR, creatorField, invalidFields).ifPresent(list::add);
addTag(TagKey.GENRE, genreField, invalidFields).ifPresent(list::add);
addTag(TagKey.TAGS, tagsField, invalidFields).ifPresent(list::add);
createTagIfDefined(TagKey.GAP, gapField).ifPresent(list::add);
createTagIfDefined(TagKey.YEAR, yearField).ifPresent(list::add);
createTagIfDefined(TagKey.LANGUAGE, languageField).ifPresent(list::add);
createTagIfDefined(TagKey.CREATOR, creatorField).ifPresent(list::add);
createTagIfDefined(TagKey.GENRE, genreField).ifPresent(list::add);
createTagIfDefined(TagKey.TAGS, tagsField).ifPresent(list::add);
return list;
}

private Optional<Tag> addTag(TagKey key, TextField field, List<Control> invalidControls) {
if (!invalidControls.contains(field)) {
return Optional.of(new Tag(key, field.getText()));
}
return Optional.empty();
private Optional<Tag> createTagIfDefined(TagKey key, TextField field) {
return Optional.of(field.getText())
.filter(value -> !value.isEmpty())
.map(value -> new Tag(key, value));
}

}
2 changes: 1 addition & 1 deletion src/main/java/com/github/nianna/karedi/dialog/EditFilenamesDialog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public void initialize() {
Platform.runLater(() -> {
validationSupport.registerValidator(titleField, TagValueValidators.forKey(TagKey.TITLE));
validationSupport.registerValidator(artistField, TagValueValidators.forKey(TagKey.ARTIST));
validationSupport.registerValidator(coverExtensionField, TagValueValidators.defaultValidator());
validationSupport.registerValidator(coverExtensionField, TagValueValidators.requiredValueValidator());
validationSupport.initInitialDecoration();
includeBackgroundCheckBox.setSelected(!hideBackground);
includeVideoCheckBox.setSelected(!hideVideo);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/github/nianna/karedi/dialog/EditTagDialog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class EditTagDialog extends Dialog<Tag> {

private ValidationDecoration valueDecoration = new GraphicValidationDecoration();
private ValidationDecoration keyDecoration = new GraphicValidationDecoration();
private Validator<String> valueValidator = TagValueValidators.defaultValidator();
private Validator<String> valueValidator = TagValueValidators.requiredValueValidator();
private Validator<String> keyValidator = Validator
.createEmptyValidator(I18N.get("dialog.tag.key_required"));

Expand Down
17 changes: 10 additions & 7 deletions src/main/java/com/github/nianna/karedi/song/DuplicatedTagsConsistencyValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@

import com.github.nianna.karedi.problem.InconsistentTagsProblem;
import com.github.nianna.karedi.problem.TagProblem;
import com.github.nianna.karedi.song.tag.FormatSpecification;
import com.github.nianna.karedi.song.tag.TagKey;

import java.util.Map;
import java.util.Optional;

import static java.util.Objects.nonNull;

public class DuplicatedTagsConsistencyValidator {

private static final Map<TagKey, TagKey> TAG_DUPLICATES = Map.of(
Expand All @@ -25,14 +24,18 @@ private DuplicatedTagsConsistencyValidator() {
}

public static Optional<TagProblem> validate(Song song, TagKey key) {
if (TAG_DUPLICATES.containsKey(key)) {
if (hasSupportedDuplicateKey(song, key)) {
String tagValue = song.getTagValue(key).orElseThrow();
String similarTagValue = song.getTagValue(TAG_DUPLICATES.get(key)).orElse(null);
if (nonNull(similarTagValue) && !tagValue.equals(similarTagValue)) {
return Optional.of(new InconsistentTagsProblem(key, TAG_DUPLICATES.get(key)));
}
return song.getTagValue(TAG_DUPLICATES.get(key))
.filter(similarTagValue -> !tagValue.equals(similarTagValue))
.map(ignored -> new InconsistentTagsProblem(key, TAG_DUPLICATES.get(key)));
}
return Optional.empty();
}

private static boolean hasSupportedDuplicateKey(Song song, TagKey key) {
return TAG_DUPLICATES.containsKey(key)
&& FormatSpecification.supports(song.getFormatSpecificationVersion(), TAG_DUPLICATES.get(key));
}

}
20 changes: 13 additions & 7 deletions src/main/java/com/github/nianna/karedi/song/SongChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@
import com.github.nianna.karedi.problem.ProblemsCombiner;
import com.github.nianna.karedi.problem.TagForNonexistentTrackProblem;
import com.github.nianna.karedi.problem.TagValidationErrorProblem;
import com.github.nianna.karedi.problem.UnsupportedTagProblem;
import com.github.nianna.karedi.song.tag.FormatSpecification;
import com.github.nianna.karedi.song.tag.MultiplayerTags;
import com.github.nianna.karedi.song.tag.Tag;
import com.github.nianna.karedi.song.tag.TagKey;
import com.github.nianna.karedi.song.tag.TagKeyValidator;
import com.github.nianna.karedi.song.tag.TagValueValidators;
import com.github.nianna.karedi.util.Converter;
import com.github.nianna.karedi.util.ListenersUtils;
Expand Down Expand Up @@ -92,10 +93,18 @@ public ObservableList<Problem> getProblems() {

private void refreshTag(Tag tag) {
removeTagProblems(tag);
if (FormatSpecification.supports(song.getFormatSpecificationVersion(), tag.getKey())) {
doValidateTag(tag);
} else {
combiner.add(new UnsupportedTagProblem(tag.getKey()));
}
}

private void doValidateTag(Tag tag) {
TagKey.optionalValueOf(tag.getKey()).ifPresent(tagKey -> {
validateValue(tagKey, tag.getValue());
performAdditionalValueValidation(tagKey);
DuplicatedTagsConsistencyValidator.validate(song, tagKey).ifPresent(combiner::add);
validateValue(tagKey, tag.getValue());
performAdditionalValueValidation(tagKey);
DuplicatedTagsConsistencyValidator.validate(song, tagKey).ifPresent(combiner::add);
});
validateTagKey(tag.getKey());
}
Expand All @@ -105,9 +114,6 @@ private void onTagRemoved(Tag tag) {
}

private void validateTagKey(String tagKey) {
song.formatSpecificationVersion()
.flatMap(formatVersion -> TagKeyValidator.validate(tagKey, formatVersion))
.ifPresent(combiner::add);
if (MultiplayerTags.isANameTagKey(tagKey)) {
MultiplayerTags.getTrackNumber(tagKey)
.filter(trackIndex -> trackIndex >= song.getTrackCount())
Expand Down
21 changes: 0 additions & 21 deletions src/main/java/com/github/nianna/karedi/song/tag/TagKeyValidator.java
View file

This file was deleted.

92 changes: 43 additions & 49 deletions src/main/java/com/github/nianna/karedi/song/tag/TagValueValidators.java
View file
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
package com.github.nianna.karedi.song.tag;

import java.time.Year;
import java.util.Optional;

import org.controlsfx.validation.ValidationResult;
import org.controlsfx.validation.Validator;

import javafx.scene.control.Control;
import com.github.nianna.karedi.I18N;
import com.github.nianna.karedi.util.Converter;
import com.github.nianna.karedi.util.ForbiddenCharacterRegex;
import com.github.nianna.karedi.util.StringValidators;
import javafx.scene.control.Control;
import org.controlsfx.validation.ValidationResult;
import org.controlsfx.validation.Validator;

import java.time.Year;
import java.util.Optional;

public class TagValueValidators {

private TagValueValidators() {
}

public static Validator<String> forKey(String key) {
return TagKey.optionalValueOf(key).map(TagValueValidators::forKey).orElse(defaultValidator());
return TagKey.optionalValueOf(key).map(TagValueValidators::forKey).orElse(requiredValueValidator());
}

public static Validator<String> forKey(TagKey key) {
Expand All @@ -28,30 +27,10 @@ public static Validator<String> forKey(TagKey key) {
};
}

private static Validator<String> specificForKey(TagKey key) {
switch (key) {
case BPM:
return TagValueValidators::bpmValidator;
case YEAR:
return TagValueValidators::yearValidator;
default:
return defaultValidator();
}
}

public static Validator<String> defaultValidator() {
public static Validator<String> requiredValueValidator() {
return Validator.createEmptyValidator(I18N.get("validator.tag.value_required"));
}

private static boolean canBeNegative(TagKey key) {
switch (key) {
case VIDEOGAP:
return true;
default:
return false;
}
}

public static Optional<String> forbiddenCharacterRegex(TagKey key) {
if (TagKey.expectsAFileName(key)) {
return Optional.of(ForbiddenCharacterRegex.FOR_FILENAME);
Expand All @@ -77,6 +56,26 @@ public static Optional<String> forbiddenCharacterRegex(String key) {
return TagKey.optionalValueOf(key).flatMap(TagValueValidators::forbiddenCharacterRegex);
}

public static ValidationResult validate(TagKey key, String value) {
return forKey(key).apply(null, value);
}

public static boolean hasValidationErrors(TagKey key, String value) {
return !validate(key, value).getErrors().isEmpty();
}

private static Validator<String> specificForKey(TagKey key) {
return switch (key) {
case BPM -> TagValueValidators::bpmValidator;
case YEAR -> TagValueValidators::yearValidator;
default -> requiredValueValidator();
};
}

private static boolean canBeNegative(TagKey key) {
return key == TagKey.VIDEOGAP;
}

private static Validator<String> legalInputValidator(TagKey key) {
if (TagKey.expectsAnInteger(key)) {
if (canBeNegative(key)) {
Expand Down Expand Up @@ -105,30 +104,25 @@ private static Validator<String> legalInputValidator(TagKey key) {
}

private static ValidationResult bpmValidator(Control c, String newValue) {
ValidationResult result = ValidationResult.fromWarningIf(c, I18N.get("validator.tag.bpm_range"), Converter
.toDouble(newValue).map(value -> (value < 200 || value > 400)).orElse(false));
return result;
return ValidationResult.fromWarningIf(
c,
I18N.get("validator.tag.bpm_range"),
Converter
.toDouble(newValue)
.map(value -> (value < 200 || value > 400))
.orElse(false)
);
}

private static ValidationResult yearValidator(Control c, String newValue) {
int currentYear = Year.now().getValue();
ValidationResult result = ValidationResult.fromWarningIf(c, I18N.get("validator.tag.year.too_big"),
Converter.toInteger(newValue).map(value -> {
return value > currentYear;
}).orElse(false));
return result;
}

public static ValidationResult validate(TagKey key, String value) {
return forKey(key).apply(null, value);
}

public static boolean hasValidationErrors(TagKey key, String value) {
return validate(key, value).getErrors().size() > 0;
}

public static boolean hasValidationWarnings(TagKey key, String value) {
return validate(key, value).getWarnings().size() > 0;
return ValidationResult.fromWarningIf(
c,
I18N.get("validator.tag.year.too_big"),
Converter.toInteger(newValue)
.map(value -> value > currentYear)
.orElse(false)
);
}

}

0 comments on commit bcfb8cc

Please sign in to comment.