Fix overrides of project spec parameters from command line (#874)

bin/test_regclean

@@ -26,6 +26,7 @@ echo "export UDMI_REGISTRY_SUFFIX=$UDMI_REGISTRY_SUFFIX"
 echo "export UDMI_ALT_REGISTRY=$UDMI_ALT_REGISTRY"
 
 site_path=sites/udmi_site_model
+iot_config=$site_path/cloud_iot_config.json
 output_file=$site_path/out/registration_summary.json
 csv_file=$site_path/out/registration_summary.csv
 pubber_config=/tmp/pubber_config.json
@@ -52,8 +53,8 @@ else
     registrar_opts=
 fi
 
-registry_id=`jq -r .registry_id $site_path/cloud_iot_config.json`
-cloud_region=`jq -r .cloud_region $site_path/cloud_iot_config.json`
+registry_id=`jq -r .registry_id $iot_config`
+cloud_region=`jq -r .cloud_region $iot_config`
 full_config=$(realpath $site_path)/full_config.json
 
 site_args="$site_path $project_spec $registrar_opts"
@@ -119,10 +120,15 @@ rm -rf $fake_device && cp -a $clone_device $fake_device
 jq ".localnet.families.vendor.addr = \"2183213\"" $fake_metadata | sponge $fake_metadata
 
 # Construct a fullly specified configuration to test alternate forms.
-jq ".iot_provider = $iot_provider" $site_path/cloud_iot_config.json > $full_config
+jq ".iot_provider = $iot_provider" $iot_config > $full_config
 jq ".project_id = \"$project_id\"" $full_config | sponge $full_config
 jq ".udmi_namespace = $quoted_namespace" $full_config | sponge $full_config
 
+# And corrupt the default model, to make sure cmdline overrides work
+jq '.iot_provider = "dynamic"' $iot_config | sponge $iot_config
+jq '.project_id = "this-is-not-right"' $iot_config | sponge $iot_config
+jq '.udmi_namespace = "bunny"' $iot_config | sponge $iot_config
+
 echo Clean out the registry to make sure devices get removed...
 echo bin/registrar $site_args -d -x
 bin/registrar $site_args -d -x

common/src/main/java/com/google/udmi/util/SiteModel.java

@@ -90,10 +90,14 @@ public class SiteModel {
   private Map<String, CloudModel> allDevices;
 
   public SiteModel(String specPath) {
-    this(specPath, (Supplier<String>) null);
+    this(specPath, null, null);
   }
 
-  public SiteModel(String specPath, Supplier<String> specSupplier) {
+  public SiteModel(String sitePath, ExecutionConfiguration config) {
+    this(sitePath, null, config);
+  }
+
+  public SiteModel(String specPath, Supplier<String> specSupplier, ExecutionConfiguration overrides) {
     File specFile = new File(requireNonNull(specPath, "site model not defined"));
     boolean specIsFile = specFile.isFile();
     siteConf = specIsFile ? specFile : cloudConfigPath(specFile);
@@ -108,10 +112,15 @@ public SiteModel(String specPath, Supplier<String> specSupplier) {
     siteDefaults = ofNullable(
         asMap(loadFileStrict(Metadata.class, getSubdirectory(SITE_DEFAULTS_FILE))))
         .orElseGet(HashMap::new);
+    if (overrides != null && overrides.project_id != null) {
+      exeConfig.iot_provider = overrides.iot_provider;
+      exeConfig.project_id = overrides.project_id;
+      exeConfig.udmi_namespace = overrides.udmi_namespace;
+    }
   }
 
   public SiteModel(String toolName, List<String> argList) {
-    this(removeArg(argList, "site_model"), projectSpecSupplier(argList));
+    this(removeArg(argList, "site_model"), projectSpecSupplier(argList), null);
     ExecutionConfiguration executionConfiguration = getExecutionConfiguration();
     File outFile = new File(CONFIG_OUT_DIR, format("%s_conf.json", toolName));
     System.err.println("Writing reconciled configuration file to " + outFile.getAbsolutePath());
@@ -244,9 +253,6 @@ public static String getRegistryActual(String namespace,
 
   private static void augmentConfig(ExecutionConfiguration exeConfig, Matcher specMatcher) {
     try {
-      checkState(exeConfig.iot_provider == null, "config file iot_provider should be null");
-      checkState(exeConfig.project_id == null, "config file project_id should be null");
-      checkState(exeConfig.udmi_namespace == null, "config file udmi_namespace should be null");
       String iotProvider = specMatcher.group(SPEC_PROVIDER_GROUP);
       exeConfig.iot_provider = ifNotNullGet(iotProvider, IotProvider::fromValue);
       exeConfig.project_id = specMatcher.group(SPEC_PROJECT_GROUP);
@@ -406,8 +412,6 @@ private String getDeviceKeyPrefix(String targetId) {
 
   /**
    * Get the site registry name.
-   *
-   * @return site registry
    */
   public String getRegistryId() {
     return getRegistryActual(exeConfig);

validator/src/main/java/com/google/bos/iot/core/proxy/IotReflectorClient.java

@@ -44,7 +44,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.TreeMap;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.CountDownLatch;

validator/src/main/java/com/google/daq/mqtt/mapping/MappingAgent.java

@@ -159,7 +159,7 @@ private Entry<String, Metadata> convertExtra(File file) {
 
   private void initialize() {
     cloudIotManager = new CloudIotManager(executionConfiguration);
-    siteModel = new SiteModel(cloudIotManager.getSiteDir());
+    siteModel = new SiteModel(cloudIotManager.getSiteDir(), executionConfiguration);
     siteModel.initialize();
   }
 

validator/src/main/java/com/google/daq/mqtt/sequencer/SequenceRunner.java

@@ -107,20 +107,20 @@ public static void handleRequest(Map<String, String> params) {
     final String deviceId = params.remove(WebServerRunner.DEVICE_PARAM);
     final String testMode = params.remove(WebServerRunner.TEST_PARAM);
 
-    SiteModel siteModel = new SiteModel(sitePath);
-    siteModel.initialize();
-
     ExecutionConfiguration config = new ExecutionConfiguration();
     config.project_id = projectId;
     config.site_model = sitePath;
     config.device_id = deviceId;
-    config.key_file = siteModel.validatorKey();
     config.serial_no = Optional.ofNullable(serialNo).orElse(SequenceBase.SERIAL_NO_MISSING);
     config.log_level = Level.INFO.name();
     config.udmi_version = Common.getUdmiVersion();
     config.udmi_root = TOOL_ROOT;
     config.alt_project = testMode; // Sekrit hack for enabling mock components.
 
+    SiteModel siteModel = new SiteModel(sitePath, config);
+    siteModel.initialize();
+    config.key_file = siteModel.validatorKey();
+
     failures.clear();
     allTestResults.clear();
 
@@ -177,8 +177,11 @@ static ExecutionConfiguration ensureExecutionConfig() {
     try {
       System.err.println("Reading config file " + configFile.getAbsolutePath());
       ExecutionConfiguration exeConfig = ConfigUtil.readValidatorConfig(configFile);
-      SiteModel model = new SiteModel(exeConfig.site_model);
+      String udmiNamespace = exeConfig.udmi_namespace;
+      exeConfig.udmi_namespace = null; // Prevent having this processed twice.
+      SiteModel model = new SiteModel(exeConfig.site_model, exeConfig);
       model.initialize();
+      exeConfig.udmi_namespace = udmiNamespace;
       reportLoadingErrors(model, exeConfig.device_id);
       exeConfig.cloud_region = ofNullable(exeConfig.cloud_region)
           .orElse(model.getCloudRegion());

validator/src/main/java/com/google/daq/mqtt/validator/Reflector.java

@@ -181,7 +181,18 @@ public void setSiteDir(String siteDir) {
       File cloudConfig = new File(siteDir, "cloud_iot_config.json");
       ExecutionConfiguration siteConfig = CloudIotManager.validate(
           ConfigUtil.readExeConfig(cloudConfig), executionConfiguration.project_id);
-      executionConfiguration = mergeObject(executionConfiguration, siteConfig);
+
+      // These parameters should always be taken from the site_model.
+      executionConfiguration.registry_id = siteConfig.registry_id;
+      executionConfiguration.cloud_region = siteConfig.cloud_region;
+      executionConfiguration.site_name = siteConfig.site_name;
+
+      // Only use the site_model values for project_spec if not otherwise specified.
+      if (executionConfiguration.project_id == null) {
+        executionConfiguration.iot_provider = siteConfig.iot_provider;
+        executionConfiguration.project_id = siteConfig.project_id;
+        executionConfiguration.udmi_namespace = siteConfig.udmi_namespace;
+      }
     }
   }
 }