etzel-mes commented on PR #27908: URL: https://github.com/apache/camel/pull/27908#issuecomment-6200728303
Thanks, confirmed: a `milo-server` item starts as `new DataValue(StatusCode.BAD)`, the first notification of a monitored item carries that value, and with this PR the consumer delivers it. That is intended, since it is quality information like `Bad_WaitingForInitialData`, so the texts and the tests were wrong, not the predicate. CAMEL-18144 added `omitNullValues` to skip exactly this notification; the PR description now says so. Fixed in 23f226f00ad: 1. **Texts.** The option javadoc (and with it the generated JSON, catalog and endpoint DSL) and the `MiloClientConsumer` javadoc no longer give the startup value as an example. The 4.24 upgrade note now has its own section: a route may receive a Bad status with a null value at startup and after a reconnect, for example `Bad_WaitingForInitialData` or the status of a `milo-server` item that was not written yet; it checks `getStatusCode()` of the `DataValue` body before it uses the value, and filters on the status code to skip it. The component doc says the same and adds that the default still omits a Good null after a Bad status. 2. **Tests.** `MonitorItemLifecycleTest`, `MonitorItemTest` and `MonitorItemMultiConnectionsTest` now write the value before they start their consumers (`autoStartup(false)`, as in `MonitorNodesTest`). In `MonitorItemTest`, the client that samples once per second now always gets `Foo` as its first message; it got 6 messages, the old maximum, in every local run, so its bounds are one higher (5 to 7). 3. **New test.** `MonitorItemLifecycleTest#itemThatWasNotWrittenYetReportsABadStatusAtStartup` starts a consumer on an item that was not written yet and expects a Bad `DataValue` with a null value, then the written value. Why they passed: the trace is right, but the tests did not fail reliably. CI on 891bf76, which finished after your review, was green on JDK 17 and 25 with no recovered flaky tests. `MonitorItemLifecycleTest` and `MonitorItemMultiConnectionsTest` wrote `Foo` right after the context started and passed whenever the write landed before the server's first sample, or the count was checked before the second notification arrived. With `-Dsurefire.rerunFailingTestsCount=0`, `MonitorItemLifecycleTest#restartedConsumerReceivesEachValueOnce` failed in one run of nine on my machine (`message(0)` expected `Foo` but was `null`); the `full` profile reruns a failed test up to twice. In `MonitorItemTest` the `message(i)` checks are added after the messages arrived, so in the mock's default fail-fast mode they never run; only its counters could fail. With the changes, the four classes passed eight runs in a row with reruns disabled, and the module `-Psourcecheck verify` passed (57 tests, 1 skipped). Real servers with this branch: node-opcua 2.187.9 sends `Bad_WaitingForInitialData` (or `Uncertain_InitialValue`) with a null value as the first notification, and Microsoft OPC PLC 2.15.4 sends `Bad_NoCommunication` for a node in a bad state; the route now receives them by default. The default still drops Good notifications with a null value, which OPC PLC sends for null nodes, so the option keeps a purpose. The endpoint DSL javadoc is still edited by hand (its generator needs the full reactor), wrapped like the generator output; the regen check passed on 891bf76, and if it differs this time, I will apply its patch. _Claude Code on behalf of etzel-mes_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
