FRR: convert ospf area range

[dhalperi]

No description provided.

[batfish-bot]

This change is 

[codecov]

Codecov Report

Merging #6665 (560340b) into master (b483351) will increase coverage by 0.00%.
The diff coverage is 89.47%.

@@            Coverage Diff            @@
##             master    #6665   +/-   ##
=========================================
  Coverage     73.52%   73.53%           
- Complexity    36634    36672   +38     
=========================================
  Files          2933     2934    +1     
  Lines        147422   147520   +98     
  Branches      17771    17782   +11     
=========================================
+ Hits         108389   108474   +85     
- Misses        30503    30507    +4     
- Partials       8530     8539    +9     

Impacted Files	Coverage Δ	Complexity Δ
...epresentation/cumulus/CumulusFrrConfiguration.java	93.75% <66.66%> (-2.81%)	17.00 <1.00> (+1.00)	⬇️
...ish/representation/cumulus/CumulusConversions.java	81.71% <90.90%> (+0.29%)	183.00 <5.00> (+4.00)
...ar/cumulus_frr/CumulusFrrConfigurationBuilder.java	77.67% <100.00%> (+0.07%)	216.00 <1.00> (ø)
...va/org/batfish/representation/cumulus/OspfVrf.java	100.00% <100.00%> (ø)	7.00 <1.00> (+1.00)
.../org/batfish/dataplane/rib/RouteAdvertisement.java	83.67% <0.00%> (-4.09%)	18.00% <0.00%> (-1.00%)
...ain/java/org/batfish/symbolic/IngressLocation.java	65.78% <0.00%> (-2.64%)	15.00% <0.00%> (-1.00%)
...org/batfish/datamodel/flow/BidirectionalTrace.java	81.81% <0.00%> (-2.28%)	14.00% <0.00%> (-1.00%)
...src/main/java/org/batfish/coordinator/PoolMgr.java	59.52% <0.00%> (-1.20%)	15.00% <0.00%> (-1.00%)
...ain/java/org/batfish/storage/FileBasedStorage.java	84.76% <0.00%> (-0.67%)	249.00% <0.00%> (ø%)
...java/org/batfish/dataplane/ibdp/VirtualRouter.java	90.32% <0.00%> (-0.32%)	206.00% <0.00%> (-1.00%)
... and 10 more

[corinaminer]

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @arifogel and @dhalperi)

---

projects/batfish/src/main/java/org/batfish/representation/cumulus/CumulusConversions.java, line 1494 at r1 (raw file):

      }
      // Anything not summarized is permitted.
      lines.add(new RouteFilterLine(LineAction.PERMIT, PrefixRange.ALL));

nit: won't this make lines have size vsArea.getRanges().size() + 1? Not sure how strict ImmutableList.builderWithExpectedSize is

---

projects/batfish/src/main/java/org/batfish/representation/cumulus/OspfVrf.java, line 32 at r1 (raw file):

  public @Nonnull Map<Long, OspfArea> getAreas() {
    return Collections.unmodifiableMap(_areas);

how is this different from/preferable to ImmutableMap.copyOf()? Either may be fine, i've just never seen this one

---

projects/batfish/src/test/java/org/batfish/grammar/cumulus_frr/CumulusFrrGrammarTest.java, line 1918 at r1 (raw file):

      assertThat(area.getSummaries(), hasKeys(Prefix.parse("1.255.0.0/17")));
      OspfAreaSummary summary = Iterables.getOnlyElement(area.getSummaries().values());
      assertThat(summary.getAdvertised(), equalTo(true));

i assume this means you confirmed FRR should always advertise summary routes (when contributing routes are present)?

[dhalperi]

Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @arifogel and @corinaminer)

---

projects/batfish/src/main/java/org/batfish/representation/cumulus/CumulusConversions.java, line 1494 at r1 (raw file):

Previously, corinaminer (Corina Miner) wrote…

nit: won't this make lines have size vsArea.getRanges().size() + 1? Not sure how strict ImmutableList.builderWithExpectedSize is

It's not strict (you don't have to be right about the expectation, it will do the usual behavior of reallocating bigger), but allocating the correct size will be more efficient space-wise. Good catch!

---

projects/batfish/src/main/java/org/batfish/representation/cumulus/OspfVrf.java, line 32 at r1 (raw file):

Previously, corinaminer (Corina Miner) wrote…

how is this different from/preferable to ImmutableMap.copyOf()? Either may be fine, i've just never seen this one

This one is a wrapper. It doesn't iterate on or copy the underlying structure, it does reflect updates.

I tend to use this in conversion code since the copy won't be used for anything.

---

projects/batfish/src/test/java/org/batfish/grammar/cumulus_frr/CumulusFrrGrammarTest.java, line 1918 at r1 (raw file):

Previously, corinaminer (Corina Miner) wrote…

i assume this means you confirmed FRR should always advertise summary routes (when contributing routes are present)?

Yes.

[corinaminer]

Reviewed 1 of 1 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @arifogel)