ciimsproxy/plans/2026-07-08-ciimsproxy-refactor-plan-v2.md

CIIMS Proxy — Refactor & Refine Plan

Based on 2026-07-08-ciimsproxy-codebase-analysis.md Go 1.22 · 4 source files · 16 tests

---

Objective

Address the remaining correctness gaps, improve abstraction quality, and optimize performance identified in the codebase analysis. The proxy is already functional and stable; this plan targets the next tier of quality improvements without breaking the existing HTTP API contract. SOAP template cleanup must preserve SOAP semantics; any byte-for-byte wire-format change should be explicit and isolated.

Expected outcomes:

• receiveMessage detects and reports CIIMS SOAP faults instead of silently returning empty results.
• HTTP non-2xx responses from CIIMS are treated as errors while still allowing SOAP fault bodies to be parsed and returned.
• SOAP response parsing is hardened for expected XML formatting variations; any retained regex patterns are compiled once at startup, not per request.
• A configured HTTP transport client is injected into handlers/tests instead of relying on package-level mutable client state.
• Deprecated ioutil.ReadAll is replaced with io.ReadAll.
• Handler-level (Gin endpoint) tests are added.
• Domain types and a Config struct improve type safety and testability.
• Template indentation is normalized for readability only if the team accepts the resulting byte-for-byte SOAP request change.

---

Implementation Plan

Phase 1 — Correctness (High Priority)

1. Add SOAP Fault Detection to receiveMessage

• In cmd/main/main.go, after the outbound CIIMS call in receiveMessage, call internal.GetErrMsg(resp) and return 500 with the fault text if non-empty.
• If the outbound call returns a typed HTTP status error with a response body, inspect that body with internal.GetErrMsg before falling back to the transport/status error message.
• Log the SOAP fault at error level, consistent with sendMessage.
• Add a handler-level test in cmd/main/main_test.go: mock CIIMS returns ErrMsg for a receive call, verify /receive returns 500 {"error":"<fault text>"} instead of 200 {"msgs":[]}.

2. Check HTTP Status Codes from CIIMS

• In internal/http.go, after client.Do(req), check resp.StatusCode.
• Always read and close the response body once. If status is not 2xx, return the body string alongside a typed error such as HTTPStatusError{StatusCode, Status, Body}.
• Make HTTPStatusError.Error() include the status code and a truncated/sanitized body snippet, while still exposing the full body field for SOAP fault parsing by the handler.
• Update sendMessage and receiveMessage error handling: when err != nil, first parse resp or the typed error body for a SOAP fault; if found, return that fault, otherwise return the transport/status error.
• Add a test: mock CIIMS returns 500 Internal Server Error, verify Send() returns an error.
• Add a test: mock CIIMS returns HTTP 500 with SOAP fault XML, verify the handler returns the SOAP fault text rather than a generic HTTP status error.
• Add a test: mock CIIMS returns 200 OK, verify Send() succeeds normally.

Phase 2 — Performance (High Priority)

3. Harden Response Parsing and Pre-compile Patterns

• Prefer replacing regex-based parsing in GetMsgs and GetErrMsg with encoding/xml.Decoder token parsing that matches elements by local name (string, errorMessage) regardless of namespace prefix.
• If regex is retained as a short-term step, update patterns to handle expected formatting variations such as attributes on <prefix:string> and multiline content.
• If regex is retained, move regexp.MustCompile(msgExp) and regexp.MustCompile(errExp) from function bodies to package-level var declarations.
• Name them msgRegex and errRegex.
• Update GetMsgs and GetErrMsg to use the pre-compiled variables.
• Add tests for multiline string payloads, attributes on string elements, whitespace variations, and different namespace prefixes.
• Verify all existing tests pass without changes unless parser behavior is intentionally broadened.

4. Inject a Configured HTTP Client

• Do not introduce package-level mutable client state such as var httpClient *http.Client plus InitClient; this risks nil-client bugs and test pollution.
• In internal/http.go, introduce a transport type such as type Client struct { httpClient *http.Client }.
• Add NewClient(timeoutSec int) (*Client, error) to validate/default the timeout and construct the underlying http.Client.
• Move outbound send behavior to a method such as func (c *Client) Send(url, msg string) (string, error).
• Construct the client in main() after resolving configuration and pass it to handlers through a Config/handler struct or closure.
• Update tests to create their own internal.Client instances so timeout settings do not leak between tests.
• Add a test that verifies the injected client timeout is honored.
• Ensure the existing TestSend_NetworkTimeout test still passes.

Phase 3 — Modernization (Medium Priority)

5. Replace Deprecated ioutil.ReadAll

• In internal/http.go, replace ioutil.ReadAll(resp.Body) with io.ReadAll(resp.Body).
• Remove the "io/ioutil" import and add "io".
• Verify build and tests pass.

6. Add Handler-Level Tests

• Create cmd/main/main_test.go with Gin's httptest setup.
• Test POST /send with valid JSON → mock CIIMS returns success → assert 200 {"error":""}.
• Test POST /send with valid JSON → mock CIIMS returns SOAP fault → assert 500 with fault text.
• Test POST /send with missing required field → assert 400.
• Update handlers to detect *http.MaxBytesError from binding and return http.StatusRequestEntityTooLarge.
• Test POST /send with body exceeding 1 MB → assert 413.
• Test POST /receive with valid JSON → mock CIIMS returns messages → assert 200 {"msgs":[...]}.
• Test POST /receive with count = 0 → assert 400.
• Test POST /receive with count = 1001 → assert 400.
• Test POST /receive with valid JSON → mock CIIMS returns SOAP fault → assert 500.
• Test GET /ping → assert 200 {"message":"pong"}.

