From ad2fcf12e5508f2b660f2aa2f1ed07b13ce23976 Mon Sep 17 00:00:00 2001 From: Ulli Hafner Date: Thu, 30 Jun 2022 13:05:06 +0200 Subject: [PATCH] Introduce a `SafeFraction` that carefully handles an overflow when doing calculations with `Fraction` instances. --- .../plugins/coverage/model/CoverageNode.java | 27 +------ .../coverage/model/CoveragePercentage.java | 3 +- .../plugins/coverage/model/SafeFraction.java | 80 +++++++++++++++++++ .../model/CoveragePercentageTest.java | 7 ++ .../coverage/model/SafeFractionTest.java | 43 ++++++++++ 5 files changed, 135 insertions(+), 25 deletions(-) create mode 100644 plugin/src/main/java/io/jenkins/plugins/coverage/model/SafeFraction.java create mode 100644 plugin/src/test/java/io/jenkins/plugins/coverage/model/SafeFractionTest.java diff --git a/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoverageNode.java b/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoverageNode.java index d11e43666..d0232637a 100644 --- a/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoverageNode.java +++ b/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoverageNode.java @@ -28,10 +28,10 @@ import edu.hm.hafner.util.VisibleForTesting; import edu.umd.cs.findbugs.annotations.CheckForNull; -import io.jenkins.plugins.coverage.model.Coverage.CoverageBuilder; - import one.util.streamex.StreamEx; +import io.jenkins.plugins.coverage.model.Coverage.CoverageBuilder; + /** * A hierarchical decomposition of coverage results. * @@ -370,7 +370,7 @@ public SortedMap computeDelta(final CoverageNode refer SortedMap referencePercentages = reference.getMetricFractions(); metricPercentages.forEach((key, value) -> deltaPercentages.put(key, - saveSubtractFraction(value, referencePercentages.getOrDefault(key, Fraction.ZERO)))); + new SafeFraction(value).subtract(referencePercentages.getOrDefault(key, Fraction.ZERO)))); return deltaPercentages; } @@ -387,27 +387,6 @@ public SortedMap computeDeltaAsPercentage(fi .toSortedMap(Entry::getKey, e -> CoveragePercentage.valueOf(e.getValue())); } - /** - * Calculates the difference between two fraction. Since there might be an arithmetic exception due to an overflow, - * the method handles it and calculates the difference based on the double values of the fractions. - * - * @param minuend - * The minuend as a fraction - * @param subtrahend - * The subtrahend as a fraction - * - * @return the difference as a fraction - */ - private Fraction saveSubtractFraction(final Fraction minuend, final Fraction subtrahend) { - try { - return minuend.subtract(subtrahend); - } - catch (ArithmeticException e) { - double diff = minuend.doubleValue() - subtrahend.doubleValue(); - return Fraction.getFraction(diff); - } - } - /** * Returns recursively all nodes for the specified metric type. * diff --git a/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoveragePercentage.java b/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoveragePercentage.java index a02137c52..b4f504084 100644 --- a/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoveragePercentage.java +++ b/plugin/src/main/java/io/jenkins/plugins/coverage/model/CoveragePercentage.java @@ -18,6 +18,7 @@ public final class CoveragePercentage implements Serializable { private static final long serialVersionUID = 3324942976687883481L; static final String DENOMINATOR_ZERO_MESSAGE = "The denominator must not be zero"; + private static final Fraction HUNDRED = Fraction.getFraction("100.0"); /** * Creates an instance of {@link CoveragePercentage} from a {@link Fraction fraction} within the range [0,1]. @@ -28,7 +29,7 @@ public final class CoveragePercentage implements Serializable { * @return the created instance */ public static CoveragePercentage valueOf(final Fraction fraction) { - Fraction percentage = fraction.multiplyBy(Fraction.getFraction("100.0")); + Fraction percentage = new SafeFraction(fraction).multiplyBy(HUNDRED); return new CoveragePercentage(percentage.getNumerator(), percentage.getDenominator()); } diff --git a/plugin/src/main/java/io/jenkins/plugins/coverage/model/SafeFraction.java b/plugin/src/main/java/io/jenkins/plugins/coverage/model/SafeFraction.java new file mode 100644 index 000000000..596c6317d --- /dev/null +++ b/plugin/src/main/java/io/jenkins/plugins/coverage/model/SafeFraction.java @@ -0,0 +1,80 @@ +package io.jenkins.plugins.coverage.model; + +import org.apache.commons.lang3.math.Fraction; + +/** + * A small wrapper for {@link Fraction} instances that avoids an arithmetic overflow by using double based operations in + * case of an exception. + * + * @author Ullrich Hafner + */ +public class SafeFraction { + private final Fraction fraction; + + /** + * Creates a new fraction instance that wraps the specified fraction with safe operations. + * + * @param fraction + * the fraction to wrap + */ + public SafeFraction(final Fraction fraction) { + this.fraction = fraction; + } + + /** + * Multiplies the value of this fraction by another, returning the result in reduced form. Since there might be an + * arithmetic exception due to an overflow, this method will handle this situation by calculating the multiplication + * based on the double values of the fractions. + * + * @param multiplicator + * the fraction to multiply by + * + * @return a {@code Fraction} instance with the resulting values + */ + public Fraction multiplyBy(final Fraction multiplicator) { + try { + return fraction.multiplyBy(multiplicator); + } + catch (ArithmeticException exception) { + return Fraction.getFraction(fraction.doubleValue() * multiplicator.doubleValue()); + } + } + + /** + * Subtracts the value of another fraction from the value of this one, returning the result in reduced form. Since + * there might be an arithmetic exception due to an overflow, this method will handle this situation by calculating + * the subtraction based on the double values of the fractions. + * + * @param subtrahend + * the fraction to subtract + * + * @return a {@code Fraction} instance with the resulting values + */ + public Fraction subtract(final Fraction subtrahend) { + try { + return fraction.subtract(subtrahend); + } + catch (ArithmeticException exception) { + return Fraction.getFraction(fraction.doubleValue() - subtrahend.doubleValue()); + } + } + + /** + * Adds the value of another fraction to the value of this one, returning the result in reduced form. Since + * there might be an arithmetic exception due to an overflow, this method will handle this situation by calculating + * the addition based on the double values of the fractions. + * + * @param summand + * the fraction to add + * + * @return a {@code Fraction} instance with the resulting values + */ + public Fraction add(final Fraction summand) { + try { + return fraction.add(summand); + } + catch (ArithmeticException exception) { + return Fraction.getFraction(fraction.doubleValue() + summand.doubleValue()); + } + } +} diff --git a/plugin/src/test/java/io/jenkins/plugins/coverage/model/CoveragePercentageTest.java b/plugin/src/test/java/io/jenkins/plugins/coverage/model/CoveragePercentageTest.java index d6a9f06c7..3d9964650 100644 --- a/plugin/src/test/java/io/jenkins/plugins/coverage/model/CoveragePercentageTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/coverage/model/CoveragePercentageTest.java @@ -21,6 +21,13 @@ class CoveragePercentageTest { private static final double COVERAGE_PERCENTAGE = 50.0; private static final Locale LOCALE = Locale.GERMAN; + @Test + void shouldHandleOverflow() { + Fraction fraction = Fraction.getFraction(Integer.MAX_VALUE - 1, Integer.MAX_VALUE - 1); + CoveragePercentage coveragePercentage = valueOf(fraction); + assertThat(coveragePercentage.getDoubleValue()).isEqualTo(100); + } + @Test void shouldCreateCoveragePercentageFromFraction() { Fraction fraction = Fraction.getFraction(COVERAGE_FRACTION); diff --git a/plugin/src/test/java/io/jenkins/plugins/coverage/model/SafeFractionTest.java b/plugin/src/test/java/io/jenkins/plugins/coverage/model/SafeFractionTest.java new file mode 100644 index 000000000..24f70fc09 --- /dev/null +++ b/plugin/src/test/java/io/jenkins/plugins/coverage/model/SafeFractionTest.java @@ -0,0 +1,43 @@ +package io.jenkins.plugins.coverage.model; + +import org.apache.commons.lang3.math.Fraction; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.*; + +/** + * Tests the class {@link SafeFraction}. + * + * @author Ullrich Hafner + */ +class SafeFractionTest { + @Test + void shouldDelegateToFraction() { + Fraction ten = Fraction.getFraction(10, 1); + SafeFraction safeFraction = new SafeFraction(ten); + assertThat(safeFraction.multiplyBy(ten).doubleValue()).isEqualTo(100.0); + assertThat(safeFraction.subtract(ten).doubleValue()).isEqualTo(0); + assertThat(safeFraction.add(ten).doubleValue()).isEqualTo(20.0); + } + + @Test + void shouldHandleOverflowForMultiply() { + Fraction fraction = Fraction.getFraction(Integer.MAX_VALUE - 1, Integer.MAX_VALUE - 1); + SafeFraction safeFraction = new SafeFraction(fraction); + assertThat(safeFraction.multiplyBy(Fraction.getFraction("100.0")).doubleValue()).isEqualTo(100.0); + } + + @Test + void shouldHandleOverflowForSubtract() { + Fraction fraction = Fraction.getFraction(Integer.MAX_VALUE - 1, Integer.MAX_VALUE - 1); + SafeFraction safeFraction = new SafeFraction(fraction); + assertThat(safeFraction.subtract(Fraction.getFraction("100.0")).doubleValue()).isEqualTo(-99.0); + } + + @Test + void shouldHandleOverflowForAdd() { + Fraction fraction = Fraction.getFraction(Integer.MAX_VALUE - 1, Integer.MAX_VALUE - 1); + SafeFraction safeFraction = new SafeFraction(fraction); + assertThat(safeFraction.add(Fraction.getFraction("100.0")).doubleValue()).isEqualTo(101.0); + } +}