Skip to content
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.

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

Fix subType streaming validator errors #272

Merged
merged 15 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions .gencode_hash.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
f2bdc1264c53c504d170399fd11d2ac15fa0eee62f35ef8892a0d6b23a3bcc1b gencode/docs/config.html
7f4f659da86796660d8ed610bb74bb2e6cff6688a1e92b2019b20ebc34227033 gencode/docs/envelope.html
8221d216d08bc6b67f6f081132aa86f78934149bc5be23fe9132eb187d176a86 gencode/docs/envelope.html
99a1cc3cb90112a06997feae86f27f6e4fb415b56746257a4bd08a205d616fef gencode/docs/event_discovery.html
8e3be05180ede024f0376e9f4102406f603f72b125df2b923043aa8b7b0637cc gencode/docs/event_pointset.html
5d173e050b2583b442636153559fe3bd2fb26efe85ff6db01cc0123bcff9b266 gencode/docs/event_system.html
Expand All @@ -19,7 +19,7 @@ b6ff9b8739a9c3bb6972f73db6fc54f451189c13b273e58bc11cb3d82c74ad40 gencode/java/u
c6a74b140cd4098269fe1f3a296a05e38dffc69604e2708cc154b40607e46418 gencode/java/udmi/schema/Config.java
09903b8d8897e2478b0acb3a346050adc166a21694ff32a763b247e0d065090b gencode/java/udmi/schema/Discovery.java
090bbaf1508aa6ca8380af936af990673f300eb5a940c9e8ab94deb64efa2b7b gencode/java/udmi/schema/Entry.java
8ba08bd70eadd32129e2101a8b8dfd1ed90a4592822ab055db90e845c7e91dbb gencode/java/udmi/schema/Envelope.java
e22684e98a6dd9f213093c4e8293f353cd5faafb66264fc34a06167c6c3833a3 gencode/java/udmi/schema/Envelope.java
82826aecc86ea17cb978913a260fdc79ee0476a27a519cc608bc80595212cbb8 gencode/java/udmi/schema/Event.java
ac5fef42349b983a9f64bd875bc530cbee7bbb79a7f690bb49ac8a977ec89e78 gencode/java/udmi/schema/FamilyDiscoveryEvent.java
bb91a167bca6f92c779d60dbf1ce1372b9a198f49978deb8ec7ebcd6632aa80b gencode/java/udmi/schema/Firmware.java
Expand Down
16 changes: 13 additions & 3 deletions .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,28 @@ jobs:
- name: code checks
run: |
bin/setup_base
bin/clone_model
bin/gencode check
pubber/bin/build check
validator/bin/build check
- name: telemetry validator
env:
GCP_TARGET_PROJECT: ${{ secrets.GCP_TARGET_PROJECT }}
if: "${{ env.GCP_TARGET_PROJECT != '' }}"
run: |
bin/test_validator $GCP_TARGET_PROJECT
cat out/validation_report.json && echo
cat out/devices/AHU-1/state.json && echo
diff -u /tmp/validator.out etc/validator.out && echo No validator diff detected.
- name: sequence tests
env:
GCP_TARGET_PROJECT: ${{ secrets.GCP_TARGET_PROJECT }}
if: "${{ env.GCP_TARGET_PROJECT != '' }}"
run: |
bin/test_sequencer $GCP_TARGET_PROJECT
fgrep 'RESULT ' /tmp/test_log.txt | sed -e 's/.*sequencer RESULT/RESULT/' > /tmp/results.txt
diff /tmp/results.txt etc/sequence.out && echo No output diff detected.
- name: validation logs
fgrep 'RESULT ' /tmp/test_log.txt | sed -e 's/.*sequencer RESULT/RESULT/' > /tmp/sequencer.out
diff -u /tmp/sequencer.out etc/sequencer.out && echo No output diff detected.
- name: device ouput logs
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: output

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll fold this into another PR

if: ${{ always() }}
run: more `find out/devices/ -type f`
- name: pubber logs
Expand Down
1 change: 0 additions & 1 deletion bin/test_sequencer
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ fi
project_id=$1
shift

bin/clone_model
site_path=udmi_site_model
device_id=AHU-1

Expand Down
68 changes: 68 additions & 0 deletions bin/test_validator
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#!/bin/bash -e

ROOT_DIR=$(dirname $0)/..
cd $ROOT_DIR

