Disallow null for StatusCode field in DataValue (#1394)

A `null` value is semantically the same, and encodes the same, as `StatusCodes.GOOD`. Any distinction is lost during serialization.

Author: kevinherron

milo-examples/client-examples/src/main/java/org/eclipse/milo/examples/client/WriteExample.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2024 the Eclipse Milo Authors
+ * Copyright (c) 2025 the Eclipse Milo Authors
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -39,8 +39,8 @@ public void run(OpcUaClient client, CompletableFuture<OpcUaClient> future) throw
     for (int i = 0; i < 10; i++) {
       Variant v = Variant.ofInt32(i);
 
-      // don't write status or timestamps
-      DataValue dv = new DataValue(v, null, null);
+      // don't write timestamps
+      DataValue dv = DataValue.valueOnly(v);
 
       List<StatusCode> statusCodes = client.writeValues(nodeIds, List.of(dv));
 

opc-ua-stack/encoding-json/src/test/java/org/eclipse/milo/opcua/stack/core/encoding/json/OpcUaJsonEncoderTest.java

@@ -890,13 +890,13 @@ public void writeDataValue() throws IOException {
 
     // omit all fields
     encoder.reset(writer = new StringWriter());
-    encoder.encodeDataValue(null, new DataValue(Variant.NULL_VALUE, null, null));
+    encoder.encodeDataValue(null, new DataValue(Variant.NULL_VALUE, StatusCode.GOOD, null));
     assertEquals("", writer.toString());
 
     // omit all fields while embedded in object
     encoder.reset(writer = new StringWriter());
     encoder.jsonWriter.beginObject();
-    encoder.encodeDataValue("foo", new DataValue(Variant.NULL_VALUE, null, null));
+    encoder.encodeDataValue("foo", new DataValue(Variant.NULL_VALUE, StatusCode.GOOD, null));
     encoder.jsonWriter.endObject();
     assertEquals("{}", writer.toString());
   }

opc-ua-stack/stack-core/src/main/java/org/eclipse/milo/opcua/stack/core/encoding/binary/OpcUaBinaryEncoder.java

@@ -292,11 +292,11 @@ public void encodeDataValue(DataValue value) throws UaSerializationException {
     } else {
       int mask = 0x00;
 
-      if (value.value() != null && value.value().isNotNull()) {
+      if (value.value().isNotNull()) {
         mask |= 0x01;
       }
 
-      if (!StatusCode.GOOD.equals(value.statusCode())) {
+      if (value.getStatusCode().getValue() != 0L) {
         mask |= 0x02;
       }
 

opc-ua-stack/stack-core/src/main/java/org/eclipse/milo/opcua/stack/core/types/builtin/DataValue.java

@@ -20,7 +20,7 @@
 @NullMarked
 public record DataValue(
     Variant value,
-    @Nullable StatusCode statusCode,
+    StatusCode statusCode,
     @Nullable DateTime sourceTime,
     @Nullable UShort sourcePicoseconds,
     @Nullable DateTime serverTime,
@@ -42,13 +42,13 @@ public DataValue(Variant value, StatusCode status) {
     this(value, status, DateTime.now());
   }
 
-  public DataValue(Variant value, @Nullable StatusCode status, @Nullable DateTime time) {
+  public DataValue(Variant value, StatusCode status, @Nullable DateTime time) {
     this(value, status, time, time);
   }
 
   public DataValue(
       Variant value,
-      @Nullable StatusCode status,
+      StatusCode status,
       @Nullable DateTime sourceTime,
       @Nullable DateTime serverTime) {
 
@@ -59,7 +59,7 @@ public Variant getValue() {
     return value;
   }
 
-  public @Nullable StatusCode getStatusCode() {
+  public StatusCode getStatusCode() {
     return statusCode;
   }
 
@@ -149,19 +149,21 @@ public static DataValue derivedNonValue(DataValue from, TimestampsToReturn times
   }
 
   /**
-   * Create a {@link DataValue} containing *only* the Variant. All other fields will be null.
+   * Create a {@link DataValue} containing only a value.
+   *
+   * <p>{@link StatusCode#GOOD} is implied, and other fields will be null.
    *
    * @param v the value {@link Variant}.
    * @return a {@link DataValue} containing only the value.
    */
   public static DataValue valueOnly(Variant v) {
-    return new DataValue(v, null, null, null, null, null);
+    return new DataValue(v, StatusCode.GOOD, null, null, null, null);
   }
 
   public static class Builder {
 
     public Variant value = Variant.NULL_VALUE;
-    public @Nullable StatusCode status;
+    public StatusCode status = StatusCode.GOOD;
     public @Nullable DateTime sourceTime;
     public @Nullable UShort sourcePicoseconds;
     public @Nullable DateTime serverTime;
@@ -188,7 +190,7 @@ public Builder setStatus(long statusCode) {
       return this;
     }
 
-    public Builder setStatus(@Nullable StatusCode status) {
+    public Builder setStatus(StatusCode status) {
       this.status = status;
       return this;
     }

opc-ua-stack/stack-core/src/test/java/org/eclipse/milo/opcua/stack/core/encoding/binary/OpcUaBinaryEncoderTest.java

@@ -12,14 +12,14 @@
 
 import static org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.Unsigned.ubyte;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import io.netty.buffer.ByteBuf;
 import io.netty.buffer.Unpooled;
 import org.eclipse.milo.opcua.stack.core.OpcUaDataType;
+import org.eclipse.milo.opcua.stack.core.StatusCodes;
 import org.eclipse.milo.opcua.stack.core.encoding.DefaultEncodingContext;
-import org.eclipse.milo.opcua.stack.core.types.builtin.ExtensionObject;
-import org.eclipse.milo.opcua.stack.core.types.builtin.Matrix;
-import org.eclipse.milo.opcua.stack.core.types.builtin.Variant;
+import org.eclipse.milo.opcua.stack.core.types.builtin.*;
 import org.eclipse.milo.opcua.stack.core.types.builtin.unsigned.UByte;
 import org.eclipse.milo.opcua.stack.core.types.enumerated.ApplicationType;
 import org.eclipse.milo.opcua.stack.core.types.structured.AccessLevelType;
@@ -205,4 +205,70 @@ void encodeVariantOfStructMatrix() {
         decodedMatrix.transform(
             v -> ((ExtensionObject) v).decode(DefaultEncodingContext.INSTANCE)));
   }
+
+  @Test
+  void dataValueEncodingMaskValueBit() {
+    DataValue dataValue = new DataValue(Variant.ofBoolean(true), StatusCode.GOOD, null);
+    encoder.encodeDataValue(dataValue);
+
+    // get the EncodingMask byte out of the buffer and ensure only the Value bit is set
+    byte encodingMask = buffer.getByte(buffer.readerIndex());
+    assertEquals(0b00000001, encodingMask);
+
+    // decode the DataValue and ensure we decoded the value as true
+    var decoder = new OpcUaBinaryDecoder(DefaultEncodingContext.INSTANCE).setBuffer(buffer);
+    DataValue decodedDataValue = decoder.decodeDataValue();
+
+    assertEquals(Variant.ofBoolean(true), decodedDataValue.value());
+  }
+
+  @Test
+  void dataValueEncodingMaskNullValueBit() {
+    DataValue dataValue = new DataValue(Variant.ofNull(), StatusCode.GOOD, null);
+    encoder.encodeDataValue(dataValue);
+
+    // get the EncodingMask byte out of the buffer and ensure no bits are set
+    byte encodingMask = buffer.getByte(buffer.readerIndex());
+    assertEquals(0b00000000, encodingMask);
+
+    // decode the DataValue and ensure we decoded the value as null
+    var decoder = new OpcUaBinaryDecoder(DefaultEncodingContext.INSTANCE).setBuffer(buffer);
+    DataValue decodedDataValue = decoder.decodeDataValue();
+
+    assertTrue(decodedDataValue.value().isNull());
+  }
+
+  @Test
+  void dataValueEncodingMaskStatusCodeBit() {
+    DataValue dataValue = new DataValue(Variant.ofBoolean(true), StatusCode.GOOD, null);
+    encoder.encodeDataValue(dataValue);
+
+    // get the EncodingMask byte out of the buffer and ensure only the Value bit is set
+    byte encodingMask = buffer.getByte(buffer.readerIndex());
+    assertEquals(0b00000001, encodingMask);
+
+    // decode the DataValue and ensure we decoded the StatusCode as GOOD
+    var decoder = new OpcUaBinaryDecoder(DefaultEncodingContext.INSTANCE).setBuffer(buffer);
+    DataValue decodedDataValue = decoder.decodeDataValue();
+
+    assertEquals(StatusCode.GOOD, decodedDataValue.statusCode());
+  }
+
+  @Test
+  void dataValueEncodingMaskStatusCodeGoodOverloadBit() {
+    DataValue dataValue =
+        new DataValue(Variant.ofBoolean(true), new StatusCode(StatusCodes.Good_Overload), null);
+    encoder.encodeDataValue(dataValue);
+
+    // get the EncodingMask byte out of the buffer and ensure both the Value and StatusCode bits are
+    // set
+    byte encodingMask = buffer.getByte(buffer.readerIndex());
+    assertEquals(0b00000011, encodingMask);
+
+    // decode the DataValue and ensure we decoded the same StatusCode
+    var decoder = new OpcUaBinaryDecoder(DefaultEncodingContext.INSTANCE).setBuffer(buffer);
+    DataValue decodedDataValue = decoder.decodeDataValue();
+
+    assertEquals(new StatusCode(StatusCodes.Good_Overload), decodedDataValue.statusCode());
+  }
 }