Fix OSPF session semantics for interfaces with multiple addresses

[arifogel]

• Key OSPF sessions additionally on ConcreteInterfaceAddress
• Check compatibility of addresses
• Consider all addresses instead of just primary

[batfish-bot]

This change is 

[progwriter]

Reviewed 15 of 15 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arifogel)

---

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

              .forEach(
                  (neighborId, neighbor) -> {
                    if (neighbor.isPassive()) {

nit: I'd probably factor this out

[arifogel]

Reviewable status: 15 of 17 files reviewed, 1 unresolved discussion (waiting on @progwriter)

---

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

Previously, progwriter (Victor Heorhiadi) wrote…

nit: I'd probably factor this out

Factor what out? I don't see anything in the loop that can happen outside.

[arifogel]

Reviewable status: 14 of 17 files reviewed, all discussions resolved (waiting on @progwriter)

---

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

Previously, arifogel (Ari Fogel) wrote…

Factor what out? I don't see anything in the loop that can happen outside.

made loop simpler anyhow. resolving.

[arifogel]

Reviewed 3 of 3 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved

[progwriter]

Reviewable status: complete! all files reviewed, all discussions resolved

---

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

Previously, arifogel (Ari Fogel) wrote…

made loop simpler anyhow. resolving.

I find very nested loops are difficult to test, whereas a static func that takes several params is easier to reason about.
However, not worth blocking at this time, imo.

[arifogel]

Reviewed 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved

[arifogel]

Reviewable status: complete! all files reviewed, all discussions resolved

---

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

Previously, progwriter (Victor Heorhiadi) wrote…

I find very nested loops are difficult to test, whereas a static func that takes several params is easier to reason about.
However, not worth blocking at this time, imo.

Oh you mean factor out the whole interior into a helper function?
Sure, but later. Not really in scope of this PR.

[codecov]

Codecov Report

Merging #5021 into master will increase coverage by 0.02%.
The diff coverage is 89.02%.

@@             Coverage Diff              @@
##             master    #5021      +/-   ##
============================================
+ Coverage     76.65%   76.68%   +0.02%     
- Complexity    30122    30194      +72     
============================================
  Files          2405     2411       +6     
  Lines        115971   116201     +230     
  Branches      13688    13710      +22     
============================================
+ Hits          88898    89103     +205     
- Misses        20345    20348       +3     
- Partials       6728     6750      +22

Impacted Files	Coverage Δ	Complexity Δ
...java/org/batfish/question/edges/EdgesAnswerer.java	82.6% <ø> (ø)	44 <0> (ø)	⬇️
.../ospfsession/OspfSessionCompatibilityAnswerer.java	73.56% <ø> (-0.6%)	12 <0> (ø)
...org/batfish/dataplane/ibdp/OspfRoutingProcess.java	88.76% <100%> (-0.08%)	165 <7> (ø)
...n/java/org/batfish/datamodel/ospf/OspfProcess.java	90.76% <100%> (ø)	51 <1> (ø)	⬇️
.../java/org/batfish/datamodel/ospf/OspfTopology.java	94.11% <100%> (+0.17%)	22 <0> (ø)	⬇️
.../org/batfish/datamodel/ospf/OspfTopologyUtils.java	85.62% <86%> (-1.63%)	39 <5> (+1)
...a/org/batfish/datamodel/NetworkConfigurations.java	78.46% <88.88%> (+1.26%)	30 <6> (+5)	⬆️
...g/batfish/datamodel/ospf/OspfNeighborConfigId.java	86.48% <88.88%> (-1.02%)	18 <3> (+2)
...batfish/representation/palo_alto/RuleEndpoint.java	56.52% <0%> (-18.48%)	4% <0%> (+1%)
...java/org/batfish/symbolic/state/EdgeStateExpr.java	73.91% <0%> (-4.35%)	10% <0%> (-1%)
... and 27 more