Adding tests for bad gateway target and point ref (faucetsdn#948)

.github/workflows/integration.yaml

@@ -6,10 +6,6 @@ on:
       - '**'
   workflow_dispatch:
 
-concurrency:
-  group: it-${{ github.repository }}
-  cancel-in-progress: true
-
 jobs:
   images:
     name: Build Docker Images
@@ -72,14 +68,23 @@ jobs:
     runs-on: ubuntu-latest
     timeout-minutes: 5
     needs: images
+    strategy:
+      fail-fast: false
+      matrix:
+        device_id: [ "AHU-1", "AHU-22", "GAT-123" ]
     env:
       IMAGE_NAME: ${{ github.repository }}
       REF_NAME: ${{ github.ref_name }}
+      DEVICE_ID: ${{ matrix.device_id }}
     steps:
       - name: Setup Environment
         run: |
+          sudo apt-get install moreutils
           git clone https://github.com/faucetsdn/udmi_site_model.git
           ln -s udmi_site_model/ site_model
+          (cd site_model; git log -n 1)
+          jq ".device_id = \"$DEVICE_ID\"" site_model/cloud_iot_config.json | sponge site_model/cloud_iot_config.json
+          jq . site_model/cloud_iot_config.json
           docker network create udminet --subnet 192.168.99.0/24
       - name: Start UDMIS container
         run: |
@@ -116,13 +121,20 @@ jobs:
           docker logs pubber 2>&1 | fgrep "Connection complete"
       - name: Sequencer run
         run: |
+          SEQUENCER_TESTS="broken_config extra_config device_config_acked"
+          [[ $REF_NAME =~ test- ]] && SEQUENCER_TESTS=${REF_NAME#test-}
           docker run --net udminet --name sequencer -v $(realpath site_model):/root/site_model \
             ghcr.io/$IMAGE_NAME:validator-$REF_NAME bin/sequencer site_model/cloud_iot_config.json \
-            broken_config extra_config device_config_acked
+            $SEQUENCER_TESTS
       - name: Sequencer results
         run: |
-          cat site_model/out/devices/AHU-1/results.md
-          [[ $(cat site_model/out/devices/AHU-1/results.md | fgrep 'stable | pass' | wc -l) == 3 ]]
+          cat site_model/out/devices/$DEVICE_ID/results.md
+          if [[ $REF_NAME =~ test- ]]; then
+            SPECIFIC_TEST=${REF_NAME#test-}
+            more site_model/out/devices/$DEVICE_ID/tests/$SPECIFIC_TEST/*
+          else
+            [[ $(cat site_model/out/devices/$DEVICE_ID/results.md | fgrep 'stable | pass' | wc -l) == 3 ]]
+          fi
       - name: UDMIS logs
         if: ${{ !cancelled() }}
         run: docker logs udmis

bin/clone_model

@@ -5,7 +5,7 @@ mkdir -p $ROOT_DIR/sites
 cd $ROOT_DIR/sites 
 
 MODEL_DIR=udmi_site_model
-MODEL_VER=1.18
+MODEL_VER=1.19
 TEST_SITE_GIT=https://github.com/faucetsdn/$MODEL_DIR.git
 MODEL_REPO=origin
 

common/src/main/java/daq/pubber/ProtocolFamily.java

@@ -1,5 +1,8 @@
 package daq.pubber;
 
+import com.google.common.collect.ImmutableSet;
+import java.util.Set;
+
 public interface ProtocolFamily {
 
   String VENDOR = "vendor";
@@ -10,4 +13,6 @@ public interface ProtocolFamily {
   String BACNET = "bacnet";
   String MODBUS = "modbus";
   String INVALID = "invalid";
+
+  Set<String> FAMILIES = ImmutableSet.of(VENDOR, IPV_4, IPV_6, ETHER, IOT, BACNET, MODBUS);
 }

docs/specs/sequences/generated.md

@@ -31,7 +31,9 @@ Some caveats:
 -->
 
 <!-- START GENERATED, do not edit anything after this line! -->
-* [bad_target_family](#bad_target_family-preview): Error handling for badly formed target address family
+* [bad_point_ref](#bad_point_ref-preview): Error handling for badly formed gateway point ref
+* [bad_target_address](#bad_target_address-preview): Error handling for badly formed gateway target address
+* [bad_target_family](#bad_target_family-preview): Error handling for badly formed gateway target family
 * [broken_config](#broken_config-stable): Check that the device correctly handles a broken (non-json) config message.
 * [config_logging](#config_logging-stable): Check that the device publishes minimum required log entries when receiving config
 * [device_config_acked](#device_config_acked-stable): Check that the device MQTT-acknowledges a sent config.
@@ -58,9 +60,21 @@ Some caveats:
 * [system_last_update](#system_last_update-stable): Check that last_update state is correctly set in response to a config update.
 * [valid_serial_no](#valid_serial_no-stable)
 
+## bad_point_ref (PREVIEW)
+
+Error handling for badly formed gateway point ref
+
+1. Test skipped: Not a proxied device
+
+## bad_target_address (PREVIEW)
+
+Error handling for badly formed gateway target address
+
+1. Test skipped: Not a proxied device
+
 ## bad_target_family (PREVIEW)
 
-Error handling for badly formed target address family
+Error handling for badly formed gateway target family
 
 1. Test skipped: Not a proxied device
 

etc/schema_itemized.out

@@ -47,3 +47,6 @@
 43 gateway_proxy_state
 47 device_config_acked
 48 bad_target_family
+49 bad_target_address
+50 bad_point_ref
+54 bad_point_ref

etc/schema_nostate.out

@@ -1,2 +1,2 @@
-RESULT pass schemas device_state_stable BETA 5/5 Schema validation passed
-RESULT pass schemas events_pointset_stable BETA 5/5 Schema validation passed
+RESULT pass schemas device_state_stable STABLE 5/5 Schema validation passed
+RESULT pass schemas events_pointset_stable STABLE 5/5 Schema validation passed

etc/sequencer.out

@@ -16,6 +16,8 @@ RESULT pass enumeration multi_enumeration ALPHA 5/5 Sequence complete
 RESULT pass enumeration.families family_enumeration ALPHA 5/5 Sequence complete
 RESULT pass enumeration.features feature_enumeration PREVIEW 5/5 Sequence complete
 RESULT pass enumeration.pointset pointset_enumeration ALPHA 5/5 Sequence complete
+RESULT skip gateway bad_point_ref PREVIEW 0/0 Not a proxied device
+RESULT skip gateway bad_target_address PREVIEW 0/0 Not a proxied device
 RESULT skip gateway bad_target_family PREVIEW 0/0 Not a proxied device
 RESULT skip gateway gateway_attach_handling ALPHA 0/0 Not a gateway
 RESULT skip gateway gateway_proxy_events BETA 0/0 Not a gateway

etc/sequencer_nostate.out

@@ -6,6 +6,8 @@ RESULT skip endpoint.config endpoint_failure_and_restart PREVIEW 0/0 State testi
 RESULT skip endpoint.config endpoint_redirect_and_restart PREVIEW 0/0 State testing disabled
 RESULT skip enumeration empty_enumeration PREVIEW 0/0 State testing disabled
 RESULT skip enumeration.features feature_enumeration PREVIEW 0/0 State testing disabled
+RESULT skip gateway bad_point_ref PREVIEW 0/0 Not a proxied device
+RESULT skip gateway bad_target_address PREVIEW 0/0 Not a proxied device
 RESULT skip gateway bad_target_family PREVIEW 0/0 Not a proxied device
 RESULT skip gateway gateway_proxy_events BETA 0/0 Not a gateway
 RESULT skip gateway gateway_proxy_state PREVIEW 0/0 Not a gateway

etc/test_itemized.in

@@ -46,3 +46,9 @@ TEST gateway_proxy_state noState
 WITH AHU-22
 TEST device_config_acked
 TEST bad_target_family
+TEST bad_target_address
+TEST bad_point_ref
+
+# Test a proxy device with a different configuration
+WITH SNS-4
+TEST bad_point_ref

etc/test_itemized.out

@@ -44,3 +44,6 @@
 43 RESULT pass gateway gateway_proxy_state PREVIEW 5/5 Sequence complete
 47 RESULT skip system device_config_acked STABLE 0/0 No config check for proxy device
 48 RESULT pass gateway bad_target_family PREVIEW 5/5 Sequence complete
+49 RESULT pass gateway bad_target_address PREVIEW 5/5 Sequence complete
+50 RESULT pass gateway bad_point_ref PREVIEW 5/5 Sequence complete
+54 RESULT skip gateway bad_point_ref PREVIEW 0/0 No testing target defined for 'tweaked_ref'

pubber/src/main/java/daq/pubber/BasicPoint.java

@@ -1,12 +1,16 @@
 package daq.pubber;
 
+import static com.google.udmi.util.GeneralUtils.deepCopy;
 import static com.google.udmi.util.GeneralUtils.getNow;
+import static com.google.udmi.util.GeneralUtils.isTrue;
 
+import java.util.Objects;
 import udmi.schema.Category;
 import udmi.schema.Entry;
 import udmi.schema.PointDiscovery;
 import udmi.schema.PointPointsetConfig;
 import udmi.schema.PointPointsetEvents;
+import udmi.schema.PointPointsetModel;
 import udmi.schema.PointPointsetState;
 import udmi.schema.PointPointsetState.Value_state;
 
@@ -19,21 +23,19 @@ public abstract class BasicPoint implements AbstractPoint {
   protected final PointPointsetEvents data = new PointPointsetEvents();
   private final PointPointsetState state = new PointPointsetState();
   private final boolean writable;
+  private final String pointRef;
   protected boolean written;
   private boolean dirty;
 
   /**
    * Construct a maybe writable point.
-   *
-   * @param name     Point name
-   * @param writable True if writable
-   * @param units    Units for the point
    */
-  public BasicPoint(String name, boolean writable, String units) {
+  public BasicPoint(String name, PointPointsetModel pointModel) {
     this.name = name;
-    this.writable = writable;
-    state.units = units;
+    writable = isTrue(pointModel.writable);
+    state.units = pointModel.units;
     dirty = true;
+    pointRef = pointModel.ref;
   }
 
   protected abstract Object getValue();
@@ -72,14 +74,29 @@ public PointPointsetEvents getData() {
    * @param config Configuration to set
    */
   public void setConfig(PointPointsetConfig config) {
-    Value_state previous = state.value_state;
+    Value_state previousValueState = state.value_state;
+    Entry previousStatus = deepCopy(state.status);
+    updateStateConfig(config);
+    dirty = dirty
+        || state.value_state != previousValueState
+        || !Objects.equals(state.status, previousStatus);
+  }
+
+  /**
+   * Update the state of this point based off of a new config.
+   */
+  public void updateStateConfig(PointPointsetConfig config) {
     state.status = null;
 
+    if (config != null && !Objects.equals(pointRef, config.ref)) {
+      state.status = createEntryFrom(Category.POINTSET_POINT_FAILURE, "Invalid point ref");
+      return;
+    }
+
     if (config == null || config.set_value == null) {
       written = false;
       state.value_state = null;
       updateData();
-      dirty = state.value_state != previous;
       return;
     }
 
@@ -88,20 +105,17 @@ public void setConfig(PointPointsetConfig config) {
         state.status = createEntryFrom(Category.POINTSET_POINT_INVALID,
             "Written value is not valid");
         state.value_state = Value_state.INVALID;
-        dirty = state.value_state != previous;
         return;
       }
     } catch (Exception ex) {
       state.status = createEntryFrom(Category.POINTSET_POINT_FAILURE, ex.getMessage());
       state.value_state = Value_state.FAILURE;
-      dirty = state.value_state != previous;
       return;
     }
 
     if (!writable) {
       state.status = createEntryFrom(Category.POINTSET_POINT_FAILURE, "Point is not writable");
       state.value_state = Value_state.FAILURE;
-      dirty = state.value_state != previous;
       return;
     }
 
@@ -113,7 +127,6 @@ public void setConfig(PointPointsetConfig config) {
       state.status = createEntryFrom(Category.POINTSET_POINT_FAILURE, ex.getMessage());
       state.value_state = Value_state.FAILURE;
     }
-    dirty = state.value_state != previous;
   }
 
   @Override

pubber/src/main/java/daq/pubber/GatewayManager.java

@@ -1,7 +1,6 @@
 package daq.pubber;
 
 import static com.google.udmi.util.GeneralUtils.catchToNull;
-import static com.google.udmi.util.GeneralUtils.deepCopy;
 import static com.google.udmi.util.GeneralUtils.getNow;
 import static com.google.udmi.util.GeneralUtils.ifNotNullGet;
 import static com.google.udmi.util.GeneralUtils.ifNotNullThen;
@@ -11,14 +10,13 @@
 import static com.google.udmi.util.GeneralUtils.isTrue;
 import static java.lang.String.format;
 import static java.util.Optional.ofNullable;
+import static java.util.function.Predicate.not;
 import static udmi.schema.Category.GATEWAY_PROXY_TARGET;
 
 import com.google.udmi.util.SiteModel;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import udmi.schema.Config;
 import udmi.schema.Entry;
 import udmi.schema.GatewayConfig;
@@ -51,16 +49,12 @@ private Map<String, ProxyDevice> createProxyDevices(List<String> proxyIds) {
     }
 
     Map<String, ProxyDevice> devices = new HashMap<>();
-    if (!proxyIds.isEmpty()) {
-      String firstId = proxyIds.stream().sorted().findFirst().orElseThrow();
-      String noProxyId = ifTrueGet(isTrue(options.noProxy), () -> firstId);
-      ifNotNullThen(noProxyId, id -> warn(format("Not proxying device %s", noProxyId)));
-      proxyIds.forEach(id -> {
-        if (!id.equals(noProxyId)) {
-          devices.put(id, new ProxyDevice(host, id, config));
-        }
-      });
-    }
+
+    String firstId = proxyIds.stream().sorted().findFirst().orElse(null);
+    String noProxyId = ifTrueGet(isTrue(options.noProxy), () -> firstId);
+    ifNotNullThen(noProxyId, id -> warn(format("Not proxying device %s", noProxyId)));
+    proxyIds.stream().filter(not(id -> id.equals(noProxyId)))
+        .forEach(id -> devices.put(id, new ProxyDevice(host, id, config)));
 
     ifTrueThen(options.extraDevice, () -> devices.put(EXTRA_PROXY_DEVICE, makeExtraDevice()));
 
@@ -92,7 +86,9 @@ ProxyDevice makeExtraDevice() {
   }
 
   /**
-   * Update gateway operation based off of a gateway configuration block.
+   * Update gateway operation based off of a gateway configuration block. This happens in two
+   * slightly different forms, one for the gateway proper (primarily indicating what devices
+   * should be proxy targets), and the other for the proxy devices themselves.
    */
   public void updateConfig(GatewayConfig gateway) {
     if (gateway == null) {
@@ -107,7 +103,10 @@ public void updateConfig(GatewayConfig gateway) {
 
     if (gateway.proxy_ids == null || gateway.target != null) {
       try {
-        String family = validateGatewayFamily(catchToNull(() -> gateway.target.family));
+        String addr = catchToNull(() -> gateway.target.addr);
+        String family = ofNullable(catchToNull(() -> gateway.target.family))
+            .orElse(ProtocolFamily.VENDOR);
+        validateGatewayFamily(family, addr);
         setGatewayStatus(GATEWAY_PROXY_TARGET, Level.DEBUG, "gateway target family " + family);
       } catch (Exception e) {
         setGatewayStatus(GATEWAY_PROXY_TARGET, Level.ERROR, e.getMessage());
@@ -129,14 +128,17 @@ private void updateState() {
     updateState(ofNullable((Object) gatewayState).orElse(GatewayState.class));
   }
 
-  private String validateGatewayFamily(String family) {
-    if (family == null) {
-      return null;
+  private void validateGatewayFamily(String family, String addr) {
+    if (!ProtocolFamily.FAMILIES.contains(family)) {
+      throw new IllegalArgumentException("Unrecognized address family " + family);
+    }
+
+    String expectedAddr = catchToNull(() -> metadata.localnet.families.get(family).addr);
+
+    if (expectedAddr != null && !expectedAddr.equals(addr)) {
+      throw new IllegalStateException(
+          format("Family address was %s, expected %s", addr, expectedAddr));
     }
-    debug("Validating gateway family " + family);
-    Objects.requireNonNull(catchToNull(() -> metadata.localnet.families.get(family).addr),
-        format("Address family %s addr is null or undefined", family));
-    return family;
   }
 
   private void configExtraDevice() {

pubber/src/main/java/daq/pubber/ManagerBase.java

@@ -154,7 +154,7 @@ protected synchronized void startPeriodicSend() {
     warn(format("Starting %s %s sender with delay %ds",
         deviceId, this.getClass().getSimpleName(), sec));
     if (sec != 0) {
-      periodicUpdate(); // To this now to synchronously raise any obvious exceptions.
+      periodicUpdate(); // Do this now to synchronously raise any obvious exceptions.
       periodicSender = schedulePeriodic(sec, this::periodicUpdate);
     }
   }