if [[ $# != 1 ]]; then
echo Usage: $0 PROJECT_ID
false
fi
project_id=$1
shift

site_path=udmi_site_model
device_id=AHU-1

serial_no=validator-$RANDOM
echo Using pubber with serial $serial_no

PUBBER_OUT=pubber.out

pids=`ps ax | fgrep pubber | fgrep java | awk '{print $1}'`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This feels heavy handed? Is this to strengthen repeated runs of this test, or to stop interference with other running tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes -- but I'd defer any of those changes to something separate from the basic functionality. I agree with your comments, but I wouldn't want to get into it as part of this PR!

if [[ -n $pids ]]; then
echo Killing pubber pids $pids
kill $pids
fi

rm -rf out/devices

# Prepare auth key used by reflector
cp udmi_site_model/devices/AHU-1/rsa_private.pkcs8 \
udmi_site_model/gcp_reflect_key.pkcs8

bin/validator $site_path $project_id &
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we trap on exit of validator so we know if it exits while running pubber?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe-- the code as it stands checks the output of what validator does, so it doesn't matter if it exits prematurely or not... Either the generated output is correct or not.

vpid=$!
sleep 10
echo Started validator pid $vpid

pubber/bin/build

echo Writing pubber output to $PUBBER_OUT
echo bin/pubber $site_path $project_id $device_id $serial_no
bin/pubber $site_path $project_id $device_id $serial_no > $PUBBER_OUT 2>&1 &

WAITING=10
for i in `seq 1 $WAITING`; do
if fgrep "Connection complete" $PUBBER_OUT; then
break
fi
echo Waiting for pubber startup $((WAITING - i))...
sleep 2
done

if [[ $i == $WAITING ]]; then
echo pubber startup failed:
cat $PUBBER_OUT
false
fi

echo Waiting for system to run for a bit...
sleep 30

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this becomes more cumbersome, or we get unpredictable test results, I suggest using the shell's jobs command to focus our kill usage

For example:

jobs -x kill %pubber
jobs -x kill %validator

Although I'm not sure whether the shell would see either java or pubber/validator

pids=`ps ax | fgrep pubber | fgrep java | awk '{print $1}'`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Or at least pids=`pgrep pubber` ?

echo Killing pids $vpid $pids
kill $vpid $pids

outfiles=`find out/devices -name \*.out` || true
echo Found .out files $outfiles, copying to /tmp/validator.out
more $outfiles > /tmp/validator.out || true
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be cat?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

more includes the filename in the output, so it's convenient for logging since you get more context, rather than just a bunch of stuff combined together

18 changes: 13 additions & 5 deletions bin/validator
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,18 @@

ROOT_DIR=$(realpath $(dirname $0)/..)

if [ $# != 3 ]; then
echo $0 SITE_PATH PROJECT_ID SUBSCRIPTION_ID
if [ $# != 2 -a $# != 3 ]; then
echo $0 SITE_PATH PROJECT_ID [SUBSCRIPTION_ID]
false
fi

site_path=$(realpath $1)
project_id=$2
subscription=$3
shift 3
shift 2
if [[ -n $1 ]]; then
subscription=$1
shift
fi

if [ ! -f $site_path/cloud_iot_config.json ]; then
echo cloud_iot_config.json not found at $site_path
Expand All @@ -22,4 +25,9 @@ $ROOT_DIR/validator/bin/build > /dev/null

echo Running tools version `(cd $ROOT_DIR; git describe)`

$ROOT_DIR/validator/bin/validate $project_id schema pubsub $subscription $site_path
if [[ -n $subscription ]]; then
$ROOT_DIR/validator/bin/validate $project_id schema pubsub $subscription $site_path
else
echo No subscription supplied, assuming UDMI reflector.
$ROOT_DIR/validator/bin/validate $project_id schema reflect -- $site_path
fi
File renamed without changes.
23 changes: 23 additions & 0 deletions etc/validator.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
::::::::::::::
out/devices/AHU-1/state_pointset.out
::::::::::::::
While converting to json node: 1 schema violations found
1 schema violations found
object instance has properties which are not allowed by the schema: ["version"]
::::::::::::::
out/devices/AHU-1/state_system.out
::::::::::::::
While converting to json node: 2 schema violations found
2 schema violations found
object has missing required properties (["last_config"])
object instance has properties which are not allowed by the schema: ["version"]
::::::::::::::
out/devices/AHU-1/state.out
::::::::::::::
While converting to json node: 2 schema violations found
2 schema violations found
instance value ("states") not found in enum (possible values: ["update","discovery","system","gateway","localnet","metadata","pointset","blobset"])
instance value ("update") not found in enum (possible values: ["event","command","state","config"])
While converting to json node: 1 schema violations found
1 schema violations found
object has missing required properties (["last_config"])
4 changes: 2 additions & 2 deletions gencode/docs/envelope.html

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions gencode/java/udmi/schema/Envelope.java

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions schema/envelope.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
},
"subFolder": {
"enum": [
"update",
"discovery",
"system",
"gateway",
Expand All @@ -44,7 +45,6 @@
"deviceRegistryId",
"deviceNumId",
"deviceId",
"subFolder",
"subType"
"subFolder"
]
}
2 changes: 1 addition & 1 deletion tests/envelope.tests/empty.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ Validating 1 schemas
Validating 1 files against envelope.json
Against input envelope.tests/empty.json
1 schema violations found
object has missing required properties (["deviceId","deviceNumId","deviceRegistryId","projectId","subFolder","subType"])
object has missing required properties (["deviceId","deviceNumId","deviceRegistryId","projectId","subFolder"])
2 changes: 1 addition & 1 deletion tests/envelope.tests/errors2.out
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ Validating 1 schemas
3 schema violations found
ECMA 262 regex "^[0-9]+$" does not match input string "-9213923812"
ECMA 262 regex "^[A-Z]{2,6}-[0-9]{1,6}$" does not match input string "FCUs_02_NW_12"
object has missing required properties (["projectId","subFolder","subType"])
object has missing required properties (["projectId","subFolder"])
4 changes: 2 additions & 2 deletions validator/.idea/runConfigurations/Validator.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
import java.util.TreeSet;
import java.util.function.BiConsumer;
import java.util.stream.Collectors;
import udmi.schema.Envelope.SubFolder;
import udmi.schema.Envelope.SubType;
import udmi.schema.Metadata;
import udmi.schema.PointsetEvent;

Expand Down Expand Up @@ -85,12 +87,14 @@ public class Validator {
private static final String REPORT_JSON_FILENAME = "validation_report.json";
public static final File METADATA_REPORT_FILE = new File(OUT_BASE_FILE, REPORT_JSON_FILENAME);
private static final String DEVICE_REGISTRY_ID_KEY = "deviceRegistryId";
private static final String UNKNOWN_SCHEMA_DEFAULT = "unknown";
private static final String UNKNOWN_FOLDER_DEFAULT = "unknown";
private static final String EVENT_POINTSET = "event_pointset";
private static final String GCP_REFLECT_KEY_PKCS8 = "gcp_reflect_key.pkcs8";
private static final String EMPTY_MESSAGE = "{}";
private static final String CONFIG_PREFIX = "config_";
private static final String STATE_PREFIX = "state_";
private static final String UNKNOWN_TYPE_DEFAULT = "event";

private final String projectId;
private final Map<String, ReportingDevice> expectedDevices = new TreeMap<>();
private final Set<String> extraDevices = new TreeSet<>();
Expand Down Expand Up @@ -252,7 +256,7 @@ private void validatePubSub(String instName) {
}

private void validateReflector(String instName) {
deviceId = instName;
deviceId = NO_SITE.equals(instName) ? null : instName;
String keyFile = new File(siteDir, GCP_REFLECT_KEY_PKCS8).getAbsolutePath();
System.err.println("Loading reflector key file from " + keyFile);
IotCoreClient client = new IotCoreClient(projectId, cloudIotConfig, keyFile);
Expand All @@ -261,7 +265,7 @@ private void validateReflector(String instName) {

private void messageLoop(MessagePublisher client) {
System.err.println(
"Entering message loop on " + client.getSubscriptionId() + " for device " + deviceId);
"Entering message loop on " + client.getSubscriptionId() + " with device " + deviceId);
BiConsumer<Map<String, Object>, Map<String, String>> validator = messageValidator();
boolean initialized = false;
while (client.isActive()) {
Expand All @@ -279,8 +283,10 @@ private void messageLoop(MessagePublisher client) {
}

private void sendInitializationQuery(MessagePublisher client) {
System.err.println("Sending initialization query messages");
client.publish(deviceId, STATES_QUERY_TOPIC, EMPTY_MESSAGE);
if (deviceId != null) {
System.err.println("Sending initialization query messages for device " + deviceId);
client.publish(deviceId, STATES_QUERY_TOPIC, EMPTY_MESSAGE);
}
}

private Set<String> convertIgnoreSet(String ignoreSpec) {
Expand Down Expand Up @@ -464,22 +470,29 @@ private String makeSchemaName(Map<String, String> attributes) {
String subType = attributes.get("subType");

if (Strings.isNullOrEmpty(subFolder)) {
subFolder = UNKNOWN_SCHEMA_DEFAULT;
subFolder = UNKNOWN_FOLDER_DEFAULT;
}

if (Strings.isNullOrEmpty(subType)) {
return "event_" + subFolder;
subType = UNKNOWN_TYPE_DEFAULT;
}

if (subFolder.equals("update")) {
if (!subType.equals("config") && !subType.equals("state")) {
throw new RuntimeException("Unrecognized update type " + subType);
if (matches(subType, SubFolder.UPDATE)) {
if (!subFolder.endsWith("s")) {
throw new RuntimeException("Update subFolder missing plural: " + subFolder);
}
String singularFolder = subFolder.substring(0, subFolder.length() - 1);
if (!matches(singularFolder, SubType.CONFIG) && !matches(singularFolder, SubType.STATE)) {
throw new RuntimeException("Unrecognized update type: " + subFolder);
}
} else if (!subType.endsWith("s")) {
throw new RuntimeException("Malformed plural subType " + subType);
return singularFolder;
}

return String.format("%s_%s", subType.substring(0, subType.length() - 1), subFolder);
return String.format("%s_%s", subType, subFolder);
}

private boolean matches(String target, Object value) {
return target.equals(value.toString());
}

private ReportingDevice getReportingDevice(String deviceId) {
Expand Down