Removed freezing for RIBs

[progwriter]

Now RIBs keep an up-to-date collection of all routes. Fixes #646

---

This change is 

[dhalperi]

What is the expected semantics of the return value here?

Questions:

1. Should clients be able to modify the return value?
2. Should clients see updated values if this AbstractRib is modified later?

[progwriter]

1. They should not.
2. Good question. I believe the answer here is also no since we use getRoutes to iterate over a particular snapshot of the RIB.

[dhalperi]

Review status: 0 of 4 files reviewed at latest revision, 1 unresolved discussion.

---

projects/batfish/src/main/java/org/batfish/bdp/AbstractRib.java, line 443 at r1 (raw file):

Previously, progwriter (Victor Heorhiadi) wrote…

1. They should not.
2. Good question. I believe the answer here is also no since we use getRoutes to iterate over a particular snapshot of the RIB.

So it sounds like we either need to return a Collections.unmodifiableList or an ImmutableList.

Or figure out the combination of memoizing + access pattern that makes this work out?

---

Comments from Reviewable

[dhalperi]

why not ImmutableSet.of();?

[dhalperi]

Minor comments, but

---

Reviewed 3 of 4 files at r1, 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 4 unresolved discussions, some commit checks failed.

---

projects/batfish/src/main/java/org/batfish/bdp/AbstractRib.java, line 213 at r2 (raw file):

    /**
     * Add the route to the set of routes at this node

I feel like this function adds questionable value over the 5 places it was used, but okay :)

---

projects/batfish/src/test/java/org/batfish/bdp/AbstractRibTest.java, line 212 at r2 (raw file):

    // Add completely new route and check that the size increases
    rib.mergeRoute(new OspfIntraAreaRoute(new Prefix("2.2.2.2/32"), null, 100, 30, 1));
    assertThat(rib.getRoutes(), hasSize(2));

We should test the invariants we agreed upon in the PR discussion.

1. if I get routes, I can't modify the result.
2. if I get routes, then modify the RIB, the routes I previously got did not change.

and probably also

3. if I get routes, then don't modify and get again, I should get actually the same object. (assertThat(rib.getRoutes(), is(prev)) for example).

(each in its own unit test).

---

Comments from Reviewable

[dhalperi]

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[dhalperi]

I played with this last night and it seemed to shave something like a minute (out of 6.5 mins) off of building a real data plane. Nice.

---

Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

Comments from Reviewable

[dhalperi]

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

---

Comments from Reviewable