Skip to content

Fix whitespace handling in ProcessSystem stream references #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
EvenSol merged 1 commit into from
Apr 8, 2026
+28 −5
Merged

Fix whitespace handling in ProcessSystem stream references #62

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/main/java/neqsim/process/processmodel/ProcessSystem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3931,16 +3931,17 @@ public StreamInterface resolveStreamReference(String ref) {
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;
}
Comment on lines 3931 to 3945
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

ProcessEquipmentInterface unit = getUnit(unitName);
Expand Down
22 changes: 22 additions & 0 deletions src/test/java/neqsim/process/processmodel/JsonProcessBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,28 @@ void testBuildWithSeparatorAndCompressor() {
assertNotNull(process.getUnit("Comp"));
}

@Test
void testBuildWithWhitespaceAroundStreamReference() {
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
void testBuildWithWhitespaceAroundStreamReference() {
void testResolveStreamReferenceTrimsWhitespace() {

Copilot uses AI. Check for mistakes.
SystemSrkEos fluid = new SystemSrkEos(298.15, 50.0);
fluid.addComponent("methane", 0.85);
fluid.addComponent("ethane", 0.10);
fluid.addComponent("propane", 0.05);
fluid.setMixingRule("classic");

Stream feed = new Stream("feed", fluid);
neqsim.process.equipment.separator.Separator separator =
new neqsim.process.equipment.separator.Separator("HP Sep", feed);
ProcessSystem process = new ProcessSystem();
process.add(feed);
process.add(separator);

assertNotNull(process.resolveStreamReference(" feed "),
"resolveStreamReference should trim whitespace for plain stream names");
StreamInterface gasOutWithWhitespace = process.resolveStreamReference(" HP Sep. gasOut ");
assertNotNull(gasOutWithWhitespace,
Comment on lines +85 to +86
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
StreamInterface gasOutWithWhitespace = process.resolveStreamReference(" HP Sep. gasOut ");
assertNotNull(gasOutWithWhitespace,
StreamInterface liquidOutWithWhitespace =
process.resolveStreamReference(" HP Sep. liquidOut ");
assertEquals(separator.getLiquidOutStream(), liquidOutWithWhitespace,

Copilot uses AI. Check for mistakes.
"resolveStreamReference should trim whitespace around unit and port tokens");
}

@Test
void testBuildWithMultipleFluids() {
String json = "{" + "\"fluids\": {" + " \"gas\": {" + " \"model\": \"SRK\","
Expand Down
Loading