Skip to content

add query param to show all meter values for a transaction#2047

Merged
goekay merged 1 commit into
masterfrom
2046-add-query-param-to-show-all-meter-values-for-a-transaction
May 21, 2026
Merged

add query param to show all meter values for a transaction#2047
goekay merged 1 commit into
masterfrom
2046-add-query-param-to-show-all-meter-values-for-a-transaction

Conversation

@goekay
Copy link
Copy Markdown
Member

@goekay goekay commented May 21, 2026

Screenshot 2026-05-21 at 12 29 17

@goekay goekay linked an issue May 21, 2026 that may be closed by this pull request
@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Add query parameter to show all meter values for transactions

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add query parameter to filter meter values by energy type
• Allow retrieving all meter values or energy values only
• Update repository, service, and controller layers
• Add UI toggle for meter value filtering option
Diagram
flowchart LR
  A["REST API & Web Controller"] -->|energyValuesOnly param| B["TransactionService"]
  B -->|energyValuesOnly flag| C["TransactionRepository"]
  C -->|conditional filtering| D["Database Query"]
  D -->|filtered results| E["Transaction Details"]
  F["UI Form"] -->|toggle selection| A

Loading

File Changes

1. src/main/java/de/rwth/idsg/steve/repository/TransactionRepository.java ✨ Enhancement +1/-1

Add energyValuesOnly parameter to interface

• Add energyValuesOnly boolean parameter to getDetails() method signature
• Enables filtering of meter values at repository interface level

src/main/java/de/rwth/idsg/steve/repository/TransactionRepository.java


2. src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java ✨ Enhancement +19/-8

Implement conditional meter value filtering logic

• Add energyValuesOnly parameter to getDetails() implementation
• Move unit condition filtering into conditional block based on parameter
• Apply conditional filter in stream processing for energy values
• Only filter by energy type when energyValuesOnly is true

src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java


3. src/main/java/de/rwth/idsg/steve/service/TransactionService.java ✨ Enhancement +5/-5

Propagate energyValuesOnly parameter through service

• Add energyValuesOnly parameter to getDetails() and writeTransactionMeterValuesCSV() methods
• Pass parameter through to repository layer
• Update stop() method to pass true for energy values only

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


View more (5)
4. src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java ✨ Enhancement +4/-2

Add REST API query parameter for filtering

• Add @RequestParam import
• Add energyValuesOnly query parameter with default value true
• Pass parameter to service layer in getTransactionDetails() endpoint

src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java


5. src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java ✨ Enhancement +8/-3

Add web controller support for filtering parameter

• Add @RequestParam import
• Add energyValuesOnly parameter to transaction details view method
• Add energyValuesOnly parameter to CSV export method
• Pass parameter to service and add to model attributes

src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java


6. src/test/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImplIT.java 🧪 Tests +2/-2

Update integration tests for new parameter

• Update test calls to getDetails() with true parameter
• Ensures tests pass with new method signature

src/test/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImplIT.java


7. src/test/java/de/rwth/idsg/steve/utils/__DatabasePreparer__.java 🧪 Tests +1/-1

Update test utility for new parameter

• Update getDetails() call with true parameter
• Maintains compatibility with new method signature

src/test/java/de/rwth/idsg/steve/utils/DatabasePreparer.java


8. src/main/webapp/WEB-INF/views/data-man/transactionDetails.jsp ✨ Enhancement +13/-3

Add UI controls for meter value filtering

• Add dropdown selector for energyValuesOnly filter option
• Add Apply button to refresh view with selected filter
• Pass energyValuesOnly parameter to CSV download form
• Improve UI layout with centered forms

src/main/webapp/WEB-INF/views/data-man/transactionDetails.jsp


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 21, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0)

Grey Divider


Remediation recommended

1. Unbounded all-values fetch 🐞 Bug ☼ Reliability
Description
When energyValuesOnly=false, TransactionRepositoryImpl.getDetails fetches all matching
CONNECTOR_METER_VALUE rows with no cap and materializes them into a List before returning via
REST/CSV. For long-running/active (stopTimestamp==null) transactions this can produce very large
responses and high memory/DB load, risking timeouts or OOM under legitimate or abusive requests.
Code

src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java[R226-242]

Evidence
The repository builds its timestamp condition to include all meter values since the start time for
active transactions, then executes an unrestricted selectFrom(CONNECTOR_METER_VALUE)...fetch() and
materializes the result. The REST and manager controllers expose energyValuesOnly as a request
parameter (defaulting to true but allowing false), and CSV generation calls getDetails then writes
all values, so an opt-in request can trigger the unbounded path end-to-end.

src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java[184-211]
src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java[226-259]
src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java[93-101]
src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java[93-112]
src/main/java/de/rwth/idsg/steve/service/TransactionService.java[74-77]

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

### Issue description
`TransactionRepositoryImpl.getDetails(..., energyValuesOnly=false)` can load and return an unbounded number of meter-value rows. The code currently fetches all rows into memory (`fetch() ... toList()`), which can create huge in-memory lists and responses for long-running/active transactions.

### Issue Context
This is now user-selectable via `energyValuesOnly=false` on both the REST endpoint and the manager UI/CSV download.

### Fix Focus Areas
- src/main/java/de/rwth/idsg/steve/repository/impl/TransactionRepositoryImpl.java[184-262]
- src/main/java/de/rwth/idsg/steve/web/api/TransactionsRestController.java[93-101]
- src/main/java/de/rwth/idsg/steve/web/controller/TransactionsReservationsController.java[93-112]
- src/main/java/de/rwth/idsg/steve/service/TransactionService.java[74-77]

### Suggested fix approach
- Add a hard server-side cap (e.g., `MAX_METER_VALUES`) when `energyValuesOnly=false` (or always), implemented via `.limit(MAX_METER_VALUES + 1)` and return a clear 400/413-style error if exceeded.
- Alternatively/additionally introduce pagination (e.g., `page`/`size` or `limit`/`offset`) for the details endpoint and CSV download, so callers can retrieve data safely in chunks.
- If CSV is expected to be large, consider streaming with a DB cursor/`fetchLazy()` + incremental CSV write to avoid holding all values in memory.

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


Grey Divider

Qodo Logo

@goekay goekay merged commit af556df into master May 21, 2026
41 checks passed
@goekay goekay deleted the 2046-add-query-param-to-show-all-meter-values-for-a-transaction branch May 21, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add query param to show all meter values for a transaction

1 participant