fix(report): safely enable query by example - #3339
Open
Ben-Heerema wants to merge 21 commits into
Open
Conversation
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Contributor
Reviewer's GuideRe-enables Query-by-Example by routing validated, read-only SELECTs through a bounded JDBC execution path, guarding it behind a default-on feature flag, and wiring localized UI/error handling and tests around the new behavior. Sequence diagram for validated Query-by-Example execution flowsequenceDiagram
actor User
participant RptByExample2Action
participant RptByExampleData
participant QueryByExampleSqlValidator
participant LegacyJdbcQuery
participant CarlosDB
User->>RptByExample2Action: submit POST /RptByExample2 with sql
RptByExample2Action->>RptByExample2Action: isEnabled(properties)
alt [feature disabled]
RptByExample2Action->>RptByExampleData: audit(providerNo, sql, 0, 0, disabled)
RptByExample2Action-->>User: view disabled message
else [feature enabled]
alt [sql is blank]
RptByExample2Action->>RptByExampleData: audit(providerNo, sql, 0, 0, rejected)
RptByExample2Action-->>User: view validation error
else [sql present]
RptByExample2Action->>RptByExampleData: execute(sql, properties, providerNo)
RptByExampleData->>QueryByExampleSqlValidator: validate(sql, properties)
QueryByExampleSqlValidator->>LegacyJdbcQuery: trustedSelectSql(sql)
QueryByExampleSqlValidator-->>RptByExampleData: TrustedSql
RptByExampleData->>LegacyJdbcQuery: getConnection()
LegacyJdbcQuery-->>RptByExampleData: Connection
RptByExampleData->>CarlosDB: executeQuery(TrustedSql)
CarlosDB-->>RptByExampleData: ResultSet
RptByExampleData-->>RptByExample2Action: QueryResult(html, rowCount)
RptByExample2Action->>RptByExample2Action: write2Database(sql, providerNo)
RptByExample2Action-->>User: view results
RptByExampleData->>RptByExampleData: audit(providerNo, sql, durationMs, rowCount, outcome)
end
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughQuery-by-Example now validates submitted read-only SQL with JSqlParser, executes bounded JDBC queries, renders limited results, reports localized outcomes, and scopes report access and favorites by provider. ChangesQuery-by-Example execution
Estimated code review effort: 5 (Critical) | ~90 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In RptByExample2Action.execute, the SQLException/RuntimeException catch block only sets a request attribute and swallows the exception; consider logging the underlying error (including the provider and a query hash) so operational issues can be diagnosed while still avoiding logging raw SQL.
- QueryByExampleSqlValidator.rejectBlockedFunctions currently recompiles a regex for each function on every call; you could precompute and cache these Patterns (or use a single combined pattern) to avoid repeated compilation on hot paths.
- RptByExampleData.audit logs every query at INFO, which may generate a large volume of logs in active environments; consider using a dedicated logger or a lower log level (or making the level configurable) to avoid overwhelming general application logs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In RptByExample2Action.execute, the SQLException/RuntimeException catch block only sets a request attribute and swallows the exception; consider logging the underlying error (including the provider and a query hash) so operational issues can be diagnosed while still avoiding logging raw SQL.
- QueryByExampleSqlValidator.rejectBlockedFunctions currently recompiles a regex for each function on every call; you could precompute and cache these Patterns (or use a single combined pattern) to avoid repeated compilation on hot paths.
- RptByExampleData.audit logs every query at INFO, which may generate a large volume of logs in active environments; consider using a dedicated logger or a lower log level (or making the level configurable) to avoid overwhelming general application logs.
## Individual Comments
### Comment 1
<location path="src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.java" line_range="66-67" />
<code_context>
+ throw new QueryByExampleValidationException("The query could not be parsed as a single SELECT", e);
+ }
+
+ if (!(statement instanceof Select) || statement instanceof SetOperationList) {
+ throw new QueryByExampleValidationException("Only one SELECT statement is allowed");
+ }
+
</code_context>
<issue_to_address>
**🚨 issue (security):** UNION / set-operation detection is ineffective and will not block multi-SELECT queries
With JSqlParser, a `SELECT ... UNION SELECT ...` is parsed as a `Select` whose `SelectBody` is a `SetOperationList`, not as a `SetOperationList` `Statement`. That means `statement instanceof SetOperationList` is always false and set operations are not blocked. To correctly reject multi-SELECT queries, first assert `statement instanceof Select`, then check `((Select) statement).getSelectBody() instanceof SetOperationList` and fail in that case. Since this endpoint processes user-supplied SQL, allowing set operations here is a security risk.
</issue_to_address>
### Comment 2
<location path="src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java" line_range="141-147" />
<code_context>
return SUCCESS;
}
+ static boolean isEnabled(Properties properties) {
+ String configured = properties.getProperty(ENABLED_PROPERTY);
+ return configured == null || configured.isBlank()
+ || configured.equalsIgnoreCase("true")
+ || configured.equalsIgnoreCase("yes")
+ || configured.equalsIgnoreCase("on");
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Configuration flag handling is brittle when the property contains surrounding whitespace
`isEnabled` compares the raw `configured` value without trimming, so values like `" true"` or `"yes "` will be treated as disabled even though they’re semantically valid. To avoid surprising behavior when editing `carlos.properties`, normalize before comparison, e.g.:
```java
String configured = properties.getProperty(ENABLED_PROPERTY);
if (configured == null) {
return true;
}
String normalized = configured.trim();
return normalized.isEmpty()
|| normalized.equalsIgnoreCase("true")
|| normalized.equalsIgnoreCase("yes")
|| normalized.equalsIgnoreCase("on");
```
```suggestion
static boolean isEnabled(Properties properties) {
String configured = properties.getProperty(ENABLED_PROPERTY);
if (configured == null) {
return true;
}
String normalized = configured.trim();
return normalized.isEmpty()
|| normalized.equalsIgnoreCase("true")
|| normalized.equalsIgnoreCase("yes")
|| normalized.equalsIgnoreCase("on");
}
```
</issue_to_address>
### Comment 3
<location path="src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.java" line_range="59-61" />
<code_context>
+ throw new QueryByExampleValidationException(e.getMessage(), e);
+ }
+
+ Statement statement;
+ try {
+ statement = CCJSqlParserUtil.parse(sql);
+ } catch (JSQLParserException | RuntimeException e) {
+ throw new QueryByExampleValidationException("The query could not be parsed as a single SELECT", e);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Parsing the un-normalized SQL instead of the trusted form may diverge from the JDBC execution boundary
You obtain a normalized/validated `TrustedSql` via `LegacyJdbcQuery.trustedSelectSql(sql)` but still parse the original `sql` with JSqlParser. If `trustedSelectSql` modifies the query (e.g., strips comments, enforces a single statement, normalizes whitespace), the query you validate may differ from what JDBC actually executes. To keep the validation boundary aligned, parse `trustedSql.sql()` instead so structural checks are performed on the exact SQL being run.
Suggested implementation:
```java
Statement statement;
try {
statement = CCJSqlParserUtil.parse(trustedSql.sql());
} catch (JSQLParserException | RuntimeException e) {
throw new QueryByExampleValidationException("The query could not be parsed as a single SELECT", e);
}
```
```java
String sqlWithoutStringLiterals = stripStringLiterals(trustedSql.sql());
if (LOCKING_SELECT.matcher(sqlWithoutStringLiterals).find()) {
throw new QueryByExampleValidationException("Locking SELECT statements are not allowed");
}
```
These changes assume `LegacyJdbcQuery.TrustedSql.sql()` returns the exact SQL string that will be executed via JDBC. If `TrustedSql` exposes a different accessor name for the normalized SQL, use that instead of `sql()`. Also consider updating any other validation logic in this class that still relies on the original `sql` string so that all validations are performed against the trusted SQL form.
</issue_to_address>
### Comment 4
<location path="src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.java" line_range="68-77" />
<code_context>
+ private static Stream<String> unsafeQueries() {
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for the "application schema not configured" failure paths in applicationSchema()
Current tests only cover valid `db_name` configurations. Since `applicationSchema(Properties)` can fail when `db_name` is null, blank, or resolves to an empty schema before the `?`, please add tests that pass these misconfigurations and assert a `QueryByExampleValidationException` with the expected message to verify the validator fails closed in these cases.
Suggested implementation:
```java
private static Stream<String> unsafeQueries() {
return Stream.of(
"show tables",
"describe demographic",
"explain select * from demographic",
"update demographic set last_name='x'",
"select * from demographic union select * from provider",
"select * from demographic; select * from provider",
"select * from demographic -- comment",
"select * from other_database.demographic",
"select sleep(1)"
);
}
@ParameterizedTest(name = "application schema misconfiguration: {1}")
@MethodSource("invalidApplicationSchemas")
@DisplayName("fails closed when application schema is not configured correctly")
void shouldFailClosedWhenApplicationSchemaMisconfigured(Properties misconfiguredProperties, String expectedMessage) {
assertThatThrownBy(() -> QueryByExampleSqlValidator.applicationSchema(misconfiguredProperties))
.isInstanceOf(QueryByExampleValidationException.class)
.hasMessage(expectedMessage);
}
private static Stream<Arguments> invalidApplicationSchemas() {
Properties withoutDbName = new Properties();
Properties withBlankDbName = new Properties();
withBlankDbName.setProperty("db_name", " ");
Properties withEmptySchema = new Properties();
withEmptySchema.setProperty("db_name", "?");
return Stream.of(
Arguments.of(withoutDbName, "application schema not configured"),
Arguments.of(withBlankDbName, "application schema not configured"),
Arguments.of(withEmptySchema, "application schema not configured")
);
}
```
1. Ensure the test file imports `java.util.Properties` and `org.junit.jupiter.params.provider.Arguments`. If they are not already present, update the import section, for example:
<<<<<<< SEARCH
import java.util.stream.Stream;
=======
import java.util.Properties;
import java.util.stream.Stream;
import org.junit.jupiter.params.provider.Arguments;
>>>>>>> REPLACE
2. Verify the exact error message thrown by `applicationSchema(Properties)` for these failure paths. If the message differs (e.g., includes more detail), update the `"application schema not configured"` literal in `invalidApplicationSchemas()` and/or switch to `hasMessageContaining(...)` to match the actual behavior.
3. If `db_name` is configured via a different property key, adjust the `setProperty("db_name", ...)` calls accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In RptByExample.jsp the result-limit message is hard-coded to 1000 while the cap is defined as RptByExampleData.MAX_ROWS; consider passing the limit from the backend (or referencing the constant indirectly) so the UI text and enforcement cannot drift apart.
- The isEnabled(Properties) helper treats any non-empty, non-true-like value as disabled; if that’s intentional, it may be worth explicitly handling/logging unexpected values to avoid accidental feature shutdown due to misconfiguration.
- QueryByExampleSqlValidator.rejectBlockedFunctions builds a new regex Pattern for each function on every call; you could precompile these patterns once (e.g., in a static map) to avoid repeated compilation for each validation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In RptByExample.jsp the result-limit message is hard-coded to 1000 while the cap is defined as RptByExampleData.MAX_ROWS; consider passing the limit from the backend (or referencing the constant indirectly) so the UI text and enforcement cannot drift apart.
- The isEnabled(Properties) helper treats any non-empty, non-true-like value as disabled; if that’s intentional, it may be worth explicitly handling/logging unexpected values to avoid accidental feature shutdown due to misconfiguration.
- QueryByExampleSqlValidator.rejectBlockedFunctions builds a new regex Pattern for each function on every call; you could precompile these patterns once (e.g., in a static map) to avoid repeated compilation for each validation.
## Individual Comments
### Comment 1
<location path="src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.java" line_range="66-67" />
<code_context>
+ throw new QueryByExampleValidationException("The query could not be parsed as a single SELECT", e);
+ }
+
+ if (!(statement instanceof Select) || statement instanceof SetOperationList) {
+ throw new QueryByExampleValidationException("Only one SELECT statement is allowed");
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The SetOperationList check will never be true, so UNION/SET operations are not actually blocked.
`SetOperationList` is not a subtype of `Statement`, so `statement instanceof SetOperationList` is always false. As a result, this only checks that the SQL is a `Select` and does not actually reject UNION/INTERSECT queries. You likely need to inspect the `Select`’s `SelectBody` (e.g. `((Select) statement).getSelectBody()`) and reject when it is a `SetOperationList`, or otherwise use the JSqlParser types that specifically model set operations.
</issue_to_address>
### Comment 2
<location path="src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.java" line_range="96" />
<code_context>
+
+ private static void rejectOtherSchemas(Statement statement, String applicationSchema)
+ throws QueryByExampleValidationException {
+ Set<String> tables = new TablesNamesFinder<Void>().getTables(statement);
+ for (String table : tables) {
+ String normalizedTable = unquoteIdentifier(table);
</code_context>
<issue_to_address>
**issue (bug_risk):** TablesNamesFinder is non-generic; the `<Void>` type argument is invalid and may not compile.
`TablesNamesFinder` in JSqlParser 5.x is not generic, so `new TablesNamesFinder<Void>()` will not compile; use `new TablesNamesFinder()` instead. Also, verify that `getTables(statement)` returns all table references you need (including those in nested subselects) for your schema-qualification checks.
</issue_to_address>
### Comment 3
<location path="src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp" line_range="236-237" />
<code_context>
<c:if test="${not empty results}">
<div class="mt-3">
+ <p class="text-muted small">
+ <fmt:message key="oscarReport.RptByExample.MsgResultLimit">
+ <fmt:param value="1000"/>
+ </fmt:message>
+ </p>
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Result limit is hard-coded in the JSP and can diverge from the backend MAX_ROWS constant.
Here the JSP hard-codes `1000` while the backend uses `RptByExampleData.MAX_ROWS`. If `MAX_ROWS` changes, the UI text will be wrong. Please derive the displayed limit from `MAX_ROWS` (e.g., pass it as a request attribute or via a message parameter) so it’s defined in a single place.
Suggested implementation:
```
<p class="text-muted small">
<fmt:message key="oscarReport.RptByExample.MsgResultLimit">
<fmt:param value="${rptByExampleMaxRows}"/>
</fmt:message>
</p>
```
In the controller / action that forwards to `RptByExample.jsp`, set a request attribute (matching the name used above) from the backend constant, for example:
- In the relevant Java controller/servlet:
`request.setAttribute("rptByExampleMaxRows", RptByExampleData.MAX_ROWS);`
or in Spring MVC:
`model.addAttribute("rptByExampleMaxRows", RptByExampleData.MAX_ROWS);`
This ensures the UI message always reflects the single source of truth `RptByExampleData.MAX_ROWS`.
</issue_to_address>
### Comment 4
<location path="src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.java" line_range="68-69" />
<code_context>
+ .isInstanceOf(QueryByExampleValidationException.class);
+ }
+
+ private static Stream<String> unsafeQueries() {
+ return Stream.of(
+ "show tables",
+ "describe demographic",
</code_context>
<issue_to_address>
**suggestion (testing):** Cover schema-configuration edge cases and additional allowed/blocked qualification scenarios
The `unsafeQueries` parameterization covers many unsafe patterns, but a few validator behaviours are still untested:
1. `applicationSchema(Properties)` error paths:
- When `db_name` is null, blank, or effectively empty after stripping the query string (e.g. `"?useUnicode=true"`), the validator should throw `QueryByExampleValidationException` with the configured message. A dedicated test (outside `unsafeQueries`) would document this fail-closed behaviour for misconfigured schemas.
2. Qualified-table edge cases:
- A query using the configured schema qualifier with quotes/odd casing (e.g. `select * from """OSCAR_McMaster""".demographic`) should be accepted, proving `unquoteIdentifier` + case-insensitive comparison work.
- A query where the schema name is only a prefix/suffix of the configured one (e.g. `oscar_mcmaster_backup.demographic` when configured `oscar_mcmaster`) should be rejected, to guard against `lastIndexOf('.')`/substring treating similar prefixes as the same schema.
These tests would better pin down schema-boundary enforcement under configuration quirks and naming collisions.
Suggested implementation:
```java
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.assertThatCode;
```
```java
private static Stream<String> unsafeQueries() {
return Stream.of(
"show tables",
"describe demographic",
"explain select * from demographic",
"update demographic set last_name='x'",
"select * from demographic union select * from provider",
"select * from demographic; select * from provider",
"select * from demographic -- comment",
"select * from other_database.demographic",
"select sleep(1)",
"select benchmark(1000, md5('x'))",
"select get_lock('qbe', 1)",
"select release_lock('qbe')",
"select * from oscar_mcmaster_backup.demographic"
```
```java
@ParameterizedTest(name = "rejects: {0}")
@MethodSource("unsafeQueries")
@DisplayName("rejects unsafe or out-of-scope SQL")
void shouldRejectUnsafeQueries(String sql) {
assertThatThrownBy(() -> QueryByExampleSqlValidator.validate(sql, properties))
.isInstanceOf(QueryByExampleValidationException.class);
}
@Test
@DisplayName("fails closed when application schema db_name is null")
void shouldFailClosedWhenSchemaDbNameIsNull() {
Properties misconfigured = new Properties(properties);
misconfigured.remove("db_name");
assertThatThrownBy(() -> QueryByExampleSqlValidator.applicationSchema(misconfigured))
.isInstanceOf(QueryByExampleValidationException.class);
}
@Test
@DisplayName("fails closed when application schema db_name is blank")
void shouldFailClosedWhenSchemaDbNameIsBlank() {
Properties misconfigured = new Properties(properties);
misconfigured.setProperty("db_name", " ");
assertThatThrownBy(() -> QueryByExampleSqlValidator.applicationSchema(misconfigured))
.isInstanceOf(QueryByExampleValidationException.class);
}
@Test
@DisplayName("fails closed when application schema db_name is effectively empty after query string is stripped")
void shouldFailClosedWhenSchemaDbNameIsEmptyAfterStrippingQueryString() {
Properties misconfigured = new Properties(properties);
// Simulate a URL whose path is only a query string so the extracted db_name is empty
misconfigured.setProperty("db_url", "jdbc:mysql://localhost:3306/?useUnicode=true");
misconfigured.remove("db_name");
assertThatThrownBy(() -> QueryByExampleSqlValidator.applicationSchema(misconfigured))
.isInstanceOf(QueryByExampleValidationException.class);
}
@Test
@DisplayName("allows queries qualified with quoted configured schema name, case-insensitively")
void shouldAllowQuotedConfiguredSchemaQualifier() {
Properties configured = new Properties(properties);
configured.setProperty("db_name", "oscar_mcmaster");
String sql = "select * from \"OSCAR_McMaster\".demographic";
assertThatCode(() -> QueryByExampleSqlValidator.validate(sql, configured))
.doesNotThrowAnyException();
}
@Test
@DisplayName("rejects queries when schema qualifier only shares prefix with configured schema")
void shouldRejectSimilarButDifferentSchemaQualifier() {
Properties configured = new Properties(properties);
configured.setProperty("db_name", "oscar_mcmaster");
String sql = "select * from oscar_mcmaster_backup.demographic";
assertThatThrownBy(() -> QueryByExampleSqlValidator.validate(sql, configured))
.isInstanceOf(QueryByExampleValidationException.class);
}
private static Stream<String> unsafeQueries() {
```
- The new tests assume:
- A static method `QueryByExampleSqlValidator.applicationSchema(Properties)` exists and throws `QueryByExampleValidationException` on misconfiguration.
- The test class has a `properties` field preconfigured with at least `db_url` and/or `db_name`. If this differs, adjust the setup inside the new tests to match your actual configuration contract.
- If `applicationSchema` does not use `db_url` in the way assumed (deriving the schema name from the JDBC URL), adapt `shouldFailClosedWhenSchemaDbNameIsEmptyAfterStrippingQueryString` to construct the misconfiguration that leads to an effectively empty schema name in your implementation.
- If `db_name` is stored under a different key or via a dedicated configuration object, replace `"db_name"` and `"db_url"` with the appropriate keys or builder calls.
- If `assertThatCode` is already statically imported under a different form or you use a different style for "no exception" assertions, you can remove the added import and adjust `shouldAllowQuotedConfiguredSchemaQualifier` accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 17 files
Architecture diagram
sequenceDiagram
participant Browser as Browser
participant JSP as RptByExample.jsp
participant Action as RptByExample2Action
participant Validator as QueryByExampleSqlValidator
participant LegacyJdbc as LegacyJdbcQuery
participant DS as CARLOS DataSource
participant DB as Application DB (oscar_mcmaster)
participant History as report_by_examples table
Note over Browser,History: Query-by-Example Submission Flow
Browser->>JSP: POST /RptByExample2 (sql, CSRF token)
JSP->>Action: execute() via Struts2
Action->>Action: Check QUERY_BY_EXAMPLE_ENABLED
alt Disabled
Action->>Action: Set queryDisabled=true
Action-->>JSP: SUCCESS (form displayed with warning)
else Enabled
Action->>Action: Validate non-empty sql
opt Empty SQL
Action->>Action: Set queryValidationError=true
Action-->>JSP: SUCCESS (no execution)
end
Action->>Action: Get datasource properties (db_name)
Action->>Validator: validate(sql, properties)
Validator->>LegacyJdbc: trustSelectSql(sql)
LegacyJdbc-->>Validator: TrustedSql wrapper
Validator->>Validator: Parse SQL with JSqlParser
Validator->>Validator: Reject non-SELECT, UNION, comments, stacked statements
Validator->>Validator: Check schema qualifier matches application DB
Validator->>Validator: Reject locking (FOR UPDATE/SHARE), INTO, blocked functions
alt Validation failure
Validator-->>Action: QueryByExampleValidationException
Action->>Action: Set queryValidationError=true
Action-->>JSP: SUCCESS (error message)
else Validation passes
Validator-->>Action: TrustedSql object
Action->>DS: getConnection()
DS-->>Action: Connection (read-write by default)
Action->>DB: setReadOnly(true)
Action->>DB: prepareStatement(trustedSql, FORWARD_ONLY, CONCUR_READ_ONLY)
Action->>DB: setMaxRows(1000)
Action->>DB: setQueryTimeout(15)
Action->>DB: executeQuery()
DB-->>Action: ResultSet (read-only, capped)
Action->>Action: RptResultStruct.getStructureWithCount(rs)
Action-->>DB: close resources, restore setReadOnly(false)
alt Success
Action->>DB: write2Database(sql, providerNo) — save history
DB-->>Action: History saved
Action-->>JSP: SUCCESS (results HTML, rowCount)
else SQLTimeout
Action->>Action: Set queryTimeout=true
Action-->>JSP: SUCCESS (timeout message)
else SQLException/Runtime
Action->>Action: Set queryExecutionError=true
Action-->>JSP: SUCCESS (error message)
end
end
end
Note over Action: Audit log (no raw SQL): provider, SHA256 hash, duration, rowCount, outcome
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
coderabbitai
Bot
added
Bug fix
Review effort [1-5]: 4
Tests
type: security
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/io/github/carlos_emr/carlos/report/data/RptResultStruct.java (1)
63-82: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap header cells in
<tr>/<thead>for valid markup.
<th>elements at lines 73-82 are appended directly inside<table>with no enclosing<tr>or<thead>. Browsers using the HTML5 parsing algorithm typically recover by inserting an implied row, but the emitted markup is not valid HTML and is inconsistent with the siblinggetStructure2method, which correctly wraps its headers in<thead><tr>...</tr></thead>.Wrap the header cells the same way
getStructure2does, for markup correctness and consistency.♻️ Proposed fix
sb.append("<table id='results'>"); + sb.append("<thead><tr>"); for (int i = 0; i < columns; i++) { // for each column in result set columnLabels[i] = rsmd.getColumnLabel(i + 1); // put names in array // use i+1 or else you're going to get an exception // insert headings for table sb.append("<th class='headerColor'>"); sb.append(Encode.forHtml(columnLabels[i])); sb.append("</th>"); } + sb.append("</tr></thead>");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/io/github/carlos_emr/carlos/report/data/RptResultStruct.java` around lines 63 - 82, Update getStructureWithCount to wrap the generated header cells in a thead and tr structure, matching the existing getStructure2 implementation. Open the header row before the column loop, keep each encoded th generation unchanged, and close the row and thead before emitting the table body.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.java`:
- Around line 37-51: Add comprehensive JavaDoc to the public
QueryByExampleSqlValidator class and its validate method, documenting the
accepted SQL shape, required properties schema, exact TrustedSql return
contract, and the conditions that cause QueryByExampleValidationException.
Include `@param`, `@return`, `@throws`, and an accurate `@since` tag, while preserving
the existing validation behavior.
In
`@src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.java`:
- Around line 26-34: Add JavaDoc to both public
QueryByExampleValidationException constructors, documenting the message and
cause parameters and their semantics. Add a class-level `@since` tag using the
timestamp from git history, while preserving the existing exception behavior and
class description.
In `@src/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java`:
- Around line 45-48: Update the class JavaDoc for RptByExampleData to remove the
obsolete FluReportGenerate description and document its actual contract:
validate SQL with QueryByExampleSqlValidator, execute queries read-only under
row and timeout limits, and audit execution outcomes.
- Around line 74-92: In the query execution block of RptByExampleData, replace
the unparameterized connection.prepareStatement(trustedSql.sql(), ...) flow with
a read-only forward-only Statement created via createStatement(...) and execute
the SQL using executeQuery(trustedSql.sql()). Preserve the existing max-row,
timeout, result-processing, and resource-management behavior, and update the
related test matcher to expect createStatement(...).
In `@src/main/resources/oscarResources_en.properties`:
- Line 8928: Parameterize the oscarReport.RptByExample.MsgTimeout message with
{0} instead of hardcoding “15-second”; update the corresponding entries in the
Spanish, French, Polish, and Brazilian Portuguese bundles, and pass
RptByExampleData.QUERY_TIMEOUT_SECONDS as the fmt:param in RptByExample.jsp’s
queryTimeout alert block.
In `@src/main/resources/oscarResources_es.properties`:
- Around line 6449-6455: Add a separate “# TODO: translate” comment immediately
above each of the fallback keys oscarReport.RptByExample.MsgValidationError,
MsgTimeout, MsgExecutionError, MsgHistoryError, and MsgResultLimit, while
preserving their existing English values and the current comment above
MsgDisabled.
In `@src/main/resources/oscarResources_pl.properties`:
- Around line 5867-5873: Add a separate “# TODO: translate” comment immediately
above each remaining Query-by-Example fallback key—MsgValidationError,
MsgTimeout, MsgExecutionError, MsgHistoryError, and MsgResultLimit—while
preserving their English fallback values and the existing MsgDisabled entry.
In `@src/main/resources/oscarResources_pt_BR.properties`:
- Around line 7354-7360: Add a separate “# TODO: translate” comment immediately
above each untranslated Query-by-Example fallback key: MsgValidationError,
MsgTimeout, MsgExecutionError, MsgHistoryError, and MsgResultLimit, while
preserving their English values and the existing marker above MsgDisabled.
In `@src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp`:
- Around line 237-247: Update the results notice in RptByExample.jsp to consume
the existing resultRowCount request attribute alongside resultLimit, using an
appropriate message parameter or message key so users see both the actual rows
returned and the configured limit.
In
`@src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.java`:
- Around line 42-90: Rename the four test methods in this diff to BDD names with
exactly one underscore and a lowercase context segment:
shouldAllowReadOnlyApplicationSelects_whenSchemaIsConfigured,
shouldAllowBlockedFunctionNameInsideLiteral_whenFunctionTextIsQuoted,
shouldRejectSetOperationSelects_whenMultipleQueriesAreCombined, and
shouldRejectMissingApplicationSchema_whenSchemaIsUnavailable. Keep each test’s
behavior unchanged.
In
`@src/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java`:
- Line 81: The test method names should follow the repository’s one-underscore
BDD convention. Rename shouldExecuteBoundedReadOnlyQueryAndRestoreConnection to
a should<Action>_<context><Condition> form such as
shouldExecuteQuery_whenSqlIsValidated, and rename shouldRejectBeforeConnecting
similarly, such as shouldRejectQuery_whenSqlIsUnsafe; leave the test behavior
unchanged.
---
Outside diff comments:
In `@src/main/java/io/github/carlos_emr/carlos/report/data/RptResultStruct.java`:
- Around line 63-82: Update getStructureWithCount to wrap the generated header
cells in a thead and tr structure, matching the existing getStructure2
implementation. Open the header row before the column loop, keep each encoded th
generation unchanged, and close the row and thead before emitting the table
body.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 01bd83f2-8693-4ede-9d37-aae53e3c4993
📒 Files selected for processing (19)
dependencies-lock.jsonpom.xmlsrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptResultStruct.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/resources/carlos.propertiessrc/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
Contributor
There was a problem hiding this comment.
2 issues found across 17 files (changes from recent commits).
Confidence score: 4/5
- In
src/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.java, the new SQL-literal scanner duplicatesQueryByExampleSqlValidator.stripStringLiterals, which can drift over time and create inconsistent handling in a security-sensitive path; that raises the chance of subtle validation gaps—extract the shared quote-stripping logic into one lower-level utility and reuse it in both places. - In
src/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java, the updated success-path setup only asserts read-only stays read-only, so it no longer exercises the writable-restore round trip after successful execution; this could let connection-state regressions slip through—add a success test that starts writable, toggles for execution, and verifies restoration.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java">
<violation number="1" location="src/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java:68">
P3: The success-path test now mocks isReadOnly()=true, so it only verifies a read-only connection stays read-only. That drops coverage of the restore-to-writable round trip on a successfully executed query over a connection that started writable (the pre-change false->false case). The restore-to-false path is only exercised in the new failure test (via setReadOnly(false) suppression), not on success. Consider keeping isReadOnly()=false in the success test (or adding a separate success case starting writable) so the full read-only->execute->restore-writable transition is asserted for the normal path.</violation>
</file>
<file name="src/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.java">
<violation number="1" location="src/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.java:619">
P3: The new SQL-literal scanner duplicates `QueryByExampleSqlValidator.stripStringLiterals`, creating two security-sensitive parsers that can drift; extracting the shared quote-stripping behavior into a lower-level utility would keep both validation boundaries consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Ben-Heerema
marked this pull request as draft
August 6, 2026 18:18
Collaborator
Author
|
@coderabbitai review |
Contributor
|
|
Contributor
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
Ben-Heerema
marked this pull request as ready for review
August 6, 2026 18:24
Contributor
There was a problem hiding this comment.
Sorry @Ben-Heerema, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/resources/oscarResources_fr.properties`:
- Line 5634: Add a “# TODO: translate” comment directly above each affected
English fallback property key, including oscarReport.RptByExample.MsgTimeout and
the property at the referenced second location, without changing their values.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d92b288d-60a4-40d0-8d88-df28e9439c35
📒 Files selected for processing (12)
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (17)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Useio.github.carlos_emr.carlos.*namespace for ALL new code; specific packages: DAOs inio.github.carlos_emr.carlos.commn.dao.*, Models inio.github.carlos_emr.carlos.commn.model.*, exception: ProviderDao inio.github.carlos_emr.carlos.dao.ProviderDao
UsePathValidationUtilsfor ALL file path operations that include user input
NEVER log or expose PHI (Protected Health Information) in logs or error messages
UseSpringUtils.getBean(ClassName.class)for Spring bean injection in Struts2 actions and non-Spring-managed classes
JavaDoc is required on all public classes and methods; do NOT include@authortags, use@sincetags with git log timestamps, use CARLOS copyright header for new files
**/*.java: Useio.github.carlos_emr.carlos.*for all new Java code, includingcommn.dao,commn.model, andcommn.dao.forms; retain legacyorg.oscarehr.common.dao.*only for test utilities andProviderDaoat its documented exception path.
Use parameterized queries only; never concatenate user input or variables into SQL or HQL.
UsePathValidationUtilsfor all file operations involving user input, including path components, existing paths, and uploads; use the returned validated value for subsequent filesystem operations.
Use sanctioned layer suffixes such as*Action,*Loader,*Resolver,*Validator,*Persister,*Calculator,*Parser,*Service,*Dao,*Dto, and*Command; do not introduce*Prep,*Manager,*Helper,*Utils, or compound role suffixes.
Public entrypoint classes and non-obvious public or protected methods should have contract-level JavaDoc; do not add@authortags, preserve accurate GPL headers, and never remove existing copyright notices.
**/*.java: Use parameterized queries exclusively; never construct SQL through string concatenation.
UseSecurityInfoManager.hasPrivilege()checks for every action, especially all Struts2*2Actionclasses.
UsePathValidationUtilsfor all file operations involving ...
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{jsp,java}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use OWASP Encoder for ALL user input output: in JSP use
${e:forHtml(value)}with taglib<%@ taglib uri="owasp.encoder.jakarta" prefix="e" %>, in Java useEncode.forHtml(value), with context-specific variants (forHtmlAttribute,forJavaScript,forCssString,forUri,forUriComponent)Use OWASP Encoder functions for every user-controlled output, selecting the encoder for the correct HTML, attribute, JavaScript, CSS, or URL context.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,sql}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use parameterized queries ONLY in SQL/HQL operations; never use string concatenation for SQL queries
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp,js,ts,xml,sql,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use “CARLOS” or “CARLOS EMR” in all user-facing content; preserve
io.github.carlos_emr.carlos.*namespaces and legacy technical references where required.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{java,jsp}: Protect all user input with context-appropriate CARLOS null-safe OWASP encoding; do not introduce raw OWASPEncode.forXxx,<e:forXxx>,${e:forXxx(...)},<c:out>, orfn:escapeXml()for new output.
Do not log or expose PHI; sanitize necessary operational identifiers withLogSafe, avoid clinical context, and do not place identifiers in browser-visible exceptions unless explicitly required by an authorized workflow.Add audit logging for patient-data access and use healthcare-specific validation while maintaining PHI protection.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp,xml}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{java,jsp,xml}: Preserve existing copyright notices and GPL versions; new files must use the CARLOS project header.
Use Java-style billing acronym casing (On,Ohip,Ra,Moh, etc.), allow onlyDiagas the short billing form, and avoid compressed legacy names such as3rd,Dig,Db,Obj,Hlp,Bean, andHandler.
Healthcare code must protect PHI, include appropriate authorization and audit behavior, and follow relevant provincial and healthcare integration standards.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp,js,ts,xml,sql}
📄 CodeRabbit inference engine (GEMINI.md)
Protect PHI: never log or expose patient health information, and apply appropriate privacy and audit controls.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
src/main/java/**/*.java
📄 CodeRabbit inference engine (GEMINI.md)
src/main/java/**/*.java: Use theio.github.carlos_emr.carlos.*namespace for all new production Java code; retainorg.oscarehr.*andoscar.*only where required for legacy compatibility.
Provide comprehensive JavaDoc for all public classes and methods; document parameters, exact return types, thrown exceptions, and deprecations with migration guidance; do not add@authortags and use accurate@sincehistory.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp,xml,js,css,sql,md}
📄 CodeRabbit inference engine (GEMINI.md)
Preserve existing copyright notices and GPL versions; use the CARLOS copyright header for new files and do not remove existing attribution.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,xml,jsp,sql}
📄 CodeRabbit inference engine (GEMINI.md)
Use the documented healthcare integration standards and domain conventions for HL7, FHIR R4, SNOMED CT, ICD coding, ATC medication codes, DICOM, and provincial systems.
Files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jspsrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*Test.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*Test.java: Unit tests must useCarlosUnitTestBasebase class for mocked tests (no database); integration tests must useCarlosTestBasefor H2 database tests
Follow BDD naming convention for test methods:should<Action>_<preposition><Condition>()with ONE underscore, camelCase, andshouldprefixUse hierarchical JUnit tags such as integration/unit, DAO or manager layer, and CRUD or query operation tags to support filtered test execution.
Files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
src/test/java/**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
src/test/java/**/*.java: Use JUnit 5 tests undersrc/test/, select the appropriateCarlosTestBase,CarlosUnitTestBase, or domain-specific base, and inspect the actual production interface before writing tests.
Name test methodsshould<Action>_<context><Condition>()with exactly one underscore, camelCase, a requiredshouldprefix, and a lowercase segment after the underscore.
For HibernateDaoSupport integration tests, flush withhibernateTemplate.flush()rather than onlyentityManager.flush(); use proper boolean HQL comparisons and explicit%wildcards for LIKE test inputs.
For manager tests, register SpringUtils mocks before static mocks, close static mocks after each test, organize large suites with@Nested, and use the sharedLogCapturefor Log4j2 assertions.
Files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
**/*2Action.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*2Action.java: IncludeSecurityInfoManager.hasPrivilege()checks as the FIRST operation in all Struts2 action execute() methods
All new Struts2 actions must follow the *2Action.java naming pattern and extendorg.apache.struts2.ActionSupport(NOTcom.opensymphony.xwork2.*)
**/*2Action.java: Every new Struts 2 action must use the*2Action.javanaming convention, generally extendorg.apache.struts2.ActionSupport, use constructor injection, and performSecurityInfoManager.hasPrivilege()checks.
Mutating*2Actionclasses must reject GET and HEAD before any side effect, and their contract-test manifest must classify newly discovered mutators.
Actions that write directly to servlet responses must returnNONEafter writing, own their error response, and avoid named-result forwarding after response output begins.
Files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
src/main/java/**/*2Action.java
📄 CodeRabbit inference engine (GEMINI.md)
All new Struts2 actions must use the
*2Action.javanaming convention, extend the appropriate Struts action base, and useorg.apache.struts2.*rather than deprecatedcom.opensymphony.xwork2.*packages.
Files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
src/main/webapp/**/*.jsp
📄 CodeRabbit inference engine (CLAUDE.md)
JSPs performing AJAX POSTs that read
input[name="CSRF-TOKEN"]must contain a real non-GET form or include/WEB-INF/jspf/csrf-token.jspfinside the body; do not use an action-less placeholder form.
src/main/webapp/**/*.jsp: Use CARLOS or CARLOS EMR as the display name in user-facing JSP content; preserve technical legacy references when they describe upstream heritage accurately.
Add a comprehensive JSP documentation comment after the copyright header describing purpose, features, parameters, and@since.
Files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
src/main/webapp/WEB-INF/jsp/**/*.jsp
📄 CodeRabbit inference engine (CLAUDE.md)
Place new JSP views under
WEB-INF/jsp/**, expose them through extensionless Struts actions, and do not create public JSP entrypoints.
Files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
**/*.jsp
📄 CodeRabbit inference engine (CLAUDE.md)
Add a comprehensive JSP comment block after the copyright header describing purpose, features, parameters, and
@since.For new JSP EL output, prefer
${e:forHtml()}and declare the Jakarta taglib as<%@ taglib uri="owasp.encoder.jakarta" prefix="e" %>when EL functions are used; do not use the legacy encoder JSP URI.
Files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
🧠 Learnings (38)
📚 Learning: 2026-02-21T01:06:58.978Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 443
File: src/main/resources/oscarResources_en.properties:34-57
Timestamp: 2026-02-21T01:06:58.978Z
Learning: In user-facing properties files (e.g., src/main/resources/oscarResources_en.properties), avoid introducing new brand domains (e.g., a new carlosgalaxy domain) in strings. When updating links for branding (such as loginApplication.image.logoText), prefer a neutral, stable link like the GitHub repo with https, or omit the link entirely if no suitable neutral target exists. Ensure all updates keep branding consistent and do not introduce new external domains in UI strings.
Applied to files:
src/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.properties
📚 Learning: 2026-05-09T19:22:01.986Z
Learnt from: phc007
Repo: carlos-emr/carlos PR: 2120
File: src/main/resources/oscarResources_pt_BR.properties:8007-8028
Timestamp: 2026-05-09T19:22:01.986Z
Learning: In this repo’s resource bundle `.properties` files under `src/main/resources/` (e.g., `oscarResources_pt_BR.properties`), you can write non-ASCII characters directly in UTF-8 without using Java-style Unicode escapes like `\u00F3`, since the application runs on Tomcat 11 + Java 21 (default UTF-8). Save these files in UTF-8 and only fall back to `\uXXXX` if a specific entry must be compatible with a different decoding setup.
Applied to files:
src/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.properties
📚 Learning: 2026-07-28T17:19:42.866Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 2440
File: src/main/resources/oscarResources_fr.properties:605-606
Timestamp: 2026-07-28T17:19:42.866Z
Learning: For locale-specific `.properties` resource bundles (e.g., `*_<locale>.properties`), if a translation for a key is missing, keep the value as the English fallback and place a `# TODO: translate` comment directly on the line immediately above the affected property key. Treat these `# TODO: translate` placeholders as intentional: during code review, do not recommend translating/replacing them unless a maintainer/translator provides a verified translation.
Applied to files:
src/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.properties
📚 Learning: 2026-07-28T17:19:45.658Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 2440
File: src/main/resources/oscarResources_pt_BR.properties:726-727
Timestamp: 2026-07-28T17:19:45.658Z
Learning: For i18n .properties files, follow docs/I18N-STANDARDS.md’s convention for intentionally-missing locale translations: when a key is missing for a locale, use an English fallback value and add a “# TODO: translate” comment immediately above that key. In code review, do not recommend translating these placeholder/fallback values, since they are intentionally left in English until a trusted translation is provided.
Applied to files:
src/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.properties
📚 Learning: 2026-03-17T16:16:27.811Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 650
File: src/main/resources/oscarResources_pl.properties:0-0
Timestamp: 2026-03-17T16:16:27.811Z
Learning: In the repo carlos-emr/carlos, i18n wording refinements for files matching src/main/resources/oscarResources_*.properties are out of scope for feature PRs. Track and fix these refinements separately (e.g., via issues or a dedicated i18n PR) so feature changes aren’t blocked by localization wording edits.
Applied to files:
src/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.properties
📚 Learning: 2026-07-28T17:19:45.547Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 2440
File: src/main/resources/oscarResources_pl.properties:646-647
Timestamp: 2026-07-28T17:19:45.547Z
Learning: For any missing locale translation entries in `src/main/resources/oscarResources_*.properties`, use the English fallback value and place a `# TODO: translate` comment immediately above the affected property key. Treat these as approved placeholders per `docs/I18N-STANDARDS.md`; do not flag them as issues requiring immediate translation.
Applied to files:
src/main/resources/oscarResources_en.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.properties
📚 Learning: 2026-03-06T23:54:01.319Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 563
File: src/main/java/io/github/carlos_emr/carlos/lab/ca/all/parsers/AlphaHandler.java:59-63
Timestamp: 2026-03-06T23:54:01.319Z
Learning: In the carlos-emr/carlos repository, for Java sources under src/main/java, empty no-arg constructors should have only a minimal one-line JavaDoc. Do not add since or other metadata tags to empty constructors, as they provide no meaningful value. Place documentation efforts in the class-level JavaDoc instead.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-03-13T21:29:23.775Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 590
File: src/main/java/io/github/carlos_emr/carlos/eform/actions/DisplayImage2Action.java:69-69
Timestamp: 2026-03-13T21:29:23.775Z
Learning: When reviewing Java source files, allow since Javadoc tags to reflect substantial rewrites or major API changes (e.g., security overhaul, new API surface). Do not flag the tag as incorrect solely because the file predates the tagged date. If flagging is considered, verify against commit history or release notes to confirm the tag corresponds to a meaningful change, not the original introduction. Apply this guidance across Java sources in the repository (src/main/java/**/*.java).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-04-11T22:46:51.202Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1556
File: src/main/java/io/github/carlos_emr/carlos/demographic/dto/DemographicListItemDTO.java:74-179
Timestamp: 2026-04-11T22:46:51.202Z
Learning: In this repository, do not require method-level JavaDoc for plain public JavaBean-style getters and setters in Java files under src/main/java/**/*.java. Their meaning should be covered by field-level documentation and/or the class-level JavaDoc. Only require JavaDoc for non-trivial public methods (e.g., constructors with logic, factory methods, and computed/derived accessors where behavior is not just returning/setting a field). In code reviews, missing JavaDoc on simple getters/setters should not be flagged as a documentation violation.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-04-11T22:47:04.719Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1556
File: src/main/java/io/github/carlos_emr/carlos/provider/dto/ProviderSummaryDTO.java:57-104
Timestamp: 2026-04-11T22:47:04.719Z
Learning: In this repository, do not require JavaDoc for trivial getter and setter methods. Only JavaDoc for types/classes and for more meaningful public API points is expected—e.g., class-level JavaDoc, constructors, static factory methods, and non-trivial public methods (such as formatting helpers). In code reviews, ignore missing JavaDoc on getters/setters.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-05-24T01:01:46.756Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/main/java/io/github/carlos_emr/carlos/utility/CachedDateFormats.java:165-216
Timestamp: 2026-05-24T01:01:46.756Z
Learning: For Java sources under src/main/java/**/*.java, follow the repository’s Documentation Standards (CLAUDE.md) regarding Javadoc requirements: do not require mechanical param/return/throws tags for trivial wrapper/delegator public methods that are essentially one-line pass-throughs to a cached or underlying instance (e.g., methods like parseDefault/formatDefault/parse/format in CachedDateFormats.java). Instead, rely on class-level Javadoc to document the overall contracts, including thread-safety, null handling, return value semantics, and any mutation/restoration behavior (such as timezone restoration). Require full per-method Javadoc tag coverage only for non-trivial public methods whose behavior is not obvious from the context/signature.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-06-25T16:10:08.376Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 3023
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarConsultationRequest/pageUtil/EConsult2Action.java:287-328
Timestamp: 2026-06-25T16:10:08.376Z
Learning: When reviewing Java code that reconstructs an origin/authority from a `java.net.URI` (e.g., `scheme://host:port`), assume `URI#getHost()` may already return bracketed IPv6 literals such as `[::1]` (not just `::1`). Do not “re-bracket” IPv6 manually when rebuilding the origin from the host returned by `getHost()`. If `getHost()` is `null`, only then apply the malformed-colon/authority fallback rejection logic on that authority-fallback path (the path that runs when `getHost()` is `null`), not on the normal host-based reconstruction path.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-03-24T17:34:18.508Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 716
File: src/main/java/io/github/carlos_emr/carlos/eform/actions/RtlPreventions2Action.java:155-156
Timestamp: 2026-03-24T17:34:18.508Z
Learning: In this repository (carlos-emr/carlos), treat `demographic_no` as an internal database surrogate integer key (the PK of the demographic table), not as PHI. When reviewing code, do not flag logging or output of `demographic_no` as PHI exposure. PHI to flag includes clinical/demographic data elements like patient name, date of birth, HIN, and address.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-04-07T02:57:10.703Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1111
File: src/main/java/io/github/carlos_emr/carlos/util/JDBCUtil.java:140-142
Timestamp: 2026-04-07T02:57:10.703Z
Learning: When reviewing this repository’s Java code, flag any unprotected use of `SchemaFactory.newInstance(...)` (e.g., instances that do not restrict `XMLConstants.ACCESS_EXTERNAL_DTD` and `XMLConstants.ACCESS_EXTERNAL_SCHEMA` to empty strings). Recommend migrating those call sites to `io.github.carlos_emr.carlos.utility.XmlUtils.createSecureSchemaFactory()` so XML Schema validation is hardened against external entity/schema access. Known migrated call sites include `IndicatorTemplateHandler.java` and `DemographicExportAction42Action.java` (post-PR `#1111`).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-03-21T14:58:44.822Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 704
File: src/test/java/io/github/carlos_emr/carlos/commn/dao/Hl7TextInfoDaoIntegrationTest.java:109-110
Timestamp: 2026-03-21T14:58:44.822Z
Learning: For the test-only HibernateTemplate implementation in src/test/java/io/github/carlos_emr/carlos/test/base/HibernateTemplate.java, positional parameter binding is 1-based (it calls query.setParameter(i + 1, ...)). Therefore, in any test code that calls hibernateTemplate.find(queryString, ...), all HQL positional parameters in queryString must use ?1, ?2, etc. Never use ?0 (or other 0-based indices), otherwise Hibernate will raise a runtime parameter binding error.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-05-24T01:02:42.707Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/webserv/rest/ReportingServiceUnitTest.java:163-270
Timestamp: 2026-05-24T01:02:42.707Z
Learning: In this repository’s Java test code (under src/test/java), CRUD/operation tags on test methods such as Tag("create"), Tag("read"), or Tag("query") are optional “filtering vocabulary” and are NOT required. Since CI does not enforce these CRUD/operation tags, do not flag missing CRUD/operation Tag values on test methods. Only enforce the mandatory test-type tags defined in CLAUDE.md (e.g., Tag("unit"), Tag("fast")).
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-05-24T01:02:47.540Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/UtilDateUtilitiesUnitTest.java:42-44
Timestamp: 2026-05-24T01:02:47.540Z
Learning: In this repository, the `CarlosUnitTestBase` test base class is intended only for tests that need to mock `SpringUtils` and `LogAction` statics. For pure static-method unit tests (e.g., tests like `UtilDateUtilitiesUnitTest`, `SafeEncodeUnitTest`, `QueryAppenderUnitTest`, `TextualizerUnitTest`, `RequestNegotiationUnitTest`), do not require/flag extending `CarlosUnitTestBase`—absence of `CarlosUnitTestBase` is expected when static mocking of `SpringUtils`/`LogAction` isn’t needed.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-05-24T01:02:46.757Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/DateUtilsConvertDate8CharUnitTest.java:39-42
Timestamp: 2026-05-24T01:02:46.757Z
Learning: When reviewing this repo’s Java code that uses CRUD/operation method `Tag(...)` annotations, treat most operation tags (e.g., `Tag("read")`) as optional filtering vocabulary. Only `Tag("integration")` and `Tag("dao")` are required for type/layer classification. Because CI (e.g., `bdd-test-naming.yml`/Surefire) enforces only method naming conventions and does not verify operation tags, do not raise review issues solely for missing optional CRUD operation `Tag` annotations.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-05-24T01:02:47.540Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/UtilDateUtilitiesUnitTest.java:42-44
Timestamp: 2026-05-24T01:02:47.540Z
Learning: In this repository’s JUnit 5 tests, treat CRUD/operation tags (Tag("read"), Tag("create"), Tag("update"), Tag("delete")) as optional filtering vocabulary. Only Tag("integration") and Tag("dao") are required for type/layer classification. If a test is missing one or more CRUD/operation tags, do not flag it as a required change; only enforce what CI actually checks (method/test naming conventions) and rely on Tag("integration")/Tag("dao") for the relevant classification.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-05-24T01:02:48.955Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/utility/CachedDateFormatsUnitTest.java:48-50
Timestamp: 2026-05-24T01:02:48.955Z
Learning: For this repo’s Java test code (src/test/java/**), do not require CRUD/operation tag annotations like Tag("read") or Tag("write") on test classes or test methods. Treat those CRUD/operation tags as optional filtering vocabulary only. For layer/type distinction, only Tag("integration") and Tag("dao") are required. Code review should avoid flagging missing Tag("read"/"write") (or similar CRUD operation tags) when tests otherwise follow the CI-enforced naming conventions.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-05-24T01:02:46.757Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/DateUtilsConvertDate8CharUnitTest.java:39-42
Timestamp: 2026-05-24T01:02:46.757Z
Learning: In this repo, `CarlosUnitTestBase` is intended only for tests that need to mock the `SpringUtils` / `LogAction` statics. For unit tests that only exercise pure static methods (e.g., `DateUtils`, `SafeEncode`, `QueryAppender`, `Textualizer`, `RequestNegotiation`) and do not interact with those statics, do not extend `CarlosUnitTestBase`; keep the test self-contained to avoid unnecessary setup with no benefit.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-08-05T17:23:50.450Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 3283
File: src/test/java/io/github/carlos_emr/carlos/documentManager/IncomingDocumentAssetRegressionTest.java:36-65
Timestamp: 2026-08-05T17:23:50.450Z
Learning: In the CARLOS repository, verify active Checkstyle configuration before claiming that CI enforces BDD-style underscore naming for test methods. The `MethodName` module is disabled in `utils/checkstyle.xml`, so tests under `src/test/java` may pass repository naming checks without that format; only claim enforcement when another active mechanism (such as a test, plugin, or CI rule) confirms it.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java
📚 Learning: 2026-03-17T16:13:20.049Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 650
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarConsultationRequest/config/pageUtil/CpsoSearch2Action.java:64-67
Timestamp: 2026-03-17T16:13:20.049Z
Learning: In this repository, field-level initialization of request via ServletActionContext.getRequest(), response via ServletActionContext.getResponse(), and securityInfoManager via SpringUtils.getBean(SecurityInfoManager.class) is the established pattern for all *2Action.java classes. Do not flag these initializations as risks or suggest moving them into execute(). Reviewers should only flag deviations from this convention if there is a documented, enforceable rationale (e.g., tests or refactoring notes) that justify a different initialization approach.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
📚 Learning: 2026-03-23T01:03:08.159Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 715
File: src/main/java/io/github/carlos_emr/carlos/commn/web/FindNextAvailableSlot2Action.java:91-94
Timestamp: 2026-03-23T01:03:08.159Z
Learning: For carlos-emr/carlos, do not raise a review finding when an action omits a null/expired-session guard on the result of LoggedInInfo.getLoggedInInfoFromSession(request) before calling securityInfoManager.hasPrivilege(). This repo uses a systemic, established pattern across existing *2Action.java classes; treat adding such a guard as a repo-wide improvement rather than an individual per-action PR issue.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
📚 Learning: 2026-03-31T15:52:59.415Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 736
File: src/main/java/io/github/carlos_emr/carlos/billings/ca/on/pageUtil/BillingDocumentErrorReportUpload2Action.java:37-38
Timestamp: 2026-03-31T15:52:59.415Z
Learning: In this repository, within `*2Action.java` classes, do not treat assignments or uses of `UploadedFile.getAbsolutePath()` (from Struts `UploadedFilesAware`) as a path traversal risk. The returned value is a framework-managed temporary file path, not user-controlled input. Only flag traversal/path issues when dealing with destination paths or user-controlled filenames such as `uploaded.getOriginalName()`. Ensure user-controlled paths/filenames are validated separately (e.g., via `PathValidationUtils.validatePath()` before any file write), and don’t require `PathValidationUtils.validateUpload()` solely for `getAbsolutePath()` usage.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
📚 Learning: 2026-04-04T04:11:45.844Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 937
File: src/main/java/io/github/carlos_emr/carlos/documentManager/actions/ManageDocument2Action.java:728-733
Timestamp: 2026-04-04T04:11:45.844Z
Learning: In PRs, when reviewing logging in *Action/*Manager/*Service/*Dao classes, treat the decision to include document filenames/paths in logs as an architectural/policy choice (not a log-sanitization/injection issue). Do not flag logging of sanitized document filenames/paths that have been passed through LogSanitizer as PHI exposure in the context of individual PRs. Limit review scope for “log sanitization” work to CRLF/log-injection prevention (and related injection-safety concerns), not to removing filename/path values from logs, unless there is a separate, enforceable technical reason beyond the injection-sanitization scope.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
📚 Learning: 2026-04-05T04:38:53.740Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1000
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarMeasurements/pageUtil/EctAddMeasurementType2Action.java:82-85
Timestamp: 2026-04-05T04:38:53.740Z
Learning: In Struts2 action classes (e.g., `EctAddMeasurementType2Action` and similar), it’s acceptable to handle validation failures by (1) storing errors on the current request via `request.setAttribute("actionErrors", new ArrayList<>(getActionErrors()))` and (2) returning a result name like `"failure"` that forwards/dispatches to the target view (JSP) while preserving request attributes. Do NOT flag this as “errors lost on redirect” because it is not a redirect. Only raise error-loss concerns when `response.sendRedirect(...)` (or equivalent redirect behavior) is used and the code does not persist errors via a session/flash mechanism before returning `NONE`.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
📚 Learning: 2026-04-05T04:39:00.147Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1000
File: src/main/java/io/github/carlos_emr/carlos/dxresearch/pageUtil/dxResearchUpdateQuickList2Action.java:0-0
Timestamp: 2026-04-05T04:39:00.147Z
Learning: In this Struts2 codebase, when a Struts2 action returns a named result (e.g., "failure") that is mapped in struts XML to another action/JSP, the default result type is `type="dispatcher"` (server-side forward). In this case, original request parameters are preserved and should be available to the forwarded-to target via `StrutsParameter` binding. During code review, do NOT treat missing query/request parameters in the forwarded-to action as data-loss just because they aren’t explicitly listed; only flag missing parameters as a data-loss issue when the code performs a genuine client-side redirect using `response.sendRedirect(...)` and the action returns `NONE` after calling `sendRedirect` (where request parameters are not preserved across the redirect).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java
📚 Learning: 2026-03-15T15:52:50.897Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 644
File: src/main/webapp/admin/updateDrugref.jsp:63-70
Timestamp: 2026-03-15T15:52:50.897Z
Learning: Guideline: For JSPs (any .jsp) that perform fetch() with POST and rely on CSRF tokens not auto-injected by CSRFGuard 4.5, insert a hidden form into the page body: <form method="post" style="display:none;"></form>. This causes CSRFGuardScriptInjectionFilter to inject the CSRF-TOKEN hidden input into that form, making the token available as document.querySelector('input[name="CSRF-TOKEN"]') for inclusion in fetch() request bodies. If getCsrfToken() returns '' as a fallback, ensure it is accompanied by a clear console.warn and that the hidden form guarantees token injection. Apply this pattern to JSPs where manual CSRF token integration is required.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-03-23T01:03:12.952Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 715
File: src/main/webapp/provider/appointmentprovideradminday.jsp:2610-2633
Timestamp: 2026-03-23T01:03:12.952Z
Learning: In this repo, avoid changing only a single caller that invokes `SearchDemographic.do` from GET to POST. If the request method for `SearchDemographic.do` needs to change, do it as one coordinated, global change across *all* JSP callers (so request semantics stay consistent) and include required CSRF token handling as part of that same migration. Do not introduce a mixed GET/POST pattern for this endpoint across the codebase.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-04-06T03:31:35.622Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1116
File: src/main/webapp/billing/CA/BC/genTAS00.jsp:181-183
Timestamp: 2026-04-06T03:31:35.622Z
Learning: When using the OWASP Java Encoder (`Encode.*`) in JSP, select the encoder based on the actual JavaScript context:
- Use `Encode.forJavaScript()` when inserting data into a JavaScript string literal (e.g., values embedded in `'...'?` or inside a `javascript:` href that contains JS code).
- Use `Encode.forJavaScriptAttribute()` only when the JavaScript appears inside an HTML event handler attribute (e.g., `onclick="..."`).
Do not flag `Encode.forJavaScript()` as incorrect merely because it contains JavaScript; only flag it if the target location is an HTML event attribute requiring `forJavaScriptAttribute()`.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-03-17T21:22:37.959Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 667
File: src/main/webapp/form/demographicMeasurementModal.jsp:242-242
Timestamp: 2026-03-17T21:22:37.959Z
Learning: In src/main/webapp/form/demographicMeasurementModal.jsp, showMeasurementDialog() builds bodyHtml from server data (this.dataField, this.measuringInstruction) and unsafely inlines a live DOM value (document.getElementById(elementId).value) into an HTML attribute context before assigning to innerHTML. This is a self-XSS/attribute-injection vector, but it has been assessed as acceptable risk given authenticated users. Do not flag as a critical XSS unless new untrusted input sources are introduced to bodyHtml. Review guidance should be: (1) Treat this as a narrow self-XSS risk, not a general XSS issue; (2) Do not escalate unless you add new untrusted inputs; (3) Document the rationale in code comments and/or review notes; (4) If future changes add untrusted sources, re-evaluate and consider sanitization, stricter encoding, or replacing innerHTML construction with safer DOM manipulation; (5) Consider a minimal CI guard that flags additions of new untrusted inputs to the bodyHtml constructio...
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-03-27T22:41:03.205Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 604
File: src/main/webapp/tickler/ticklerMain.jsp:87-88
Timestamp: 2026-03-27T22:41:03.205Z
Learning: In JSPs, it can be intentional for a Java scriptlet variable (e.g., `isDemoView` inside `<% ... %>`) and a separately named `pageContext` attribute (e.g., `pageContext.setAttribute("hasDemoView", isDemoView)`) to coexist. The scriptlet variable is used within `<% %>` code, while the `pageContext` attribute is accessed via JSTL/EL (e.g., `${hasDemoView}`). These are different namespaces—if the `pageContext` attribute is set before use, do not flag `${...}` as an unresolved/broken reference just because a similarly named scriptlet variable exists with a different name.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-03-27T23:09:08.560Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 724
File: src/main/webapp/billing/CA/BC/billingDeleteWithoutNo.jsp:148-148
Timestamp: 2026-03-27T23:09:08.560Z
Learning: In this codebase’s JSP/JS usage of `BroadcastChannel`, follow the established convention: wrap both the `new BroadcastChannel(...)` instantiation and the `postMessage(...)` call in a `try/catch` block with an empty (or comment-only) catch handler like `catch (e) { /* BroadcastChannel not supported */ }`. Do not re-run the `BroadcastChannel` logic inside the catch block. This repository’s convention does not use feature detection such as `typeof BroadcastChannel === 'undefined'` for this guard.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-03-27T23:09:13.720Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 724
File: src/main/webapp/billing/CA/CLINICAID/billingDeleteWithoutNo.jsp:77-77
Timestamp: 2026-03-27T23:09:13.720Z
Learning: When using `BroadcastChannel` in this repo, follow the established guard convention: wrap `BroadcastChannel` message posting in a `try/catch` and use an empty (or comment-only) `catch` body (e.g., `catch (e) { /* BroadcastChannel not supported */ }`). Do not use `typeof BroadcastChannel !== 'undefined'` as the primary guard; keep this pattern consistent across all JSP files in `src/main/webapp/**`—especially the JSP billing files—and for any code that posts `BroadcastChannel` messages.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-04-06T03:31:14.839Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1116
File: src/main/webapp/billing/CA/BC/support/billingfeeitem.jsp:138-143
Timestamp: 2026-04-06T03:31:14.839Z
Learning: In this repository’s JSP files, when outputting untrusted values that will be embedded inside JavaScript string literals within HTML event handler attributes (e.g., inside `onclick="..."` or any attribute whose value contains JavaScript code with values surrounded by single or double quote delimiters), prefer `Encode.forJavaScript()` as the recommended encoder for the JavaScript-string-literal context. Do not suggest replacing it with `Encode.forJavaScriptAttribute()` for these event-handler/JS-string-literal contexts; both are safe, but `forJavaScript()` is the general-purpose recommended choice for values placed in JS string literals.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-04-06T15:22:52.578Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1147
File: src/main/webapp/demographic/demographicappthistory.jsp:0-0
Timestamp: 2026-04-06T15:22:52.578Z
Learning: For JSP views under src/main/webapp, when looking up multisite attributes from a map (e.g., `siteBgColor`) keyed by a location that may have become inactive/deleted, don’t assume the key exists. Guard against null/empty results before rendering. Pattern: fetch the mapped `String[]` (e.g., `String[] sbc = siteBgColor.get(appointment.getLocation());`), derive `siteColor` and `siteLabel` with null/length checks (fall back to `""` for CSS color and to the location label when the array is missing/too short), then render with `Encode.forCssString(siteColor)` and `Encode.forHtml(siteLabel)` to avoid NPEs and incorrect output.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
📚 Learning: 2026-04-08T02:45:55.926Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1358
File: src/main/webapp/documentManager/showDocument.jsp:481-481
Timestamp: 2026-04-08T02:45:55.926Z
Learning: In this codebase’s JSPs (carlos-emr/carlos), it is an established convention to pass the numeric `docId` unquoted to `ForwardSelectedRows(docId + ':DOC', ...)` (i.e., relying on JS type coercion such as `123 + ":DOC"` → `"123:DOC"`). Do not flag these calls as inconsistent or fragile for missing quotes; do not “fix” by quoting only some occurrences, since that would create inconsistency. Ensure the value used for any JavaScript-context encoding is handled by adding/verifying `Encode.forJavaScriptAttribute(docId)` for `docId` where applicable before embedding it in JavaScript/HTML attributes.
Applied to files:
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp
🔇 Additional comments (12)
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.java (1)
25-176: LGTM!src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleValidationException.java (1)
26-50: LGTM!src/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorTest.java (1)
40-118: LGTM!src/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java (1)
42-158: LGTM!src/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataTest.java (1)
47-156: LGTM!src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExample2Action.java (1)
68-198: LGTM!src/main/resources/oscarResources_en.properties (1)
8926-8931: LGTM!src/main/resources/oscarResources_es.properties (1)
6452-6452: LGTM!Also applies to: 6455-6455
src/main/resources/oscarResources_pl.properties (1)
5870-5870: LGTM!Also applies to: 5873-5873
src/main/resources/oscarResources_pt_BR.properties (1)
7357-7357: LGTM!Also applies to: 7360-7360
src/main/webapp/WEB-INF/jsp/oscarReport/RptByExample.jsp (2)
52-52: LGTM!Also applies to: 71-73, 164-194
242-247: 🔒 Security & PrivacyVerify backend encoding before rendering
resultsas HTML.At Line [248], the JSP inserts
${results}without encoding. Verify thatRptResultStructencodes every database-derived cell value for HTML context before building this string. Add a regression test containing<and&in a result value and confirm that the rendered output contains encoded text.Source: Coding guidelines
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 19 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 19 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. OpenSSF Scorecard
Scanned Files
|
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Collaborator
Author
|
@coderabbitai review |
Contributor
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
Contributor
|
|
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Contributor
There was a problem hiding this comment.
All reported issues were addressed
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Collaborator
Author
|
@coderabbitai review |
Contributor
|
✅ Action performedReview finished.
|
Contributor
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Collaborator
Author
|
@cubic-dev-ai review |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java (1)
87-115: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd contract-level JavaDoc to the public
executemethod.
executevalidates SQL, runs a bounded read-only query, and classifies five distinct outcomes (success,rejected,timeout, SQL failure, runtime failure) while always auditing in thefinallyblock. This control flow is not obvious from the signature alone. Document the parameters, the return value, the exception contract, and the audit-on-finally behavior.📝 Proposed JavaDoc addition
+ /** + * Validates {`@code` sql} through {`@link` QueryByExampleSqlValidator}, executes it as a + * bounded, read-only, forward-only query, and audits the outcome. + * + * <p>The outcome recorded for auditing is one of {`@code` success}, {`@code` rejected}, + * {`@code` timeout}, or {`@code` failed}; audit metadata never includes raw SQL text.</p> + * + * `@param` sql the caller-submitted SQL text; only a single read-only {`@code` SELECT} is permitted + * `@param` properties configuration used to resolve the allowed application schema + * `@param` providerNo the requesting provider's identifier, used only for audit metadata + * `@return` the rendered, bounded query result on success + * `@throws` QueryByExampleValidationException if {`@code` sql} fails structural validation + * `@throws` SQLTimeoutException if execution exceeds {`@link` `#QUERY_TIMEOUT_SECONDS`} + * `@throws` SQLException for other execution failures + */ `@SuppressFBWarnings`( value = "THROWS_METHOD_THROWS_RUNTIMEEXCEPTION", justification = "Runtime failures are audited and handled by the action") public QueryResult execute(String sql, Properties properties, String providerNo) throws SQLException {As per coding guidelines: "Public entrypoint classes and non-obvious public or protected methods should have contract-level JavaDoc."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java` around lines 87 - 115, Add contract-level JavaDoc immediately above the public execute method, documenting its SQL validation and bounded read-only query behavior, all parameters, the QueryResult return value, the QueryByExampleValidationException, SQLTimeoutException, SQLException, and RuntimeException outcomes, and that auditing occurs in finally for every outcome.Source: Coding guidelines
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java (1)
65-75: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd contract-level JavaDoc to the changed public methods.
execute(),write2Database(...), anddeleteQuery(...)expose non-obvious authorization, HTTP-method, provider-ownership, persistence, and exception behavior without method JavaDoc. Add parameter, return-type, and applicable exception documentation.As per coding guidelines, JavaDoc is required on all public classes and methods, including parameters, exact return types, and thrown exceptions.
Also applies to: 108-129, 131-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java` around lines 65 - 75, Add contract-level JavaDoc to the public methods execute(), write2Database(...), and deleteQuery(...) in RptByExamplesFavorite2Action, documenting each parameter, the exact return value, authorization and HTTP-method behavior, provider-ownership and persistence semantics, and all applicable exceptions including ServletException and IOException.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java`:
- Around line 87-115: Add contract-level JavaDoc immediately above the public
execute method, documenting its SQL validation and bounded read-only query
behavior, all parameters, the QueryResult return value, the
QueryByExampleValidationException, SQLTimeoutException, SQLException, and
RuntimeException outcomes, and that auditing occurs in finally for every
outcome.
In
`@src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java`:
- Around line 65-75: Add contract-level JavaDoc to the public methods execute(),
write2Database(...), and deleteQuery(...) in RptByExamplesFavorite2Action,
documenting each parameter, the exact return value, authorization and
HTTP-method behavior, provider-ownership and persistence semantics, and all
applicable exceptions including ServletException and IOException.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fca1cace-6717-42aa-9205-ec00dacf60d2
📒 Files selected for processing (13)
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/resources/oscarResources_es.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_pt_BR.propertiessrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: tests
- GitHub Check: SpotBugs + Find Security Bugs
🧰 Additional context used
📓 Path-based instructions (15)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Useio.github.carlos_emr.carlos.*namespace for ALL new code; specific packages: DAOs inio.github.carlos_emr.carlos.commn.dao.*, Models inio.github.carlos_emr.carlos.commn.model.*, exception: ProviderDao inio.github.carlos_emr.carlos.dao.ProviderDao
UsePathValidationUtilsfor ALL file path operations that include user input
NEVER log or expose PHI (Protected Health Information) in logs or error messages
UseSpringUtils.getBean(ClassName.class)for Spring bean injection in Struts2 actions and non-Spring-managed classes
JavaDoc is required on all public classes and methods; do NOT include@authortags, use@sincetags with git log timestamps, use CARLOS copyright header for new files
**/*.java: Useio.github.carlos_emr.carlos.*for all new Java code, includingcommn.dao,commn.model, andcommn.dao.forms; retain legacyorg.oscarehr.common.dao.*only for test utilities andProviderDaoat its documented exception path.
Use parameterized queries only; never concatenate user input or variables into SQL or HQL.
UsePathValidationUtilsfor all file operations involving user input, including path components, existing paths, and uploads; use the returned validated value for subsequent filesystem operations.
Use sanctioned layer suffixes such as*Action,*Loader,*Resolver,*Validator,*Persister,*Calculator,*Parser,*Service,*Dao,*Dto, and*Command; do not introduce*Prep,*Manager,*Helper,*Utils, or compound role suffixes.
Public entrypoint classes and non-obvious public or protected methods should have contract-level JavaDoc; do not add@authortags, preserve accurate GPL headers, and never remove existing copyright notices.
**/*.java: Use parameterized queries exclusively; never construct SQL through string concatenation.
UseSecurityInfoManager.hasPrivilege()checks for every action, especially all Struts2*2Actionclasses.
UsePathValidationUtilsfor all file operations involving ...
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{jsp,java}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use OWASP Encoder for ALL user input output: in JSP use
${e:forHtml(value)}with taglib<%@ taglib uri="owasp.encoder.jakarta" prefix="e" %>, in Java useEncode.forHtml(value), with context-specific variants (forHtmlAttribute,forJavaScript,forCssString,forUri,forUriComponent)Use OWASP Encoder functions for every user-controlled output, selecting the encoder for the correct HTML, attribute, JavaScript, CSS, or URL context.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,sql}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use parameterized queries ONLY in SQL/HQL operations; never use string concatenation for SQL queries
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp,js,ts,xml,sql,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Use “CARLOS” or “CARLOS EMR” in all user-facing content; preserve
io.github.carlos_emr.carlos.*namespaces and legacy technical references where required.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{java,jsp}: Protect all user input with context-appropriate CARLOS null-safe OWASP encoding; do not introduce raw OWASPEncode.forXxx,<e:forXxx>,${e:forXxx(...)},<c:out>, orfn:escapeXml()for new output.
Do not log or expose PHI; sanitize necessary operational identifiers withLogSafe, avoid clinical context, and do not place identifiers in browser-visible exceptions unless explicitly required by an authorized workflow.Add audit logging for patient-data access and use healthcare-specific validation while maintaining PHI protection.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*Dao.java
📄 CodeRabbit inference engine (CLAUDE.md)
DAOs must not inject other DAOs; cross-DAO orchestration belongs in a service layer.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.java
**/*.{java,jsp,xml}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{java,jsp,xml}: Preserve existing copyright notices and GPL versions; new files must use the CARLOS project header.
Use Java-style billing acronym casing (On,Ohip,Ra,Moh, etc.), allow onlyDiagas the short billing form, and avoid compressed legacy names such as3rd,Dig,Db,Obj,Hlp,Bean, andHandler.
Healthcare code must protect PHI, include appropriate authorization and audit behavior, and follow relevant provincial and healthcare integration standards.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp,js,ts,xml,sql}
📄 CodeRabbit inference engine (GEMINI.md)
Protect PHI: never log or expose patient health information, and apply appropriate privacy and audit controls.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
src/main/java/**/*.java
📄 CodeRabbit inference engine (GEMINI.md)
src/main/java/**/*.java: Use theio.github.carlos_emr.carlos.*namespace for all new production Java code; retainorg.oscarehr.*andoscar.*only where required for legacy compatibility.
Provide comprehensive JavaDoc for all public classes and methods; document parameters, exact return types, thrown exceptions, and deprecations with migration guidance; do not add@authortags and use accurate@sincehistory.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,jsp,xml,js,css,sql,md}
📄 CodeRabbit inference engine (GEMINI.md)
Preserve existing copyright notices and GPL versions; use the CARLOS copyright header for new files and do not remove existing attribution.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*.{java,xml,jsp,sql}
📄 CodeRabbit inference engine (GEMINI.md)
Use the documented healthcare integration standards and domain conventions for HL7, FHIR R4, SNOMED CT, ICD coding, ATC medication codes, DICOM, and provincial systems.
Files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
**/*Test.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*Test.java: Unit tests must useCarlosUnitTestBasebase class for mocked tests (no database); integration tests must useCarlosTestBasefor H2 database tests
Follow BDD naming convention for test methods:should<Action>_<preposition><Condition>()with ONE underscore, camelCase, andshouldprefixUse hierarchical JUnit tags such as integration/unit, DAO or manager layer, and CRUD or query operation tags to support filtered test execution.
Files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
src/test/java/**/*.java
📄 CodeRabbit inference engine (CLAUDE.md)
src/test/java/**/*.java: Use JUnit 5 tests undersrc/test/, select the appropriateCarlosTestBase,CarlosUnitTestBase, or domain-specific base, and inspect the actual production interface before writing tests.
Name test methodsshould<Action>_<context><Condition>()with exactly one underscore, camelCase, a requiredshouldprefix, and a lowercase segment after the underscore.
For HibernateDaoSupport integration tests, flush withhibernateTemplate.flush()rather than onlyentityManager.flush(); use proper boolean HQL comparisons and explicit%wildcards for LIKE test inputs.
For manager tests, register SpringUtils mocks before static mocks, close static mocks after each test, organize large suites with@Nested, and use the sharedLogCapturefor Log4j2 assertions.
Files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
**/*2Action.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*2Action.java: IncludeSecurityInfoManager.hasPrivilege()checks as the FIRST operation in all Struts2 action execute() methods
All new Struts2 actions must follow the *2Action.java naming pattern and extendorg.apache.struts2.ActionSupport(NOTcom.opensymphony.xwork2.*)
**/*2Action.java: Every new Struts 2 action must use the*2Action.javanaming convention, generally extendorg.apache.struts2.ActionSupport, use constructor injection, and performSecurityInfoManager.hasPrivilege()checks.
Mutating*2Actionclasses must reject GET and HEAD before any side effect, and their contract-test manifest must classify newly discovered mutators.
Actions that write directly to servlet responses must returnNONEafter writing, own their error response, and avoid named-result forwarding after response output begins.
Files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
src/main/java/**/*2Action.java
📄 CodeRabbit inference engine (GEMINI.md)
All new Struts2 actions must use the
*2Action.javanaming convention, extend the appropriate Struts action base, and useorg.apache.struts2.*rather than deprecatedcom.opensymphony.xwork2.*packages.
Files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
🧠 Learnings (34)
📚 Learning: 2026-02-21T01:06:58.978Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 443
File: src/main/resources/oscarResources_en.properties:34-57
Timestamp: 2026-02-21T01:06:58.978Z
Learning: In user-facing properties files (e.g., src/main/resources/oscarResources_en.properties), avoid introducing new brand domains (e.g., a new carlosgalaxy domain) in strings. When updating links for branding (such as loginApplication.image.logoText), prefer a neutral, stable link like the GitHub repo with https, or omit the link entirely if no suitable neutral target exists. Ensure all updates keep branding consistent and do not introduce new external domains in UI strings.
Applied to files:
src/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_es.properties
📚 Learning: 2026-05-09T19:22:01.986Z
Learnt from: phc007
Repo: carlos-emr/carlos PR: 2120
File: src/main/resources/oscarResources_pt_BR.properties:8007-8028
Timestamp: 2026-05-09T19:22:01.986Z
Learning: In this repo’s resource bundle `.properties` files under `src/main/resources/` (e.g., `oscarResources_pt_BR.properties`), you can write non-ASCII characters directly in UTF-8 without using Java-style Unicode escapes like `\u00F3`, since the application runs on Tomcat 11 + Java 21 (default UTF-8). Save these files in UTF-8 and only fall back to `\uXXXX` if a specific entry must be compatible with a different decoding setup.
Applied to files:
src/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_es.properties
📚 Learning: 2026-07-28T17:19:42.866Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 2440
File: src/main/resources/oscarResources_fr.properties:605-606
Timestamp: 2026-07-28T17:19:42.866Z
Learning: For locale-specific `.properties` resource bundles (e.g., `*_<locale>.properties`), if a translation for a key is missing, keep the value as the English fallback and place a `# TODO: translate` comment directly on the line immediately above the affected property key. Treat these `# TODO: translate` placeholders as intentional: during code review, do not recommend translating/replacing them unless a maintainer/translator provides a verified translation.
Applied to files:
src/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_es.properties
📚 Learning: 2026-07-28T17:19:45.658Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 2440
File: src/main/resources/oscarResources_pt_BR.properties:726-727
Timestamp: 2026-07-28T17:19:45.658Z
Learning: For i18n .properties files, follow docs/I18N-STANDARDS.md’s convention for intentionally-missing locale translations: when a key is missing for a locale, use an English fallback value and add a “# TODO: translate” comment immediately above that key. In code review, do not recommend translating these placeholder/fallback values, since they are intentionally left in English until a trusted translation is provided.
Applied to files:
src/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_es.properties
📚 Learning: 2026-03-17T16:16:27.811Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 650
File: src/main/resources/oscarResources_pl.properties:0-0
Timestamp: 2026-03-17T16:16:27.811Z
Learning: In the repo carlos-emr/carlos, i18n wording refinements for files matching src/main/resources/oscarResources_*.properties are out of scope for feature PRs. Track and fix these refinements separately (e.g., via issues or a dedicated i18n PR) so feature changes aren’t blocked by localization wording edits.
Applied to files:
src/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_es.properties
📚 Learning: 2026-07-28T17:19:45.547Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 2440
File: src/main/resources/oscarResources_pl.properties:646-647
Timestamp: 2026-07-28T17:19:45.547Z
Learning: For any missing locale translation entries in `src/main/resources/oscarResources_*.properties`, use the English fallback value and place a `# TODO: translate` comment immediately above the affected property key. Treat these as approved placeholders per `docs/I18N-STANDARDS.md`; do not flag them as issues requiring immediate translation.
Applied to files:
src/main/resources/oscarResources_pt_BR.propertiessrc/main/resources/oscarResources_pl.propertiessrc/main/resources/oscarResources_fr.propertiessrc/main/resources/oscarResources_es.properties
📚 Learning: 2026-03-06T23:54:01.319Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 563
File: src/main/java/io/github/carlos_emr/carlos/lab/ca/all/parsers/AlphaHandler.java:59-63
Timestamp: 2026-03-06T23:54:01.319Z
Learning: In the carlos-emr/carlos repository, for Java sources under src/main/java, empty no-arg constructors should have only a minimal one-line JavaDoc. Do not add since or other metadata tags to empty constructors, as they provide no meaningful value. Place documentation efforts in the class-level JavaDoc instead.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-03-13T21:29:23.775Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 590
File: src/main/java/io/github/carlos_emr/carlos/eform/actions/DisplayImage2Action.java:69-69
Timestamp: 2026-03-13T21:29:23.775Z
Learning: When reviewing Java source files, allow since Javadoc tags to reflect substantial rewrites or major API changes (e.g., security overhaul, new API surface). Do not flag the tag as incorrect solely because the file predates the tagged date. If flagging is considered, verify against commit history or release notes to confirm the tag corresponds to a meaningful change, not the original introduction. Apply this guidance across Java sources in the repository (src/main/java/**/*.java).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-04-11T22:46:51.202Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1556
File: src/main/java/io/github/carlos_emr/carlos/demographic/dto/DemographicListItemDTO.java:74-179
Timestamp: 2026-04-11T22:46:51.202Z
Learning: In this repository, do not require method-level JavaDoc for plain public JavaBean-style getters and setters in Java files under src/main/java/**/*.java. Their meaning should be covered by field-level documentation and/or the class-level JavaDoc. Only require JavaDoc for non-trivial public methods (e.g., constructors with logic, factory methods, and computed/derived accessors where behavior is not just returning/setting a field). In code reviews, missing JavaDoc on simple getters/setters should not be flagged as a documentation violation.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-04-11T22:47:04.719Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1556
File: src/main/java/io/github/carlos_emr/carlos/provider/dto/ProviderSummaryDTO.java:57-104
Timestamp: 2026-04-11T22:47:04.719Z
Learning: In this repository, do not require JavaDoc for trivial getter and setter methods. Only JavaDoc for types/classes and for more meaningful public API points is expected—e.g., class-level JavaDoc, constructors, static factory methods, and non-trivial public methods (such as formatting helpers). In code reviews, ignore missing JavaDoc on getters/setters.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-05-24T01:01:46.756Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/main/java/io/github/carlos_emr/carlos/utility/CachedDateFormats.java:165-216
Timestamp: 2026-05-24T01:01:46.756Z
Learning: For Java sources under src/main/java/**/*.java, follow the repository’s Documentation Standards (CLAUDE.md) regarding Javadoc requirements: do not require mechanical param/return/throws tags for trivial wrapper/delegator public methods that are essentially one-line pass-throughs to a cached or underlying instance (e.g., methods like parseDefault/formatDefault/parse/format in CachedDateFormats.java). Instead, rely on class-level Javadoc to document the overall contracts, including thread-safety, null handling, return value semantics, and any mutation/restoration behavior (such as timezone restoration). Require full per-method Javadoc tag coverage only for non-trivial public methods whose behavior is not obvious from the context/signature.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-06-25T16:10:08.376Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 3023
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarConsultationRequest/pageUtil/EConsult2Action.java:287-328
Timestamp: 2026-06-25T16:10:08.376Z
Learning: When reviewing Java code that reconstructs an origin/authority from a `java.net.URI` (e.g., `scheme://host:port`), assume `URI#getHost()` may already return bracketed IPv6 literals such as `[::1]` (not just `::1`). Do not “re-bracket” IPv6 manually when rebuilding the origin from the host returned by `getHost()`. If `getHost()` is `null`, only then apply the malformed-colon/authority fallback rejection logic on that authority-fallback path (the path that runs when `getHost()` is `null`), not on the normal host-based reconstruction path.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-03-24T17:34:18.508Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 716
File: src/main/java/io/github/carlos_emr/carlos/eform/actions/RtlPreventions2Action.java:155-156
Timestamp: 2026-03-24T17:34:18.508Z
Learning: In this repository (carlos-emr/carlos), treat `demographic_no` as an internal database surrogate integer key (the PK of the demographic table), not as PHI. When reviewing code, do not flag logging or output of `demographic_no` as PHI exposure. PHI to flag includes clinical/demographic data elements like patient name, date of birth, HIN, and address.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-04-07T02:57:10.703Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1111
File: src/main/java/io/github/carlos_emr/carlos/util/JDBCUtil.java:140-142
Timestamp: 2026-04-07T02:57:10.703Z
Learning: When reviewing this repository’s Java code, flag any unprotected use of `SchemaFactory.newInstance(...)` (e.g., instances that do not restrict `XMLConstants.ACCESS_EXTERNAL_DTD` and `XMLConstants.ACCESS_EXTERNAL_SCHEMA` to empty strings). Recommend migrating those call sites to `io.github.carlos_emr.carlos.utility.XmlUtils.createSecureSchemaFactory()` so XML Schema validation is hardened against external entity/schema access. Known migrated call sites include `IndicatorTemplateHandler.java` and `DemographicExportAction42Action.java` (post-PR `#1111`).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/db/LegacyJdbcQuery.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.javasrc/main/java/io/github/carlos_emr/carlos/report/data/RptByExampleData.java
📚 Learning: 2026-04-02T02:30:55.977Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 780
File: src/main/java/io/github/carlos_emr/carlos/PMmodule/dao/SecUserRoleDaoImpl.java:105-107
Timestamp: 2026-04-02T02:30:55.977Z
Learning: During code reviews of Hibernate 7 deprecation-fix / `get()`→`find()` migration PRs in the repo, do not flag pre-existing call sites where potentially null/unchecked primary-key values are passed directly into `Session.get()` or `Session.find()` as a new regression. This pattern is already widespread across DAO implementations and the involved methods exhibit identical `IllegalArgumentException` behavior for null keys. If null-guard hardening is desired, require it to be tracked as a separate, repo-wide improvement PR rather than as part of the migration fix.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.java
📚 Learning: 2026-04-04T04:11:45.844Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 937
File: src/main/java/io/github/carlos_emr/carlos/documentManager/actions/ManageDocument2Action.java:728-733
Timestamp: 2026-04-04T04:11:45.844Z
Learning: In PRs, when reviewing logging in *Action/*Manager/*Service/*Dao classes, treat the decision to include document filenames/paths in logs as an architectural/policy choice (not a log-sanitization/injection issue). Do not flag logging of sanitized document filenames/paths that have been passed through LogSanitizer as PHI exposure in the context of individual PRs. Limit review scope for “log sanitization” work to CRLF/log-injection prevention (and related injection-safety concerns), not to removing filename/path values from logs, unless there is a separate, enforceable technical reason beyond the injection-sanitization scope.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.javasrc/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
📚 Learning: 2026-04-08T00:54:14.162Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1358
File: src/main/java/io/github/carlos_emr/carlos/commn/dao/ProviderLabRoutingDaoImpl.java:182-185
Timestamp: 2026-04-08T00:54:14.162Z
Learning: When reviewing this project’s DAO implementations in Java, do not flag the use of JPQL string queries created via `entityManager.createQuery(...)` as a blocker/major issue if the JPQL already uses named or positional parameters (e.g., `:param` / `?1` bound via `setParameter`). Treat Criteria API migration as an optional refactoring preference, not a correctness/security concern, and keep it out of scope for feature/bug-fix PRs. Only flag JPQL when the query is built using unsafe string concatenation that prevents parameter binding (i.e., values are inlined into the JPQL instead of bound parameters).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/commn/dao/ReportByExamplesFavoriteDao.java
📚 Learning: 2026-03-21T14:58:44.822Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 704
File: src/test/java/io/github/carlos_emr/carlos/commn/dao/Hl7TextInfoDaoIntegrationTest.java:109-110
Timestamp: 2026-03-21T14:58:44.822Z
Learning: For the test-only HibernateTemplate implementation in src/test/java/io/github/carlos_emr/carlos/test/base/HibernateTemplate.java, positional parameter binding is 1-based (it calls query.setParameter(i + 1, ...)). Therefore, in any test code that calls hibernateTemplate.find(queryString, ...), all HQL positional parameters in queryString must use ?1, ?2, etc. Never use ?0 (or other 0-based indices), otherwise Hibernate will raise a runtime parameter binding error.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-05-24T01:02:42.707Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/webserv/rest/ReportingServiceUnitTest.java:163-270
Timestamp: 2026-05-24T01:02:42.707Z
Learning: In this repository’s Java test code (under src/test/java), CRUD/operation tags on test methods such as Tag("create"), Tag("read"), or Tag("query") are optional “filtering vocabulary” and are NOT required. Since CI does not enforce these CRUD/operation tags, do not flag missing CRUD/operation Tag values on test methods. Only enforce the mandatory test-type tags defined in CLAUDE.md (e.g., Tag("unit"), Tag("fast")).
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-05-24T01:02:47.540Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/UtilDateUtilitiesUnitTest.java:42-44
Timestamp: 2026-05-24T01:02:47.540Z
Learning: In this repository, the `CarlosUnitTestBase` test base class is intended only for tests that need to mock `SpringUtils` and `LogAction` statics. For pure static-method unit tests (e.g., tests like `UtilDateUtilitiesUnitTest`, `SafeEncodeUnitTest`, `QueryAppenderUnitTest`, `TextualizerUnitTest`, `RequestNegotiationUnitTest`), do not require/flag extending `CarlosUnitTestBase`—absence of `CarlosUnitTestBase` is expected when static mocking of `SpringUtils`/`LogAction` isn’t needed.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-05-24T01:02:46.757Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/DateUtilsConvertDate8CharUnitTest.java:39-42
Timestamp: 2026-05-24T01:02:46.757Z
Learning: When reviewing this repo’s Java code that uses CRUD/operation method `Tag(...)` annotations, treat most operation tags (e.g., `Tag("read")`) as optional filtering vocabulary. Only `Tag("integration")` and `Tag("dao")` are required for type/layer classification. Because CI (e.g., `bdd-test-naming.yml`/Surefire) enforces only method naming conventions and does not verify operation tags, do not raise review issues solely for missing optional CRUD operation `Tag` annotations.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-05-24T01:02:47.540Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/UtilDateUtilitiesUnitTest.java:42-44
Timestamp: 2026-05-24T01:02:47.540Z
Learning: In this repository’s JUnit 5 tests, treat CRUD/operation tags (Tag("read"), Tag("create"), Tag("update"), Tag("delete")) as optional filtering vocabulary. Only Tag("integration") and Tag("dao") are required for type/layer classification. If a test is missing one or more CRUD/operation tags, do not flag it as a required change; only enforce what CI actually checks (method/test naming conventions) and rely on Tag("integration")/Tag("dao") for the relevant classification.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-05-24T01:02:48.955Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/utility/CachedDateFormatsUnitTest.java:48-50
Timestamp: 2026-05-24T01:02:48.955Z
Learning: For this repo’s Java test code (src/test/java/**), do not require CRUD/operation tag annotations like Tag("read") or Tag("write") on test classes or test methods. Treat those CRUD/operation tags as optional filtering vocabulary only. For layer/type distinction, only Tag("integration") and Tag("dao") are required. Code review should avoid flagging missing Tag("read"/"write") (or similar CRUD operation tags) when tests otherwise follow the CI-enforced naming conventions.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-08-07T04:54:41.609Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 3345
File: src/test/java/io/github/carlos_emr/carlos/report/data/RptFluReportDataUnitTest.java:48-49
Timestamp: 2026-08-07T04:54:41.609Z
Learning: In this repository, JUnit 5 test classes that extend `io.github.carlos_emr.carlos.test.unit.CarlosUnitTestBase` inherit its class-level `Tag("unit")` and `Tag("fast")` annotations. Do not flag these subclasses for omitting a duplicate `Tag("unit")`; the inherited tags satisfy JUnit test filtering.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-04-25T21:36:29.428Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1967
File: src/test/java/io/github/carlos_emr/carlos/billings/ca/on/pageUtil/ViewBillingShortcutPg12ActionUnitTest.java:86-86
Timestamp: 2026-04-25T21:36:29.428Z
Learning: In tests that extend `CarlosUnitTestBase`, do not flag unused Mockito stubs in `BeforeEach` setup methods as actionable issues. `CarlosUnitTestBase` does not enable Mockito strict-stubs mode, and there is no current plan to turn it on, so unused stubs will not trigger `UnnecessaryStubbingException`.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-04-25T21:36:34.936Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1967
File: src/test/java/io/github/carlos_emr/carlos/utility/ErrorPageLoggerUnitTest.java:15-15
Timestamp: 2026-04-25T21:36:34.936Z
Learning: In this repo’s unit test files, there is an intentional copy-paste-friendly convention for inner test helpers (e.g., a `private static final class CapturingAppender` extending `AbstractAppender`): it may use fully-qualified types like `java.util.List<LogEvent>` and `new java.util.ArrayList<>()` even when the outer test class already imports `List`. During code review, do not flag these as redundant-FQN or inconsistent-import nitpicks when they follow this convention for such inner appender/test helper classes.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-05-24T01:02:48.124Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/utility/DateUtilsFormatUnitTest.java:32-34
Timestamp: 2026-05-24T01:02:48.124Z
Learning: When reviewing this repository’s unit tests for pure static utility methods (e.g., classes like DateUtils.format, SafeEncodeUnitTest, QueryAppenderUnitTest, TextualizerUnitTest, RequestNegotiationUnitTest), do not require them to extend CarlosUnitTestBase or to include CRUD/operation-level tags. These tests are intended to use only Tag("unit") with no base class and no operation-level tags because CarlosUnitTestBase exists specifically to mock the SpringUtils/LogAction statics. Per CLAUDE.md and current CI enforcement (bdd-test-naming.yml/surefire), only method naming conventions are enforced for this category; missing operation tags and missing CarlosUnitTestBase should not be flagged. (Still flag issues if the test is not actually for pure static utility methods, or if it violates the required Tag("integration")/Tag("dao") expectations for the categories that require them.)
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-05-24T01:02:46.757Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1832
File: src/test/java/io/github/carlos_emr/carlos/util/DateUtilsConvertDate8CharUnitTest.java:39-42
Timestamp: 2026-05-24T01:02:46.757Z
Learning: In this repo, `CarlosUnitTestBase` is intended only for tests that need to mock the `SpringUtils` / `LogAction` statics. For unit tests that only exercise pure static methods (e.g., `DateUtils`, `SafeEncode`, `QueryAppender`, `Textualizer`, `RequestNegotiation`) and do not interact with those statics, do not extend `CarlosUnitTestBase`; keep the test self-contained to avoid unnecessary setup with no benefit.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-08-05T17:23:50.450Z
Learnt from: Ben-Heerema
Repo: carlos-emr/carlos PR: 3283
File: src/test/java/io/github/carlos_emr/carlos/documentManager/IncomingDocumentAssetRegressionTest.java:36-65
Timestamp: 2026-08-05T17:23:50.450Z
Learning: In the CARLOS repository, verify active Checkstyle configuration before claiming that CI enforces BDD-style underscore naming for test methods. The `MethodName` module is disabled in `utils/checkstyle.xml`, so tests under `src/test/java` may pass repository naming checks without that format; only claim enforcement when another active mechanism (such as a test, plugin, or CI rule) confirms it.
Applied to files:
src/test/java/io/github/carlos_emr/carlos/db/LegacyJdbcQueryUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidatorUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/ReportActionSecurityMigrationUnitTest.javasrc/test/java/io/github/carlos_emr/carlos/report/data/RptByExampleDataUnitTest.java
📚 Learning: 2026-03-17T16:13:20.049Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 650
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarConsultationRequest/config/pageUtil/CpsoSearch2Action.java:64-67
Timestamp: 2026-03-17T16:13:20.049Z
Learning: In this repository, field-level initialization of request via ServletActionContext.getRequest(), response via ServletActionContext.getResponse(), and securityInfoManager via SpringUtils.getBean(SecurityInfoManager.class) is the established pattern for all *2Action.java classes. Do not flag these initializations as risks or suggest moving them into execute(). Reviewers should only flag deviations from this convention if there is a documented, enforceable rationale (e.g., tests or refactoring notes) that justify a different initialization approach.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
📚 Learning: 2026-03-23T01:03:08.159Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 715
File: src/main/java/io/github/carlos_emr/carlos/commn/web/FindNextAvailableSlot2Action.java:91-94
Timestamp: 2026-03-23T01:03:08.159Z
Learning: For carlos-emr/carlos, do not raise a review finding when an action omits a null/expired-session guard on the result of LoggedInInfo.getLoggedInInfoFromSession(request) before calling securityInfoManager.hasPrivilege(). This repo uses a systemic, established pattern across existing *2Action.java classes; treat adding such a guard as a repo-wide improvement rather than an individual per-action PR issue.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
📚 Learning: 2026-03-31T15:52:59.415Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 736
File: src/main/java/io/github/carlos_emr/carlos/billings/ca/on/pageUtil/BillingDocumentErrorReportUpload2Action.java:37-38
Timestamp: 2026-03-31T15:52:59.415Z
Learning: In this repository, within `*2Action.java` classes, do not treat assignments or uses of `UploadedFile.getAbsolutePath()` (from Struts `UploadedFilesAware`) as a path traversal risk. The returned value is a framework-managed temporary file path, not user-controlled input. Only flag traversal/path issues when dealing with destination paths or user-controlled filenames such as `uploaded.getOriginalName()`. Ensure user-controlled paths/filenames are validated separately (e.g., via `PathValidationUtils.validatePath()` before any file write), and don’t require `PathValidationUtils.validateUpload()` solely for `getAbsolutePath()` usage.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
📚 Learning: 2026-04-05T04:38:53.740Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1000
File: src/main/java/io/github/carlos_emr/carlos/encounter/oscarMeasurements/pageUtil/EctAddMeasurementType2Action.java:82-85
Timestamp: 2026-04-05T04:38:53.740Z
Learning: In Struts2 action classes (e.g., `EctAddMeasurementType2Action` and similar), it’s acceptable to handle validation failures by (1) storing errors on the current request via `request.setAttribute("actionErrors", new ArrayList<>(getActionErrors()))` and (2) returning a result name like `"failure"` that forwards/dispatches to the target view (JSP) while preserving request attributes. Do NOT flag this as “errors lost on redirect” because it is not a redirect. Only raise error-loss concerns when `response.sendRedirect(...)` (or equivalent redirect behavior) is used and the code does not persist errors via a session/flash mechanism before returning `NONE`.
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
📚 Learning: 2026-04-05T04:39:00.147Z
Learnt from: yingbull
Repo: carlos-emr/carlos PR: 1000
File: src/main/java/io/github/carlos_emr/carlos/dxresearch/pageUtil/dxResearchUpdateQuickList2Action.java:0-0
Timestamp: 2026-04-05T04:39:00.147Z
Learning: In this Struts2 codebase, when a Struts2 action returns a named result (e.g., "failure") that is mapped in struts XML to another action/JSP, the default result type is `type="dispatcher"` (server-side forward). In this case, original request parameters are preserved and should be available to the forwarded-to target via `StrutsParameter` binding. During code review, do NOT treat missing query/request parameters in the forwarded-to action as data-loss just because they aren’t explicitly listed; only flag missing parameters as a data-loss issue when the code performs a genuine client-side redirect using `response.sendRedirect(...)` and the action returns `NONE` after calling `sendRedirect` (where request parameters are not preserved across the redirect).
Applied to files:
src/main/java/io/github/carlos_emr/carlos/report/pageUtil/RptByExamplesFavorite2Action.java
🪛 ast-grep (0.45.0)
src/main/java/io/github/carlos_emr/carlos/report/data/QueryByExampleSqlValidator.java
[warning] 101-101: MD2, MD4, and MD5 are weak hash functions
Context: "md5"
Note: [CWE-328] Use of Weak Hash.
(weak-message-digest-md5)
Contributor
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Collaborator
Author
|
Addressed the latest static-review findings in @cubic-dev-ai review |
Contributor
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 30 files
Architecture diagram
sequenceDiagram
participant Client as Browser
participant QBEJSP as QBE JSP (RptByExample.jsp)
participant FavoritesJSP as Favorites JSP (RptByExamplesFavorite.jsp)
participant Action as RptByExample2Action
participant FavoritesAction as RptByExamplesFavorite2Action
participant Security as SecurityInfoManager
participant Validator as QueryByExampleSqlValidator
participant DataService as RptByExampleData
participant ResultStruct as RptResultStruct
participant JDBC as CARLOS Datasource (Read-Only)
participant HistoryDB as ReportByExamples Table
participant FavoritesDB as ReportByExamplesFavorite Table
participant AuditLog as Application Log (Hashed SQL)
Note over Client,AuditLog: Query-by-Example Execution Flow (Authorized User)
Client->>QBEJSP: GET /oscarReport/RptByExample.jsp
QBEJSP->>Security: check _admin or _report read
Security-->>QBEJSP: authorized
QBEJSP-->>Client: render form + favorites + history
Client->>Action: POST /oscarReport/RptByExample (sql, providerNo)
Action->>Action: CHANGED: check QUERY_BY_EXAMPLE_ENABLED property
alt disabled
Action-->>Client: SUCCESS + queryDisabled=true (localized message, no execution)
else enabled
Action->>Action: validate sql is not blank
alt blank sql
Action-->>Client: SUCCESS + queryValidationError=true
else valid sql
Action->>DataService: execute(sql, properties, providerNo)
DataService->>Validator: validate(sql, properties)
Validator->>Validator: parseSingleSelect (CCJSqlParserUtil)
Validator->>Validator: rejectUnsafeTextOperations (locking/output)
Validator->>Validator: rejectUnsafeExpressionsAndOtherSchemas (tables, functions, schema)
alt validation fails
Validator-->>DataService: throw QueryByExampleValidationException
DataService->>AuditLog: audit(providerNo, queryHash, outcome=rejected)
DataService-->>Action: rethrow
Action-->>Client: SUCCESS + queryValidationError=true
else validation passes
Validator-->>DataService: TrustedSql wrapper
DataService->>JDBC: getConnection()
DataService->>JDBC: setReadOnly(true)
DataService->>JDBC: prepareStatement(trustedSql, FORWARD_ONLY, CONCUR_READ_ONLY)
DataService->>JDBC: setMaxRows(1001)
DataService->>JDBC: setQueryTimeout(15)
DataService->>JDBC: executeQuery()
JDBC-->>DataService: ResultSet (read-only, max 1001 rows)
DataService->>ResultStruct: getStructureWithCount(rs, MAX_OUTPUT_CHARACTERS, MAX_ROWS)
ResultStruct-->>DataService: StructuredResult(html, rowCount, truncated, rowLimitReached)
DataService->>JDBC: setReadOnly(original state)
JDBC-->>DataService: OK
DataService->>AuditLog: audit(providerNo, queryHash, outcome=success)
DataService-->>Action: QueryResult(html, rowCount, duration)
alt timeout
DataService->>AuditLog: audit(providerNo, queryHash, outcome=timeout)
Action-->>Client: SUCCESS + queryTimeout=15s
else SQLException
DataService->>AuditLog: audit(providerNo, queryHash, outcome=failed)
Action-->>Client: SUCCESS + queryExecutionError=true
else success
Action->>Action: write2Database(sql, providerNo)
Action->>HistoryDB: insert ReportByExamples record
alt history save fails
Action->>AuditLog: audit(providerNo, queryHash, outcome=history_failed)
Action-->>Client: SUCCESS + queryHistoryWarning
else history saved
Action-->>Client: SUCCESS + results html + row count + limit/truncation flags
end
end
end
end
end
Note over Client,FavoritesDB: Favorites Mutation Flow (Authorized User, POST only)
Client->>FavoritesJSP: POST /oscarReport/RptByExamplesFavorite (id, favoriteName, query, toDelete)
FavoritesJSP->>Security: check _admin or _admin read
Security-->>FavoritesJSP: authorized
FavoritesJSP->>FavoritesAction: execute()
FavoritesAction->>FavoritesAction: verify POST method (reject GET/HEAD with 405)
alt delete case
FavoritesAction->>FavoritesDB: requireOwnedFavorite(providerNo, id)
FavoritesDB-->>FavoritesAction: favorite (or SecurityException)
FavoritesAction->>FavoritesDB: remove(favorite)
else edit case (hasFavoriteId)
FavoritesAction->>FavoritesDB: requireOwnedFavorite(providerNo, id)
FavoritesDB-->>FavoritesAction: favorite
FavoritesAction->>FavoritesDB: merge(updated favorite)
else add case
FavoritesAction->>FavoritesDB: findByEverything(providerNo, favoriteName, query)
alt not found
FavoritesAction->>FavoritesDB: persist new ReportByExamplesFavorite
else exists
FavoritesAction->>FavoritesDB: merge existing
end
end
FavoritesDB-->>FavoritesAction: success
FavoritesAction-->>Client: redirect to favorites list
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
Collaborator
Author
|
@coderabbitai review |
Contributor
@Ben-Heerema I have started the AI code review. It will take a few minutes to complete. |
Contributor
|
|
Contributor
There was a problem hiding this comment.
No issues found across 30 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant UI as JSP (RptByExample.jsp)
participant Action as RptByExample2Action
participant Validator as QueryByExampleSqlValidator
participant Legacy as LegacyJdbcQuery (trustedSelectSql)
participant Data as RptByExampleData
participant JDBC as CARLOS Datasource
participant DAO as ReportByExamplesDao
participant Audit as Logger (Audit)
Note over UI,Audit: NEW: Query-By-Example Execution Flow
UI->>Action: POST /oscarReport/RptByExample (sql, providerNo)
Action->>Action: Check QUERY_BY_EXAMPLE_ENABLED property
alt Feature disabled
Action->>UI: request.setAttribute("queryDisabled", true)
Action->>Audit: audit(providerNo, sql, 0, 0, "disabled")
UI->>UI: Show localized disabled message
else Feature enabled
Action->>Action: POST-only check
Action->>Validator: validate(sql, properties)
Validator->>Validator: Parse with JSqlParser
Validator->>Validator: Check single SELECT, no UNION, no comments
Validator->>Validator: Verify only app schema tables (exclude security-sensitive tables)
Validator->>Validator: Reject locking, output, write, resource-control functions
Validator-->>Action: TrustedSql wrapper OR QueryByExampleValidationException
alt Validation fails
Action->>UI: request.setAttribute("queryValidationError", true)
Action->>Audit: audit(providerNo, sql, 0, 0, "rejected")
UI->>UI: Show localized validation error
else Validation passes
Action->>Data: execute(trustedSql, properties, providerNo)
Data->>JDBC: getConnection()
Data->>JDBC: connection.setReadOnly(true)
Data->>JDBC: prepareStatement(trustedSql, TYPE_FORWARD_ONLY, CONCUR_READ_ONLY)
Data->>JDBC: statement.setMaxRows(1001)
Data->>JDBC: statement.setQueryTimeout(15)
Data->>JDBC: executeQuery()
JDBC-->>Data: ResultSet (read-only, forward-only)
Data->>Data: RptResultStruct.getStructureWithCount (max 1M chars, 1000 rows)
Data-->>Action: QueryResult(html, rowCount, truncated, rowLimitReached, durationMs)
alt Execution succeeds
Action->>DAO: write2Database(sql, providerNo)
DAO-->>Action: History saved
Action->>UI: Set results, rowCount, truncation flags
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "success")
UI->>UI: Show result table with limit/truncation notices
else History save fails
Action->>UI: request.setAttribute("queryHistoryError", true)
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "history_failed")
UI->>UI: Show results + warning about history
end
end
end
Note over Data,JDBC: Error/Timeout handling
alt SQLTimeoutException
Action->>UI: request.setAttribute("queryTimeout", true)
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "timeout")
else SQLException / RuntimeException
Action->>UI: request.setAttribute("queryExecutionError", true)
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "failed")
end
You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Ben-Heerema
marked this pull request as ready for review
August 7, 2026 15:30
Contributor
There was a problem hiding this comment.
Sorry @Ben-Heerema, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
There was a problem hiding this comment.
No issues found across 30 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Architecture diagram
sequenceDiagram
participant UI as JSP (RptByExample.jsp)
participant Action as RptByExample2Action
participant Validator as QueryByExampleSqlValidator
participant Legacy as LegacyJdbcQuery (trustedSelectSql)
participant Data as RptByExampleData
participant JDBC as CARLOS Datasource
participant DAO as ReportByExamplesDao
participant Audit as Logger (Audit)
Note over UI,Audit: NEW: Query-By-Example Execution Flow
UI->>Action: POST /oscarReport/RptByExample (sql, providerNo)
Action->>Action: Check QUERY_BY_EXAMPLE_ENABLED property
alt Feature disabled
Action->>UI: request.setAttribute("queryDisabled", true)
Action->>Audit: audit(providerNo, sql, 0, 0, "disabled")
UI->>UI: Show localized disabled message
else Feature enabled
Action->>Action: POST-only check
Action->>Validator: validate(sql, properties)
Validator->>Validator: Parse with JSqlParser
Validator->>Validator: Check single SELECT, no UNION, no comments
Validator->>Validator: Verify only app schema tables (exclude security-sensitive tables)
Validator->>Validator: Reject locking, output, write, resource-control functions
Validator-->>Action: TrustedSql wrapper OR QueryByExampleValidationException
alt Validation fails
Action->>UI: request.setAttribute("queryValidationError", true)
Action->>Audit: audit(providerNo, sql, 0, 0, "rejected")
UI->>UI: Show localized validation error
else Validation passes
Action->>Data: execute(trustedSql, properties, providerNo)
Data->>JDBC: getConnection()
Data->>JDBC: connection.setReadOnly(true)
Data->>JDBC: prepareStatement(trustedSql, TYPE_FORWARD_ONLY, CONCUR_READ_ONLY)
Data->>JDBC: statement.setMaxRows(1001)
Data->>JDBC: statement.setQueryTimeout(15)
Data->>JDBC: executeQuery()
JDBC-->>Data: ResultSet (read-only, forward-only)
Data->>Data: RptResultStruct.getStructureWithCount (max 1M chars, 1000 rows)
Data-->>Action: QueryResult(html, rowCount, truncated, rowLimitReached, durationMs)
alt Execution succeeds
Action->>DAO: write2Database(sql, providerNo)
DAO-->>Action: History saved
Action->>UI: Set results, rowCount, truncation flags
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "success")
UI->>UI: Show result table with limit/truncation notices
else History save fails
Action->>UI: request.setAttribute("queryHistoryError", true)
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "history_failed")
UI->>UI: Show results + warning about history
end
end
end
Note over Data,JDBC: Error/Timeout handling
alt SQLTimeoutException
Action->>UI: request.setAttribute("queryTimeout", true)
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "timeout")
else SQLException / RuntimeException
Action->>UI: request.setAttribute("queryExecutionError", true)
Action->>Audit: audit(providerNo, sqlHash, durationMs, rowCount, "failed")
end
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Signed-off-by: Ben Heerema <ben@maplecreekmedical.ca>
|
Collaborator
Author
|
@yingbull ready for review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Re-enables Query by Example for providers with
_reportread or_adminread while keeping its existing Admin page links.QUERY_BY_EXAMPLE_ENABLEDkill switch; when disabled, the existing form, textarea, favorites, and history remain available and submissions show a localized disabled message without touching JDBC or history.SELECTagainst the configured CARLOS schema only.UNION, non-SELECT statements, cross-schema reads, locking/output clauses, and resource-control/file functions.Related Issues
Fixes #2568
How Was This Tested?
mvn -q -Dtest=QueryByExampleSqlValidatorTest,RptByExampleDataTest,ReportActionSecurityMigrationUnitTest,LegacyJdbcQueryUnitTest test47 tests passed. This also ran the dependency-lock check.
Screenshots
Not included; the existing page layout and disabled-state form are intentionally preserved.
Checklist
git commit -s)Summary by Sourcery
Re-enable the Query-by-Example reporting tool with strict, read-only SQL validation, bounded execution, and configurable kill switch while preserving the existing UI and history behavior.
New Features:
Enhancements:
Build:
Tests:
Summary by cubic
Re-enables Query by Example for
_report/_adminreaders with strict single-SELECT validation viacom.github.jsqlparser:jsqlparser, read-only bounded execution, localized feedback, and hashed audit logs. Adds a default-onQUERY_BY_EXAMPLE_ENABLEDkill switch and fixes #2568.New Features
SELECT; rejects comments, set ops, non-SELECT, cross-schema, locking/output, variables, and non-whitelisted functions; ignores keywords in strings._adminor_reportread.Bug Fixes
_admin/_reportchecks; resolves static analysis findings; adds tests confirming the kill switch prevents execution when disabled and enforcing sensitive-table restrictions.Written for commit a15394e. Summary will update on new commits.