[BUG] `findPointsInBoxWithNeighbors` divides by `maxPoints` before the `Math.max` guard (`/ by zero`)

[SuperCorleone]

Describe the Bug

long intervalMinutes = Math.max(1, period.toMinutes() / maxPoints); performs the division before Math.max(1, …), so the guard does not protect against maxPoints == 0 → ArithmeticException: / by zero. The only production caller (LocationDataApiController.java:192) passes a hardcoded 10000, so this is currently unreachable — a latent/defensive issue, not a live crash.

Root Cause

long intervalMinutes = Math.max(1, period.toMinutes() / maxPoints);  // division happens first → /0 when maxPoints==0

Suggested Fix

long intervalMinutes = maxPoints <= 0 ? 1 : Math.max(1, period.toMinutes() / maxPoints);

Corresponding Test (generated)

@Test
public void testFindPointsInBoxWithNeighbors_WhenMaxPointsIsZero_ShouldHandleDivisionByZero() {
    // Arrange - We don't actually need to mock anything since the method will handle it gracefully
    Long relevantPointCount = 150L;
    when(jdbcTemplate.queryForObject(anyString(), eq(Long.class), any(Object[].class)))
            .thenReturn(relevantPointCount);

    List<RawLocationPoint> expectedPoints = Arrays.asList(
        new RawLocationPoint(1L, startTime, new com.dedicatedcode.reitti.model.geo.GeoPoint(45.0, 0.0), 10.0, null, false, false, false, false, null),
        new RawLocationPoint(2L, endTime, new com.dedicatedcode.reitti.model.geo.GeoPoint(46.0, 1.0), 15.0, null, false, false, false, false, null)
    );

    when(jdbcTemplate.query(anyString(), eq(rawLocationPointRowMapper), any(Object[].class)))
            .thenReturn(expectedPoints);

    // Act - This should not throw an ArithmeticException anymore
    List<RawLocationPoint> result = rawLocationPointJdbcService.findPointsInBoxWithNeighbors(
        testUser, startTime, endTime, minLat, maxLat, minLon, maxLon, 0);

    // Assert - Since maxPoints is 0, the method will still call the sampling code path
    // but with intervalMinutes = 1 (due to Math.max(1, ...))
    assertEquals(2, result.size());

    verify(jdbcTemplate).queryForObject(
        "SELECT COUNT(*)\n" +
        "FROM raw_location_points\n" +
        "WHERE user_id = ?\n" +
        "  AND ST_Within(geom, ST_MakeEnvelope(?, ?, ?, ?, 4326))\n" +
        "  AND timestamp >= ?::timestamp AND timestamp < ?::timestamp\n" +
        "  AND ignored = false AND invalid = false",
        eq(Long.class),
        any(Object[].class)
    );

    verify(jdbcTemplate).query(
        "WITH box_filtered_points AS (\n" +
        "    SELECT\n" +
        "        id,\n" +
        "        user_id,\n" +
        "        timestamp,\n" +
        "        geom,\n" +
        "        accuracy_meters,\n" +
        "        elevation_meters,\n" +
        "        processed,\n" +
        "        ignored,\n" +
        "        invalid,\n" +
        "        synthetic,\n" +
        "        version,\n" +
        "        ST_Within(geom, ST_MakeEnvelope(?, ?, ?, ?, 4326)) as in_box,\n" +
        "        LAG(ST_Within(geom, ST_MakeEnvelope(?, ?, ?, ?, 4326)))\n" +
        "            OVER (ORDER BY timestamp) as prev_in_box,\n" +
        "        LEAD(ST_Within(geom, ST_MakeEnvelope(?, ?, ?, ?, 4326)))\n" +
        "            OVER (ORDER BY timestamp) as next_in_box\n" +
        "    FROM raw_location_points\n" +
        "    WHERE user_id = ?\n" +
        "      AND timestamp >= ?::timestamp AND timestamp < ?::timestamp\n" +
        "      AND ignored = false AND invalid = false\n" +
        ")\n" +
        "SELECT\n" +
        "    id,\n" +
        "    user_id,\n" +
        "    timestamp,\n" +
        "    ST_AsText(geom) as geom,\n" +
        "    accuracy_meters,\n" +
        "    elevation_meters,\n" +
        "    processed,\n" +
        "    synthetic,\n" +
        "    ignored,\n" +
        "    invalid,\n" +
        "    version\n" +
        "FROM box_filtered_points\n" +
        "WHERE in_box = true\n" +
        "   OR prev_in_box = true\n" +
        "   OR next_in_box = true\n" +
        "ORDER BY timestamp",
        eq(rawLocationPointRowMapper),
        any(Object[].class)
    );
}

This input was generated by the test case generator TestFusion developed in our STAR lab.

[dgraf-gh]

Hi @SuperCorleone,

this would be a problem if the amount would be specified by a user. Right now, it is always fixed.
I inlined the maxPoints directly into the method for now. This method is bound to be removed all the way in a future release anyway.

Thank you for the report :)