1992 Undo "timestamp monotonicity" check for MeterValues

[goekay]

No description provided.

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Remove timestamp monotonicity check for MeterValues

🐞 Bug fix

Walkthroughs

Description

• Remove timestamp monotonicity validation for MeterValues
• Allow out-of-order timestamps in transaction data
• Add test case validating out-of-order meter values
• Update existing test to expect null instead of error

Diagram

flowchart LR
  A["MeterValue Validation"] -->|Remove| B["Chronological Order Check"]
  A -->|Keep| C["Track Earliest/Latest Timestamps"]
  B -->|Previously| D["Rejected Out-of-Order Data"]
  B -->|Now| E["Allows Out-of-Order Data"]

Loading

File Changes

1. src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_ServiceValidator.java 🐞 Bug fix +1/-8

Remove timestamp monotonicity validation logic

• Removed prev variable tracking previous timestamp
• Deleted chronological order validation check that returned error for non-decreasing timestamps
• Updated comment to reflect tracking only earliest and latest timestamps
• Simplified loop logic by removing timestamp comparison logic

src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_ServiceValidator.java

---

2. src/test/java/de/rwth/idsg/steve/service/CentralSystemService16ServiceValidatorTest.java 🧪 Tests +58/-3

Update tests to allow out-of-order timestamps

• Added new test validateStop_outOfOrderTransactionData_isAllowed() with real-world out-of-order
 meter values
• Updated test validateMeterValues_timestampsOutOfOrder_returnsNull() to expect null instead of
 error
• Added helper method sampledValue() to construct SampledValue objects with context and format
• Added imports for Measurand, ReadingContext, Reason, SampledValue, and UnitOfMeasure
• Added documentation comments explaining rationale for allowing out-of-order timestamps

src/test/java/de/rwth/idsg/steve/service/CentralSystemService16ServiceValidatorTest.java

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. Sentinel timestamp misdetected 🐞 Bug ✓ Correctness

Description

validateMeterValuesInternal can return "MeterValue.timestamp is empty" even when timestamps exist if
all timestamps equal the MIN (1970-01-01T00:00:00Z) or MAX sentinel boundaries, because
latest/earliest are not updated on equality and then compared by reference to the sentinel
objects. This can reject valid meter values from devices with clocks reset to epoch 0 or extreme
values.

Code

src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_ServiceValidator.java[R117-123]

        DateTime earliest = MAX;
        DateTime latest = MIN;
-        DateTime prev = MIN;

-        // single pass: track earliest, latest, and check chronological order
+        // single pass: track earliest and latest
        for (MeterValue mv : meterValues) {
            if (mv == null) {
                continue;

Evidence

The method initializes sentinels (earliest = MAX, latest = MIN), updates only on strict
before/after, then checks earliest == MAX || latest == MIN to decide “no timestamps”. If a
MeterValue timestamp equals MIN exactly, ts.isAfter(latest) is false so latest stays MIN; the
final sentinel check triggers the empty-timestamp error even though a timestamp was present. The
same applies to MAX for earliest. This logic is in the modified region of this PR.

src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_ServiceValidator.java[47-48]
src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_ServiceValidator.java[117-139]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`validateMeterValuesInternal` uses MIN/MAX sentinel `DateTime` objects and reference comparisons (`==`) to detect absence of timestamps. Because `earliest`/`latest` update only on strict before/after, valid timestamps that are exactly equal to the sentinel boundaries (especially epoch 0) can leave a sentinel unchanged and be misreported as “timestamp is empty”.

### Issue Context
This is in the core validation path for both `MeterValuesRequest` and `StopTransactionRequest.transactionData`, so it can cause false validation failures for devices with mis-set clocks.

### Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/service/CentralSystemService16_ServiceValidator.java[110-163]

### Suggested fix
Replace the sentinel-reference approach with a boolean `foundTs` and initialize `earliest/latest` from the first non-null timestamp encountered (or update on equality too). Avoid `==` comparisons for sentinel detection.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[goekay]

partial undo of #1988