Skip to content

Commit f41402e

Browse files
thcedthced
committed
refactor: Simplify out-of-range integer check and cover new branches
The SonarCloud new-code coverage gate failed on PR #109: extracting the bound checks into checkIntegerBounds turned the exclusive-bound and beyond-long paths into "new" lines that no test exercised. - Simplify the beyond-long branch to delegate to the long path when the value fits, and otherwise reject against whichever bound lies on its side (or an int32/int64 format), collapsing the per-bound arms. - Add tests for exclusive bounds, values above 2^53 that still fit in a long, every beyond-long rejection path, and QueryParams edge cases (empty pairs, keys without '=', encoded separators).
1 parent f6fe554 commit f41402e

3 files changed

Lines changed: 116 additions & 34 deletions

File tree

‎src/main/java/com/retailsvc/http/validate/DefaultValidator.java‎

Lines changed: 14 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -424,41 +424,21 @@ private static Optional<ValidationError> checkIntegerBounds(
424424

425425
private static Optional<ValidationError> checkIntegerBounds(
426426
BigInteger n, IntegerSchema s, String pointer) {
427-
long l;
428-
try {
429-
l = n.longValueExact();
430-
} catch (ArithmeticException outsideLong) {
431-
// Magnitude exceeds long range: it is below any Long minimum (if negative) or above any Long
432-
// maximum (if positive), and cannot satisfy an int32/int64 format.
433-
if (n.signum() > 0) {
434-
if (s.maximum() != null) {
435-
return err(pointer, "maximum", "integer above maximum " + s.maximum(), n);
436-
}
437-
if (s.exclusiveMaximum() != null) {
438-
return err(
439-
pointer, "exclusiveMaximum", "integer not less than " + s.exclusiveMaximum(), n);
440-
}
441-
} else {
442-
if (s.minimum() != null) {
443-
return err(pointer, "minimum", "integer below minimum " + s.minimum(), n);
444-
}
445-
if (s.exclusiveMinimum() != null) {
446-
return err(
447-
pointer, "exclusiveMinimum", "integer not greater than " + s.exclusiveMinimum(), n);
448-
}
449-
}
450-
if (s.multipleOf() != null && n.remainder(BigInteger.valueOf(s.multipleOf())).signum() != 0) {
451-
return err(pointer, "multipleOf", "not a multiple of " + s.multipleOf(), n);
452-
}
453-
if (s.format() != null) {
454-
IntegerFormatCheck check = INTEGER_FORMAT_CHECKS.get(s.format());
455-
if (check != null) {
456-
return err(pointer, FORMAT_KEYWORD, check.message(), n);
457-
}
458-
}
459-
return OK;
427+
if (n.bitLength() < Long.SIZE) {
428+
return checkIntegerBounds(n.longValue(), s, pointer);
429+
}
430+
// Magnitude exceeds signed-long range: it breaches whichever bound lies on its side, and no
431+
// int32/int64 format can represent it.
432+
if (n.signum() > 0 && (s.maximum() != null || s.exclusiveMaximum() != null)) {
433+
return err(pointer, "maximum", "integer out of range", n);
460434
}
461-
return checkIntegerBounds(l, s, pointer);
435+
if (n.signum() < 0 && (s.minimum() != null || s.exclusiveMinimum() != null)) {
436+
return err(pointer, "minimum", "integer out of range", n);
437+
}
438+
if (s.format() != null && INTEGER_FORMAT_CHECKS.containsKey(s.format())) {
439+
return err(pointer, FORMAT_KEYWORD, INTEGER_FORMAT_CHECKS.get(s.format()).message(), n);
440+
}
441+
return OK;
462442
}
463443

464444
private static Optional<ValidationError> checkNumber(

‎src/test/java/com/retailsvc/http/internal/QueryParamsTest.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package com.retailsvc.http.internal;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.entry;
5+
6+
import org.junit.jupiter.api.Test;
7+
8+
class QueryParamsTest {
9+
10+
@Test
11+
void nullOrBlankQueryYieldsEmptyMap() {
12+
assertThat(QueryParams.parse(null)).isEmpty();
13+
assertThat(QueryParams.parse(" ")).isEmpty();
14+
}
15+
16+
@Test
17+
void encodedSeparatorStaysInsideValue() {
18+
// %26 is decoded after splitting, so it does not become a delimiter: one param, not two.
19+
assertThat(QueryParams.parse("q=a%26b=c")).containsExactly(entry("q", "a&b=c"));
20+
}
21+
22+
@Test
23+
void splitsPairsDecodesAndKeepsFirstOccurrence() {
24+
assertThat(QueryParams.parse("name=Alice%20Smith&name=Bob&city=x"))
25+
.containsExactly(entry("name", "Alice Smith"), entry("city", "x"));
26+
}
27+
28+
@Test
29+
void skipsEmptyPairsAndHandlesKeysWithoutValue() {
30+
// Empty pair (from "&&") is skipped; a key with no '=' maps to an empty value.
31+
assertThat(QueryParams.parse("a=1&&flag&b=2"))
32+
.containsExactly(entry("a", "1"), entry("flag", ""), entry("b", "2"));
33+
}
34+
}

‎src/test/java/com/retailsvc/http/validate/StringIntegerNumberTest.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,74 @@ void integerRejectsValueOverflowingLongPastMaximum() {
259259
.isEqualTo("maximum");
260260
}
261261

262+
@Test
263+
void integerExclusiveBounds() {
264+
IntegerSchema exclMin =
265+
new IntegerSchema(Set.of(TypeName.INTEGER), null, null, 5L, null, null, null, Map.of());
266+
assertThatCode(() -> v.validate(6, exclMin, "/v")).doesNotThrowAnyException();
267+
assertThatThrownBy(() -> v.validate(5, exclMin, "/v"))
268+
.extracting(t -> ((ValidationException) t).error().keyword())
269+
.isEqualTo("exclusiveMinimum");
270+
271+
IntegerSchema exclMax =
272+
new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, 5L, null, null, Map.of());
273+
assertThatCode(() -> v.validate(4, exclMax, "/v")).doesNotThrowAnyException();
274+
assertThatThrownBy(() -> v.validate(5, exclMax, "/v"))
275+
.extracting(t -> ((ValidationException) t).error().keyword())
276+
.isEqualTo("exclusiveMaximum");
277+
}
278+
279+
@Test
280+
void integerLargeButWithinLongRangeIsCheckedPrecisely() {
281+
// > 2^53 skips the exact-double fast path, but 2^60 still fits in a long and is bound-checked.
282+
IntegerSchema s =
283+
new IntegerSchema(Set.of(TypeName.INTEGER), 0L, 1000L, null, null, null, null, Map.of());
284+
assertThatThrownBy(() -> v.validate(BigInteger.TWO.pow(60), s, "/v"))
285+
.extracting(t -> ((ValidationException) t).error().keyword())
286+
.isEqualTo("maximum");
287+
}
288+
289+
@Test
290+
void integerOverflowingLongIsCheckedAgainstBoundsAndFormat() {
291+
BigInteger huge = BigInteger.TWO.pow(64).add(BigInteger.valueOf(50)); // > Long.MAX
292+
BigInteger hugeNegative = huge.negate(); // < Long.MIN
293+
294+
IntegerSchema max =
295+
new IntegerSchema(Set.of(TypeName.INTEGER), 0L, 1000L, null, null, null, null, Map.of());
296+
assertThatThrownBy(() -> v.validate(huge, max, "/v"))
297+
.extracting(t -> ((ValidationException) t).error().keyword())
298+
.isEqualTo("maximum");
299+
300+
IntegerSchema exclMax =
301+
new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, 1000L, null, null, Map.of());
302+
assertThatThrownBy(() -> v.validate(huge, exclMax, "/v"))
303+
.extracting(t -> ((ValidationException) t).error().keyword())
304+
.isEqualTo("maximum");
305+
306+
IntegerSchema min =
307+
new IntegerSchema(Set.of(TypeName.INTEGER), 0L, null, null, null, null, null, Map.of());
308+
assertThatThrownBy(() -> v.validate(hugeNegative, min, "/v"))
309+
.extracting(t -> ((ValidationException) t).error().keyword())
310+
.isEqualTo("minimum");
311+
312+
IntegerSchema exclMin =
313+
new IntegerSchema(Set.of(TypeName.INTEGER), null, null, 0L, null, null, null, Map.of());
314+
assertThatThrownBy(() -> v.validate(hugeNegative, exclMin, "/v"))
315+
.extracting(t -> ((ValidationException) t).error().keyword())
316+
.isEqualTo("minimum");
317+
318+
IntegerSchema int64 =
319+
new IntegerSchema(
320+
Set.of(TypeName.INTEGER), null, null, null, null, null, "int64", Map.of());
321+
assertThatThrownBy(() -> v.validate(huge, int64, "/v"))
322+
.extracting(t -> ((ValidationException) t).error().keyword())
323+
.isEqualTo("format");
324+
325+
IntegerSchema unbounded =
326+
new IntegerSchema(Set.of(TypeName.INTEGER), null, null, null, null, null, null, Map.of());
327+
assertThatCode(() -> v.validate(huge, unbounded, "/v")).doesNotThrowAnyException();
328+
}
329+
262330
@Test
263331
void numberAcceptsDoublesAndIntegers() {
264332
NumberSchema s =

0 commit comments

Comments
 (0)