IOS dynamic NAT rule order: sort by ACL name

projects/batfish/src/main/java/org/batfish/representation/cisco/CiscoIosDynamicNat.java

@@ -71,8 +71,38 @@ public int hashCode() {
   }
 
   @Override
-  protected int natCompare(CiscoIosNat other) {
-    return 0;
+  protected int natCompare(CiscoIosNat o) {
+    if (!(o instanceof CiscoIosDynamicNat)) {
+      return 0;
+    }
+    // Based on GNS3 testing, dynamic NAT rules are applied in order of ACL name.
+    // Deprioritize NATs with null ACLs, as they can't be converted at all.
+    CiscoIosDynamicNat other = (CiscoIosDynamicNat) o;
+    if (_aclName == null) {
+      return other._aclName == null ? 0 : -1;
+    } else if (other._aclName == null) {
+      return 1;
+    }
+    // ACLs with numeric names come first in numerical order, followed by others in lexicographical
+    // order. It is not possible to configure two rules with the same ACL.
+    int thisAcl = 0; // not a configurable ACL id
+    int otherAcl = 0;
+    try {
+      thisAcl = Integer.parseInt(_aclName);
+    } catch (NumberFormatException ignored) {
+      // expected
+    }
+    try {
+      otherAcl = Integer.parseInt(other._aclName);
+    } catch (NumberFormatException ignored) {
+      // expected
+    }
+    if (thisAcl != 0 && otherAcl != 0) {
+      return Integer.compare(otherAcl, thisAcl);
+    }
+    // Don't need to special-case exactly one ACL being numeric, because numbers come first
+    // lexicographically anyway. Non-numeric ACL names must begin with a letter.
+    return other._aclName.compareTo(_aclName);
   }
 
   @Override

projects/batfish/src/test/java/org/batfish/representation/cisco/CiscoIosDynamicNatTest.java

@@ -1,6 +1,13 @@
 package org.batfish.representation.cisco;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.lessThan;
+
+import com.google.common.collect.ImmutableList;
 import com.google.common.testing.EqualsTester;
+import java.util.List;
 import org.batfish.representation.cisco.CiscoIosNat.RuleAction;
 import org.junit.Test;
 
@@ -36,4 +43,34 @@ public void testEquals() {
     }
     et.testEquals();
   }
+
+  @Test
+  public void testNatCompare() {
+    // If both NATs have null ACLs, compare should give 0
+    assertThat(new CiscoIosDynamicNat().compareTo(new CiscoIosDynamicNat()), equalTo(0));
+
+    List<CiscoIosDynamicNat> ordered =
+        ImmutableList.of(
+            // Numeric ACLs come first, sorted numerically
+            natWithAcl("1"),
+            natWithAcl("3"),
+            natWithAcl("20"),
+            // Other ACLs are sorted lexicographically, case sensitive
+            natWithAcl("Z"),
+            natWithAcl("a"),
+            natWithAcl("b"),
+            new CiscoIosDynamicNat());
+    for (int i = 0; i < ordered.size() - 1; i++) {
+      for (int j = i + 1; j < ordered.size(); j++) {
+        assertThat(ordered.get(i).compareTo(ordered.get(j)), lessThan(0));
+        assertThat(ordered.get(j).compareTo(ordered.get(i)), greaterThan(0));
+      }
+    }
+  }
+
+  private static CiscoIosDynamicNat natWithAcl(String aclName) {
+    CiscoIosDynamicNat nat = new CiscoIosDynamicNat();
+    nat.setAclName(aclName);
+    return nat;
+  }
 }