Phase 4 — Abstraction (Low Priority)

7. Introduce Domain Types

• In internal/codec.go, define a CIIMSResponse struct with a RawXML string field.
• Add methods: IsFault() bool, ErrorMessage() string, Messages() []string.
• Update the injected client's Send() method to return (*CIIMSResponse, error) instead of (string, error).
• Preserve access to any non-2xx response body through CIIMSResponse.RawXML and/or the typed status error so SOAP faults in HTTP 500 responses remain parseable.
• Update sendMessage and receiveMessage to use the new methods.
• Update all tests to use the new return type.
• Ensure no change in external behavior.

8. Introduce a Config Struct

• In cmd/main/main.go, define a Config struct with fields ServerURL, Timeout, Listen.
• Include the injected CIIMS client/transport in Config or in a handler struct.
• Parse environment variables into a Config value in main().
• Pass Config to handler functions via closure (or a handler struct).
• Remove package-level defaultURL and timeout variables.
• Update getURL to accept the server URL as a parameter.
• Verify all handler tests pass with the new structure.

9. Remove Redundant Send Wrapper

• In internal/http.go, delete postSim after moving its logic into the injected client method.
• Decide whether to keep a package-level Send(url, message, timeout) compatibility wrapper for legacy internal tests; if kept temporarily, mark it as a thin compatibility helper and keep production handlers on the injected client.
• Otherwise, update all callers (in main.go and tests) to use client.Send(...).
• Verify build and all tests pass.

Phase 5 — Cleanup (Low Priority)

10. Normalize Template Indentation

• Confirm this is acceptable as a byte-for-byte SOAP request change before implementing.
• In internal/codec.go, make sendtpl and receivetpl use consistent indentation (all spaces or all tabs).
• Update the golden-file test constants in internal/codec_test.go (SendEsp, ReceiveEsp) to match.
• Verify TestSend and TestReceive pass with the updated expected strings.

11. Remove Unused gommon/log Dependency

• In cmd/main/main.go, replace github.com/labstack/gommon/log with Go's standard log package.
• Replace log.Infof → log.Printf, log.Errorf → log.Printf("[ERROR] ...") or use log with prefixes.
• Remove github.com/labstack/gommon from go.mod and run go mod tidy.
• Verify build and all tests pass.

---

Verification Criteria

• POST /receive with a CIIMS SOAP fault returns 500 {"error":"<fault text>"}, not 200 {"msgs":[]}.
• CIIMS returning HTTP 500 produces an error from Send(), not a successful response.
• CIIMS returning HTTP 500 with a SOAP fault body returns the SOAP fault text to the caller.
• Oversized request bodies return 413, not a generic 400.
• go test ./... shows all tests passing; response parsing handles multiline/attribute/namespace-prefix variations.
• HTTP client timeout is configured through an injected client/transport and does not rely on package-level mutable initialization.
• ioutil.ReadAll no longer appears in the codebase.
• cmd/main/main_test.go exists with at least 9 handler-level tests covering all error paths.
• internal.CIIMSResponse type exists with IsFault(), ErrorMessage(), Messages() methods.
• cmd/main has no package-level defaultURL or timeout variables.
• internal/http.go has no postSim; production code uses the injected client Send method.
• sendtpl and receivetpl use consistent indentation only if the wire-format change was explicitly accepted.
• github.com/labstack/gommon is absent from go.mod.
• go vet ./... and go build ./... produce no errors.

---

Execution Order

Phase 1 (Correctness) ──► Phase 2 (Performance) ──► Phase 3 (Modernization)
                                                            │
Phase 5 (Cleanup) ◄──────── Phase 4 (Abstraction) ◄────────┘

Phases 1 and 2 should be completed first as they fix remaining bugs and transport/testability issues. Phase 4's Config/injected-client work may be pulled earlier if it simplifies Phase 2. Phases 3–5 are quality-of-life improvements that can be done in any order, but Phase 4 should precede Phase 5 since removing gommon/log may be affected by the Config struct refactor.

---

Potential Risks and Mitigations

1. HTTP status code checking breaks CIIMS compatibility

   • Some SOAP services return 500 with a SOAP fault in the body (which is valid SOAP). The current code already handles this via GetErrMsg. The fix should check for 2xx and treat everything else as a transport error, but still allow the SOAP fault body to be extracted if present.
   • Mitigation: On non-2xx, return a typed HTTPStatusError that exposes the response body and also return/preserve that body in the response value. The handler must parse SOAP faults first, then fall back to the status error.

2. Injected http.Client changes timeout behavior

   • The current code creates a new client per request with the configured timeout. An injected client has a fixed timeout set at construction.
   • Mitigation: This is the desired production behavior, and tests should construct isolated clients. If per-request timeout overrides are needed later, use context.WithTimeout on the request context.

3. CIIMSResponse abstraction changes the return type

   • All callers of Send() must be updated.
   • Mitigation: This is a mechanical refactor. The compiler will catch all call sites. Update them in one pass.

4. Removing gommon/log changes log output format

   • Standard log package has a different default format (timestamps with date/time).
   • Mitigation: This is acceptable—Gin already uses its own logger. Standardizing on log reduces dependencies and is more idiomatic.

5. Template indentation change breaks golden-file tests

   • TestSend and TestReceive do exact string comparison.
   • Mitigation: Update the expected constants (SendEsp, ReceiveEsp) in the same commit. The tests themselves verify correctness.