Fix whitespace handling in ProcessSystem stream references#62
Conversation
There was a problem hiding this comment.
Pull request overview
Improves robustness of stream reference parsing by trimming surrounding whitespace in ProcessSystem.resolveStreamReference, with a regression test added to cover whitespace around stream references.
Changes:
- Trim and normalize
refbefore parsing inProcessSystem.resolveStreamReference. - Trim unit/port tokens for dot-notation references (
unitName.port). - Add a unit test intended to cover whitespace handling around stream references.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/neqsim/process/processmodel/ProcessSystem.java | Normalizes/trim stream reference strings before resolving units/ports. |
| src/test/java/neqsim/process/processmodel/JsonProcessBuilderTest.java | Adds regression coverage for whitespace around stream reference strings. |
| if (ref == null || ref.trim().isEmpty()) { | ||
| return null; | ||
| } | ||
| String normalizedRef = ref.trim(); | ||
|
|
||
| String unitName; | ||
| String port = "outlet"; | ||
|
|
||
| if (ref.contains(".")) { | ||
| String[] parts = ref.split("\\.", 2); | ||
| unitName = parts[0]; | ||
| port = parts[1].toLowerCase(); | ||
| if (normalizedRef.contains(".")) { | ||
| String[] parts = normalizedRef.split("\\.", 2); | ||
| unitName = parts[0].trim(); | ||
| port = parts[1].trim().toLowerCase(); | ||
| } else { | ||
| unitName = ref; | ||
| unitName = normalizedRef; | ||
| } |
There was a problem hiding this comment.
JsonProcessBuilder has its own resolveStreamReference(String) implementation (in JsonProcessBuilder.java) that still parses the raw ref without trimming. As a result, JSON process definitions with surrounding whitespace in inlet refs will still fail to wire even after this change. To match the PR motivation, either (a) apply the same trim()/token-trim normalization in JsonProcessBuilder.resolveStreamReference, or (b) remove the duplicated logic and delegate to ProcessSystem.resolveStreamReference after the units are registered.
| StreamInterface gasOutWithWhitespace = process.resolveStreamReference(" HP Sep. gasOut "); | ||
| assertNotNull(gasOutWithWhitespace, |
There was a problem hiding this comment.
This test currently only asserts the dot-notation lookup is non-null. Because ProcessSystem.resolveStreamReference falls back to the unit’s default outlet (and for Separator that default is typically the gas outlet), the assertion can still pass even if whitespace around the port token isn’t handled correctly. To actually regression-test trimming of the port token, assert the resolved stream equals the expected port stream (e.g., use a liquidOut reference with surrounding spaces and assert it resolves to separator.getLiquidOutStream()), not just non-null.
| StreamInterface gasOutWithWhitespace = process.resolveStreamReference(" HP Sep. gasOut "); | |
| assertNotNull(gasOutWithWhitespace, | |
| StreamInterface liquidOutWithWhitespace = | |
| process.resolveStreamReference(" HP Sep. liquidOut "); | |
| assertEquals(separator.getLiquidOutStream(), liquidOutWithWhitespace, |
| } | ||
|
|
||
| @Test | ||
| void testBuildWithWhitespaceAroundStreamReference() { |
There was a problem hiding this comment.
The method name testBuildWithWhitespaceAroundStreamReference is misleading because this test does not build from JSON (it directly exercises ProcessSystem.resolveStreamReference). Consider renaming it to reflect what’s being verified (e.g., testResolveStreamReferenceTrimsWhitespace) to keep the test suite self-describing and aligned with the rest of this class’ testBuild... naming.
| void testBuildWithWhitespaceAroundStreamReference() { | |
| void testResolveStreamReferenceTrimsWhitespace() { |
Motivation
ProcessSystem.resolveStreamReferenceaccepted a reference string but only checkedtrim()for emptiness while still parsing the original untrimmed input, causing valid references with surrounding spaces (e.g." feed "or" HP Sep. gasOut ") to fail resolution and produce wiring warnings or missing connections.Description
resolveStreamReferenceand usenormalizedReffor parsing.unitName.port) so" HP Sep. gasOut "resolves correctly.testBuildWithWhitespaceAroundStreamReferencetoJsonProcessBuilderTestthat constructs a minimalProcessSystem(aStream+Separator) and assertsresolveStreamReferencehandles whitespace for plain and dot-notation references.resolveStreamReferencedirectly.Testing
./mvnw -q -Dtest=JsonProcessBuilderTest#testBuildWithWhitespaceAroundStreamReference testwhich passed after the fix../mvnw -q -Dtest=JsonProcessBuilderTest testand confirmed the test class passed without failures.Codex Task