allowing dst (node or IP) as a traceroute parameter

[ratulm]

Partial fix to #622.

Change dstIp parameter to dst which can be an IP or a hostname. If the latter, we use the "canonical IP" for the host.

---

This change is 

[dhalperi]

Reviewed 10 of 10 files at r1.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Configuration.java, line 325 at r1 (raw file):

  /** Returns the lowest IP across all interfaces for now. We'll improve it later. */
  @JsonIgnore

Add @Nullable

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Configuration.java, line 340 at r1 (raw file):

    } else {
      return null;
    }

Use the java8 built-in support for minimum over a stream.

  /** Returns the lowest IP across all interfaces for now. We'll improve it later. */
  @JsonIgnore
  @Nullable
  public Ip getCanonicalIp() {
    return getVrfs()
        .values()
        .stream()
        .flatMap(v -> v.getInterfaces().values().stream())
        .flatMap(i -> i.getAllPrefixes().stream())
        .map(Prefix::getAddress)
        .min(Ip::compareTo)
        .orElse(null);
  }

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Ip.java, line 103 at r1 (raw file):

  }

  public static Ip createIpOrNull(String addr) {

Would prefer to keep this logic inline where you have it now rather than in the Ip class itself. Does not seem very general.

---

projects/question/src/main/java/org/batfish/question/TracerouteQuestionPlugin.java, line 600 at r1 (raw file):

    }

    @Override

Why delete this @Override?

---

Comments from Reviewable

[ratulm]

Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Configuration.java, line 325 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Add @Nullable

Done.

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Configuration.java, line 340 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Use the java8 built-in support for minimum over a stream.

  /** Returns the lowest IP across all interfaces for now. We'll improve it later. */
  @JsonIgnore
  @Nullable
  public Ip getCanonicalIp() {
    return getVrfs()
        .values()
        .stream()
        .flatMap(v -> v.getInterfaces().values().stream())
        .flatMap(i -> i.getAllPrefixes().stream())
        .map(Prefix::getAddress)
        .min(Ip::compareTo)
        .orElse(null);
  }

Done.

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Ip.java, line 103 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Would prefer to keep this logic inline where you have it now rather than in the Ip class itself. Does not seem very general.

done. had to change the visibility of a function in Ip.java.

---

projects/question/src/main/java/org/batfish/question/TracerouteQuestionPlugin.java, line 600 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Why delete this @Override?

accident. done.

---

Comments from Reviewable

[dhalperi]

Reviewed 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Ip.java, line 37 at r2 (raw file):

  public static final Ip ZERO = new Ip(0L);

  public static long ipStrToLong(String addr) {

Why do this vs just using the new Ip(String) constructor?, which calls ipStrToLong internally?

---

projects/question/src/main/java/org/batfish/question/TracerouteQuestionPlugin.java, line 307 at r2 (raw file):

      if (_dst != null) {
        try {
          Ip dstIp = new Ip(Ip.ipStrToLong(_dst));

I think this is the same as the existing new Ip(String) constructor without the visibility change.

---

Comments from Reviewable

[ratulm]

Review status: 9 of 11 files reviewed at latest revision, 2 unresolved discussions.

---

projects/batfish-common-protocol/src/main/java/org/batfish/datamodel/Ip.java, line 37 at r2 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

Why do this vs just using the new Ip(String) constructor?, which calls ipStrToLong internally?

argh. i am stupid.

---

projects/question/src/main/java/org/batfish/question/TracerouteQuestionPlugin.java, line 307 at r2 (raw file):

argh. i am stupid.

---

Comments from Reviewable

[dhalperi]

Reviewed 1 of 2 files at r3.
Review status: 10 of 11 files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[dhalperi]

---

Reviewed 1 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable