#2803 fix: remove debug log statement from RxDsMessageTo1 constructor#3073
#2803 fix: remove debug log statement from RxDsMessageTo1 constructor#3073bellaayang wants to merge 1 commit into
Conversation
Signed-off-by: bellaayang <jingjingyjh@126.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR comments out an unconditional debug/error log statement in the RxDsMessageTo1 constructor that was dumping the full drug interaction map to the logs, without changing any functional logic. Sequence diagram for RxDsMessageTo1 constructor logging changesequenceDiagram
participant Caller
participant RxDsMessageTo1
participant MiscUtils
participant Logger
Caller->>RxDsMessageTo1: new RxDsMessageTo1(ht)
activate RxDsMessageTo1
Note over RxDsMessageTo1: Previous behavior
RxDsMessageTo1->>MiscUtils: getLogger()
MiscUtils-->>RxDsMessageTo1: Logger
RxDsMessageTo1->>Logger: error("WHATS inn ht" + ht)
Note over RxDsMessageTo1: Current behavior
RxDsMessageTo1->>RxDsMessageTo1: read type, author, updated_by from ht
deactivate RxDsMessageTo1
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request comments out a debug log statement in RxDsMessageTo1.java. The reviewer suggests completely removing the unused debug log statement instead of leaving it commented out to maintain code cleanliness and readability.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } | ||
|
|
||
| MiscUtils.getLogger().error("WHATS inn ht" + ht); | ||
| // MiscUtils.getLogger().error("WHATS inn ht" + ht); |
There was a problem hiding this comment.
Instead of leaving commented-out code, the unused debug log statement should be completely removed to maintain code cleanliness and readability.
📝 WalkthroughSummary by CodeRabbit
WalkthroughA single debug ChangesDebug Log Removal
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Instead of commenting out the debug log, consider removing it entirely or replacing it with a properly leveled and structured log statement guarded by a configuration flag so the constructor doesn’t carry unused commented code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of commenting out the debug log, consider removing it entirely or replacing it with a properly leveled and structured log statement guarded by a configuration flag so the constructor doesn’t carry unused commented code.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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/webserv/rest/to/model/RxDsMessageTo1.java (1)
97-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove adjacent commented-out debug log on Line 97.
Line 97 contains another commented-out debug log (
//logger.error("---$string instance");) in the same constructor. While not part of the PR's targeted line, this is similarly a leftover debug artifact. Consider removing it as part of this cleanup, or it can be addressed separately.🤖 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/webserv/rest/to/model/RxDsMessageTo1.java` at line 97, Remove the leftover commented-out debug log in RxDsMessageTo1’s constructor. Clean up the adjacent commented logger statement (`logger.error` debug artifact) so the constructor contains only active code, matching the rest of the comment cleanup in this class.
🤖 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/webserv/rest/to/model/RxDsMessageTo1.java`:
- Line 134: Remove the constructor debug logging from RxDsMessageTo1 entirely
instead of leaving it commented out. Delete the MiscUtils.getLogger().error(...)
line from the constructor so no dead debug artifact remains, and keep any
handling of ht free of logging to avoid exposing sensitive clinical data.
---
Outside diff comments:
In
`@src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.java`:
- Line 97: Remove the leftover commented-out debug log in RxDsMessageTo1’s
constructor. Clean up the adjacent commented logger statement (`logger.error`
debug artifact) so the constructor contains only active code, matching the rest
of the comment cleanup in this class.
🪄 Autofix (Beta)
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
Run ID: a3de475a-4284-4597-ae4e-94b0772f6c91
📒 Files selected for processing (1)
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.java
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Sourcery review
⚠️ CI failures not shown inline (2)
GitHub Actions: DCO / DCO Sign-Off: #2803 fix: remove debug log statement from RxDsMessageTo1 constructor
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mpr_json=$(curl -fsSL \�[0m
�[36;1m -H "Authorization: ***" \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m "https://api.github.com/repos/$REPO/pulls/$PR_NUMBER")�[0m
�[36;1mpr_commit_count=$(jq -r '.commits' <<< "$pr_json")�[0m
�[36;1mbase_ref=$(jq -r '.base.ref' <<< "$pr_json")�[0m
�[36;1m�[0m
�[36;1mif [ "$pr_commit_count" -gt 250 ]; then�[0m
�[36;1m echo "::error::CARLOS EMR DCO check cannot automatically verify $pr_commit_count commits because GitHub's pull request commits API is capped at 250 commits."�[0m
GitHub Actions: DCO / 0_DCO Sign-Off.txt: #2803 fix: remove debug log statement from RxDsMessageTo1 constructor
Conclusion: failure
##[group]Run set -euo pipefail
�[36;1mset -euo pipefail�[0m
�[36;1m�[0m
�[36;1mpr_json=$(curl -fsSL \�[0m
�[36;1m -H "Authorization: ***" \�[0m
�[36;1m -H "Accept: application/vnd.github+json" \�[0m
�[36;1m "https://api.github.com/repos/$REPO/pulls/$PR_NUMBER")�[0m
�[36;1mpr_commit_count=$(jq -r '.commits' <<< "$pr_json")�[0m
�[36;1mbase_ref=$(jq -r '.base.ref' <<< "$pr_json")�[0m
�[36;1m�[0m
�[36;1mif [ "$pr_commit_count" -gt 250 ]; then�[0m
�[36;1m echo "::error::CARLOS EMR DCO check cannot automatically verify $pr_commit_count commits because GitHub's pull request commits API is capped at 250 commits."�[0m
🧰 Additional context used
📓 Path-based instructions (8)
**/*.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: Keep Java package names asio.github.carlos_emr.carlos.*for backward compatibility - DO NOT change internal package names
UsePathValidationUtilsfor ALL file path operations to prevent path traversal attacks
PHI (Patient Health Information) must NEVER be logged or exposed in error messages
Use NEW namespaceio.github.carlos_emr.carlos.*for ALL new code - avoid oldorg.oscarehr.*andoscar.*packages
Use sanctioned suffixes for layer naming:*Action,*ViewModelAssembler,*ViewModel,*Loader,*Resolver,*Composer,*Validator,*Persister,*Calculator,*Parser,*ImportService,*Service,*Dao,*Dto,*Command. Never combine two role-suffixes or use forbidden suffixes like*Manager,*Helper,*Utils,*Prep
Public JavaDoc is required for public entrypoint classes and non-obvious public/protected methods; carrier DTOs, getters/setters, and private helpers do not need method-by-method JavaDoc
DO NOT add@authortags in JavaDoc (misleading after Bitbucket→GitHub migration); use git history for accurate dates with@sincetags
Inline comments should explain the why: legacy compatibility, security/compliance constraints, file-format offsets, transaction boundaries, and non-obvious control flow. Do not narrate o...
Files:
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.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 null-safe CARLOS wrappers for all user input encoding:
<carlos:encode>tag,${carlos:forXxx()}EL functions, orSafeEncode.forXxx()Java methods. DO NOT use<e:forXxx>,${e:forXxx()}, orEncode.forXxx()directly in new code.
Files:
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.java
src/**/*.java
📄 CodeRabbit inference engine (GEMINI.md)
src/**/*.java: Keep Java packages asio.github.carlos_emr.carlos.*for backward compatibility - do NOT change these internal namespace paths
Preserve existing copyright headers in modified files and preserve GPL version exactly as-is; never remove existing copyright notices
Files:
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.java
{src/main/java/io/github/carlos_emr/carlos/**/*.java,src/main/webapp/**/*.jsp}
📄 CodeRabbit inference engine (GEMINI.md)
PHI (Patient Health Information) must NEVER be logged or exposed in error messages
Files:
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.java
src/main/java/io/github/carlos_emr/carlos/**/*.java
📄 CodeRabbit inference engine (GEMINI.md)
src/main/java/io/github/carlos_emr/carlos/**/*.java: All public classes and methods MUST have comprehensive JavaDoc; do NOT add@authortags; use@sincewith accurate git history dates; document@param,@return, and@throwstags with specific types
Add inline comments on separate lines (not same-line) for complex logic; include comprehensive comments for non-obvious algorithms or healthcare-specific calculations
Files:
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.java
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve all existing copyright headers in modified files; do not remove or change GPL version statements
Files:
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.java
**/*.{java,jsp,xml,js,css}
📄 CodeRabbit inference engine (CLAUDE.md)
New files must use CARLOS project header as defined in
docs/copyright-header-carlos.md
Files:
src/main/java/io/github/carlos_emr/carlos/webserv/rest/to/model/RxDsMessageTo1.java
🧠 Learnings (8)
📚 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/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.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/webserv/rest/to/model/RxDsMessageTo1.java
| } | ||
|
|
||
| MiscUtils.getLogger().error("WHATS inn ht" + ht); | ||
| // MiscUtils.getLogger().error("WHATS inn ht" + ht); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove the debug log line entirely rather than commenting it out.
The linked issue explicitly requests deleting this constructor log line entirely (MiscUtils.getLogger().error("WHATS inn ht" + ht);). The current change only comments it out, leaving dead code in the source. Please fully delete the line to satisfy the acceptance criterion and eliminate the debug artifact.
Additionally, the ht variable contains drug interaction data (medication identifiers, interaction context). Based on coding guidelines, PHI and sensitive clinical data must never be logged or exposed in error messages. While commented out, the code still represents a pattern that should not remain in source.
🤖 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/webserv/rest/to/model/RxDsMessageTo1.java`
at line 134, Remove the constructor debug logging from RxDsMessageTo1 entirely
instead of leaving it commented out. Delete the MiscUtils.getLogger().error(...)
line from the constructor so no dead debug artifact remains, and keep any
handling of ht free of logging to avoid exposing sensitive clinical data.
Source: Coding guidelines
1 participant
Description
Removes a leftover debug log statement from the
RxDsMessageTo1constructor that fired unconditionally on every object instantiation, dumping the full drug interaction map at ERROR level. This saturates error monitoring and leaks medication identifiers to the application log.Related Issues
Fixes #2803
How Was This Tested?
Verified the log statement is removed. No functional logic was changed.
Checklist
git commit -s)Summary by Sourcery
Bug Fixes:
Summary by cubic
Removed an unconditional ERROR-level log from the
RxDsMessageTo1constructor to stop noisy error reporting and prevent medication identifiers from being logged. Fixes #2803.Written for commit c1e0e95. Summary will update on new commits.