From 3bf4473f9dfcbd70e09df706ecb3c3ab80fb9a87 Mon Sep 17 00:00:00 2001 From: Jan-Peter Klein Date: Fri, 16 Jun 2023 11:13:24 +0200 Subject: [PATCH] implemented changes mentioned by overheadhunter --- .../cryptomator/ui/error/ErrorController.java | 84 +++++++----------- .../ui/error/ErrorControllerTest.java | 88 +++++++++++-------- 2 files changed, 84 insertions(+), 88 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/error/ErrorController.java b/src/main/java/org/cryptomator/ui/error/ErrorController.java index ae006caee..1fcc1e829 100644 --- a/src/main/java/org/cryptomator/ui/error/ErrorController.java +++ b/src/main/java/org/cryptomator/ui/error/ErrorController.java @@ -11,8 +11,11 @@ import javax.inject.Inject; import javax.inject.Named; import javafx.application.Application; import javafx.application.Platform; +import javafx.beans.binding.BooleanExpression; import javafx.beans.property.BooleanProperty; +import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleBooleanProperty; +import javafx.beans.property.SimpleObjectProperty; import javafx.fxml.FXML; import javafx.scene.Scene; import javafx.scene.input.Clipboard; @@ -60,9 +63,9 @@ public class ErrorController implements FxController { private final Environment environment; private final BooleanProperty copiedDetails = new SimpleBooleanProperty(); - private final BooleanProperty errorSolutionFound = new SimpleBooleanProperty(); + private final ObjectProperty matchingErrorDiscussion = new SimpleObjectProperty(); + private final BooleanExpression errorSolutionFound = matchingErrorDiscussion.isNotNull(); private final BooleanProperty isLoadingHttpResponse = new SimpleBooleanProperty(); - private ErrorDiscussion matchingErrorDiscussion; @Inject ErrorController(Application application, @Named("stackTrace") String stackTrace, ErrorCode errorCode, @Nullable Scene previousScene, Stage window, Environment environment, ExecutorService executorService) { @@ -97,8 +100,9 @@ public class ErrorController implements FxController { @FXML public void showSolution() { - if (matchingErrorDiscussion != null) { - application.getHostServices().showDocument(matchingErrorDiscussion.url); + if (matchingErrorDiscussion.isNotNull().get()) { + ErrorDiscussion ed = (ErrorDiscussion) matchingErrorDiscussion.get(); + application.getHostServices().showDocument(ed.url); } } @@ -137,31 +141,30 @@ public class ErrorController implements FxController { new TypeToken>() { }.getType()); - if (errorDiscussionMap.values().stream().anyMatch(this::isPartialMatchFilter)) { - Comparator comp = this::compareExactMatch; - Optional value = errorDiscussionMap.values().stream().min(comp// - .thenComparing(this::compareSecondLevelMatch)// - .thenComparing(this::compareThirdLevelMatch)// - .thenComparing(this::compareIsAnswered)// - .thenComparing(this::compareUpvoteCount)); + if (errorDiscussionMap.values().stream().anyMatch(this::containsMethodCode)) { + Comparator comp = this::compareByFullErrorCode; + Optional value = errorDiscussionMap.values().stream().filter(this::containsMethodCode)// + .min(comp// + .thenComparing(this::compareByRootCauseCode)// + .thenComparing(this::compareIsAnswered)// + .thenComparing(this::compareUpvoteCount)); if (value.isPresent()) { - matchingErrorDiscussion = value.get(); - errorSolutionFound.set(true); + matchingErrorDiscussion.set(value.get()); } } } } /** - * Checks if an ErrorDiscussion object is a partial match based on the presence of the error code's method code in its title. + * Checks if an ErrorDiscussion object's title contains the error code's method code. * * @param errorDiscussion The ErrorDiscussion object to be checked. - * @return A boolean value indicating if the ErrorDiscussion object is a partial match: - * - true if the object's title contains the error code's method code, + * @return A boolean value indicating if the ErrorDiscussion object's title contains the error code's method code: + * - true if the title contains the method code, * - false otherwise. */ - public boolean isPartialMatchFilter(ErrorDiscussion errorDiscussion) { + public boolean containsMethodCode(ErrorDiscussion errorDiscussion) { return errorDiscussion.title.contains(" " + errorCode.methodCode()); } @@ -200,16 +203,16 @@ public class ErrorController implements FxController { } /** - * Compares two ErrorDiscussion objects based on the presence of an exact match with the error code in their titles and returns the result. + * Compares two ErrorDiscussion objects based on the presence of the full error code in their titles and returns the result. * * @param ed1 The first ErrorDiscussion object. * @param ed2 The second ErrorDiscussion object. - * @return An integer indicating the comparison result based on the presence of an exact match with the error code in the titles: - * - A negative value (-1) if ed1 has an exact match with the error code in the title and ed2 does not have a match, - * - A positive value (1) if ed1 does not have a match and ed2 has an exact match with the error code in the title, - * - Or 0 if both ErrorDiscussion objects either have an exact match or do not have a match with the error code in the titles. + * @return An integer indicating the comparison result based on the presence of the full error code in the titles: + * - A negative value (-1) if ed1 contains the full error code in the title and ed2 does not have a match, + * - A positive value (1) if ed1 does not have a match and ed2 contains the full error code in the title, + * - Or 0 if both ErrorDiscussion objects either contain the full error code or do not have a match in the titles. */ - public int compareExactMatch(ErrorDiscussion ed1, ErrorDiscussion ed2) { + public int compareByFullErrorCode(ErrorDiscussion ed1, ErrorDiscussion ed2) { if (ed1.title.contains(getErrorCode()) && !ed2.title.contains(getErrorCode())) { return -1; } else if (!ed1.title.contains(getErrorCode()) && ed2.title.contains(getErrorCode())) { @@ -220,16 +223,16 @@ public class ErrorController implements FxController { } /** - * Compares two ErrorDiscussion objects based on the presence of a second-level match with the error code in their titles and returns the result. + * Compares two ErrorDiscussion objects based on the presence of the root cause code in their titles and returns the result. * * @param ed1 The first ErrorDiscussion object. * @param ed2 The second ErrorDiscussion object. - * @return An integer indicating the comparison result based on the presence of a second-level match with the error code in the titles: - * - A negative value (-1) if ed1 has a second-level match with the error code in the title and ed2 does not have a match, - * - A positive value (1) if ed1 does not have a match and ed2 has a second-level match with the error code in the title, - * - Or 0 if both ErrorDiscussion objects either have a second-level match or do not have a match with the error code in the titles. + * @return An integer indicating the comparison result based on the presence of the root cause code in the titles: + * - A negative value (-1) if ed1 contains the root cause code in the title and ed2 does not have a match, + * - A positive value (1) if ed1 does not have a match and ed2 contains the root cause code in the title, + * - Or 0 if both ErrorDiscussion objects either contain the root cause code or do not have a match in the titles. */ - public int compareSecondLevelMatch(ErrorDiscussion ed1, ErrorDiscussion ed2) { + public int compareByRootCauseCode(ErrorDiscussion ed1, ErrorDiscussion ed2) { String value = " " + errorCode.methodCode() + ErrorCode.DELIM + errorCode.rootCauseCode(); if (ed1.title.contains(value) && !ed2.title.contains(value)) { return -1; @@ -240,27 +243,6 @@ public class ErrorController implements FxController { } } - /** - * Compares two ErrorDiscussion objects based on the presence of a third-level match with the error code in their titles and returns the result. - * - * @param ed1 The first ErrorDiscussion object. - * @param ed2 The second ErrorDiscussion object. - * @return An integer indicating the comparison result based on the presence of a third-level match with the error code in the titles: - * - A negative value (-1) if ed1 has a third-level match with the error code in the title and ed2 does not have a match, - * - A positive value (1) if ed1 does not have a match and ed2 has a third-level match with the error code in the title, - * - Or 0 if both ErrorDiscussion objects either have a third-level match or do not have a match with the error code in the titles. - */ - public int compareThirdLevelMatch(ErrorDiscussion ed1, ErrorDiscussion ed2) { - String value = " " + errorCode.methodCode(); - if (ed1.title.contains(value) && !ed2.title.contains(value)) { - return -1; - } else if (!ed1.title.contains(value) && ed2.title.contains(value)) { - return 1; - } else { - return 0; - } - } - /* Getter/Setter */ public boolean isPreviousScenePresent() { return previousScene != null; @@ -286,7 +268,7 @@ public class ErrorController implements FxController { return copiedDetails.get(); } - public BooleanProperty errorSolutionFoundProperty() { + public BooleanExpression errorSolutionFoundProperty() { return errorSolutionFound; } diff --git a/src/test/java/org/cryptomator/ui/error/ErrorControllerTest.java b/src/test/java/org/cryptomator/ui/error/ErrorControllerTest.java index 6c4acb789..ae15d5d59 100644 --- a/src/test/java/org/cryptomator/ui/error/ErrorControllerTest.java +++ b/src/test/java/org/cryptomator/ui/error/ErrorControllerTest.java @@ -4,6 +4,7 @@ import org.cryptomator.common.Environment; import org.cryptomator.common.ErrorCode; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mockito; @@ -44,88 +45,101 @@ class ErrorControllerTest { return ed; } + @DisplayName("compare error discussions by upvote count") @ParameterizedTest @CsvSource(textBlock = """ - 10, 5, -1 - 8, 15, 1 - 10, 10, 0 + 10, <, 5 + 8, >, 15 + 10, =, 10 """) - public void testCompareUpvoteCount(int leftUpvoteCount, int rightUpvoteCount, int expectedResult) { + public void testCompareUpvoteCount(int leftUpvoteCount, char expected, int rightUpvoteCount) { + int expectedResult = switch (expected) { + case '<' -> -1; + case '>' -> +1; + default -> 0; + }; var left = createErrorDiscussion("", leftUpvoteCount, null); var right = createErrorDiscussion("", rightUpvoteCount, null); int result = errorController.compareUpvoteCount(left, right); Assertions.assertEquals(expectedResult, Integer.signum(result)); } + @DisplayName("compare error discussions by existence of an answer") @ParameterizedTest @CsvSource(textBlock = """ - false, false, 0 - true, true, 0 - true, false, -1 - false, true, 1 + false, =, false + true, =, true + true, <, false + false, >, true """) - public void testCompareIsAnswered(boolean leftIsAnswered, boolean rightIsAnswered, int expectedResult) { + public void testCompareIsAnswered(boolean leftIsAnswered, char expected, boolean rightIsAnswered) { var answer = new ErrorDiscussion.Answer(); + int expectedResult = switch (expected) { + case '<' -> -1; + case '>' -> +1; + default -> 0; + }; var left = createErrorDiscussion("", 0, leftIsAnswered ? answer : null); var right = createErrorDiscussion("", 0, rightIsAnswered ? answer : null); int result = errorController.compareIsAnswered(left, right); Assertions.assertEquals(expectedResult, result); } + @DisplayName("compare error codes by full error code") @ParameterizedTest @CsvSource(textBlock = """ - Error 0000:0000:0000, Error 0000:0000:0000, 0 - Error 6HU1:12H1:HU7J, Error 0000:0000:0000, -1 - Error 0000:0000:0000, Error 6HU1:12H1:HU7J, 1 + Error 0000:0000:0000, =, Error 0000:0000:0000 + Error 6HU1:12H1:HU7J, <, Error 0000:0000:0000 + Error 0000:0000:0000, >, Error 6HU1:12H1:HU7J """) - public void testCompareExactMatch(String leftTitle, String rightTitle, int expectedResult) { + public void testCompareByFullErrorCode(String leftTitle, char expected, String rightTitle) { Mockito.when(errorCode.toString()).thenReturn("6HU1:12H1:HU7J"); + int expectedResult = switch (expected) { + case '<' -> -1; + case '>' -> +1; + default -> 0; + }; var left = createErrorDiscussion(leftTitle, 0, null); var right = createErrorDiscussion(rightTitle, 0, null); - int result = errorController.compareExactMatch(left, right); + int result = errorController.compareByFullErrorCode(left, right); Assertions.assertEquals(expectedResult, result); } + @DisplayName("compare error codes by root cause") @ParameterizedTest @CsvSource(textBlock = """ - Error 6HU1:12H1:0000, Error 6HU1:12H1:0000, 0 - Error 0000:0000:0000, Error 0000:0000:0000, 0 - Error 6HU1:12H1:0000, Error 0000:0000:0000, -1 - Error 0000:0000:0000, Error 6HU1:12H1:0000, 1 + Error 6HU1:12H1:0000, =, Error 6HU1:12H1:0000 + Error 6HU1:12H1:0007, =, Error 6HU1:12H1:0042 + Error 0000:0000:0000, =, Error 0000:0000:0000 + Error 6HU1:12H1:0000, <, Error 0000:0000:0000 + Error 6HU1:12H1:0000, <, Error 6HU1:0000:0000 + Error 0000:0000:0000, >, Error 6HU1:12H1:0000 + Error 6HU1:0000:0000, >, Error 6HU1:12H1:0000 """) - public void testCompareSecondLevelMatch(String leftTitle, String rightTitle, int expectedResult) { + public void testCompareByRootCauseCode(String leftTitle, char expected, String rightTitle) { Mockito.when(errorCode.methodCode()).thenReturn("6HU1"); Mockito.when(errorCode.rootCauseCode()).thenReturn("12H1"); + int expectedResult = switch (expected) { + case '<' -> -1; + case '>' -> +1; + default -> 0; + }; var left = createErrorDiscussion(leftTitle, 0, null); var right = createErrorDiscussion(rightTitle, 0, null); - int result = errorController.compareSecondLevelMatch(left, right); - Assertions.assertEquals(expectedResult, result); - } - - @ParameterizedTest - @CsvSource(textBlock = """ - Error 6HU1:0000:0000, Error 6HU1:0000:0000, 0 - Error 0000:0000:0000, Error 0000:0000:0000, 0 - Error 6HU1:0000:0000, Error 0000:0000:0000, -1 - Error 0000:0000:0000, Error 6HU1:0000:0000, 1 - """) - public void testCompareThirdLevelMatch(String leftTitle, String rightTitle, int expectedResult) { - Mockito.when(errorCode.methodCode()).thenReturn("6HU1"); - var left = createErrorDiscussion(leftTitle, 0, null); - var right = createErrorDiscussion(rightTitle, 0, null); - int result = errorController.compareThirdLevelMatch(left, right); + int result = errorController.compareByRootCauseCode(left, right); Assertions.assertEquals(expectedResult, result); } + @DisplayName("check if the error code contains the method code") @ParameterizedTest @CsvSource(textBlock = """ Error 6HU1:0000:0000, true Error 0000:0000:0000, false """) - public void testIsPartialMatchFilter(String title, boolean expectedResult) { + public void testContainsMethodCode(String title, boolean expectedResult) { Mockito.when(errorCode.methodCode()).thenReturn("6HU1"); var ed = createErrorDiscussion(title, 0, null); - boolean result = errorController.isPartialMatchFilter(ed); + boolean result = errorController.containsMethodCode(ed); Assertions.assertEquals(expectedResult, result); } } \ No newline at end of file