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.

Sign up for GitHub

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

Add validation to Registrar. Add RegistrarTest to exercise it. #361

Merged
merged 18 commits into from
Jun 23, 2022
Merged

Add validation to Registrar. Add RegistrarTest to exercise it. #361

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
4b3824a
Add dryrun option -n
johnrandolph Jun 2, 2022
e0c6de0
Oops, shift by the option
johnrandolph Jun 2, 2022
a6b72b2
Remove dbg
johnrandolph Jun 2, 2022
bf78213
Merge remote-tracking branch 'faucet/master'
johnrandolph Jun 2, 2022
5e9de32
Merge remote-tracking branch 'faucet/master'
johnrandolph Jun 3, 2022
00ac587
Merge remote-tracking branch 'faucet/master'
johnrandolph Jun 7, 2022
b60ecdb
First pass at RegistrarTest and slight changes to Registrar.
johnrandolph Jun 9, 2022
2ad6e87
Add validation tests
johnrandolph Jun 14, 2022
d159768
Test work
johnrandolph Jun 21, 2022
6793eff
Test improvements
johnrandolph Jun 22, 2022
0f662bb
Merge branch 'master' into master
johnrandolph Jun 22, 2022
167e8de
Merge
johnrandolph Jun 22, 2022
eeb32b4
Fixes from first review
johnrandolph Jun 22, 2022
cc2f8b6
Move execute to method, keep argv parsing in class
johnrandolph Jun 23, 2022
7afa5b0
Import
johnrandolph Jun 23, 2022
bac9875
Merge remote-tracking branch 'faucet/master'
johnrandolph Jun 23, 2022
fdfda2e
Lint fixes
johnrandolph Jun 23, 2022
45ea044
Up the warnings
johnrandolph Jun 23, 2022
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
64 changes: 31 additions & 33 deletions validator/src/main/java/com/google/daq/mqtt/registrar/Registrar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public class Registrar {
private Metadata siteMetadata;
private Map<String, Map<String, String>> lastErrorSummary;
private boolean validateMetadata = false;
private List<String> deviceList;

/**
* Main entry point for registrar.
Expand All @@ -111,37 +112,11 @@ public class Registrar {
public static void main(String[] args) {
ArrayList<String> argList = new ArrayList<>(List.of(args));
Registrar registrar = new Registrar();
executeWithRegistrar(registrar, argList);
processArgs(argList, registrar);
registrar.execute();
}

public static void executeWithRegistrar(Registrar registrar, ArrayList<String> argList) {
try {
boolean processAllDevices = processArgs(argList, registrar);

if (registrar.schemaBase == null) {
registrar.setToolRoot(null);
}

registrar.loadSiteMetadata();

if (processAllDevices) {
registrar.processDevices();
} else {
registrar.processDevices(argList);
}

registrar.writeErrors();
registrar.shutdown();
} catch (ExceptionMap em) {
ExceptionMap.format(em, ERROR_FORMAT_INDENT).write(System.err);
throw new RuntimeException("mapped exceptions", em);
} catch (Exception ex) {
ex.printStackTrace();
throw new RuntimeException("main exception", ex);
}
}

private static boolean processArgs(List<String> argList, Registrar registrar) {
public static void processArgs(List<String> argList, Registrar registrar) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still not sure why this is static? The general adage with static methods for Java is to avoid them unless they need to be static for some reason. I think this comes down to the issue that you can't mock/override a static method, so it makes future testing/extension a problem sometimes. Not likely in this case, but just on general practice. E.g., I would expect this to be just a regular method on the Registrar class (no need to pass in the explicit parameter). Anyway, that's mainly the Java-style that I've always used, but it's been so long that I'm not 100% sure where it came from. I checked the Java style guide and it doesn't say anything about it. Doesn't really matter -- I also optimize for "least number of words" and if you made it a class method, all references to "registrar." go away reducing the overall footprint of the class -- and there's no world in which you would call this method without a registrar class!

while (argList.size() > 0) {
String option = argList.remove(0);
switch (option) {
Expand All @@ -167,16 +142,35 @@ private static boolean processArgs(List<String> argList, Registrar registrar) {
registrar.setValidateMetadata(true);
break;
case "--":
return false;
break;
default:
if (option.startsWith("-")) {
throw new RuntimeException("Unknown cmdline option " + option);
}
// Add the current non-option back into the list and use it as device names list.
argList.add(0, option);
return false;
registrar.setDeviceList(argList);
return;
}
}
return true;
}

public void execute() {
try {
if (schemaBase == null) {
setToolRoot(null);
}
loadSiteMetadata();
processDevices();
writeErrors();
shutdown();
} catch (ExceptionMap em) {
ExceptionMap.format(em, ERROR_FORMAT_INDENT).write(System.err);
throw new RuntimeException("mapped exceptions", em);
} catch (Exception ex) {
ex.printStackTrace();
throw new RuntimeException("main exception", ex);
}
}

private void setIdleLimit(String option) {
Expand All @@ -192,6 +186,10 @@ private void setValidateMetadata(boolean validateMetadata) {
this.validateMetadata = validateMetadata;
}

private void setDeviceList(List<String> deviceList) {
this.deviceList = deviceList;
}

private void setFeedTopic(String feedTopic) {
System.err.println("Sending device feed to topic " + feedTopic);
feedPusher = new PubSubPusher(projectId, feedTopic);
Expand Down Expand Up @@ -279,7 +277,7 @@ private String getGenerationString() {
}

private void processDevices() {
processDevices(null);
processDevices(this.deviceList);
}

private void processDevices(List<String> devices) {
Expand Down
7 changes: 5 additions & 2 deletions validator/src/main/java/com/google/daq/mqtt/registrar/RegistrarTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import org.junit.Test;
Expand Down Expand Up @@ -72,7 +73,8 @@ private RegistrarUnderTest getRegistrarUnderTest() {
ArrayList<String> argList = new ArrayList<String>();
argList.add("-s");
argList.add(SITE_PATH);
registrar.executeWithRegistrar(registrar, argList);
Registrar.processArgs(argList, registrar);
registrar.execute();
assertErrorSummaryValidateSuccess(registrar.getLastErrorSummary());
}

Expand All @@ -83,7 +85,8 @@ private RegistrarUnderTest getRegistrarUnderTest() {
argList.add("-t");
argList.add("-s");
argList.add(SITE_PATH);
registrar.executeWithRegistrar(registrar, argList);
Registrar.processArgs(argList, registrar);
registrar.execute();
assertErrorSummaryValidateFailure(registrar.getLastErrorSummary());
}

Expand Down