GEODE-9897: Improve ClientUserAuthsTest

[demery-pivotal]

PROBLEM 1

In ClientUserAuthsTest, the before() method uses the spy()
mechanism to override getNextId() on the ClientUserAuths instance it
is testing. This bypasses the actual implementation for all tests,
leaving the following important ClientUserAuths responsibilities
untested:

• Detect when all IDs have all been used.
• Clear all existing authorizations when all IDs have been used, to
  force reauthentication.
• Re-seed the ID generator when all IDs have been used.

The tests are likely doing this because getNextId() relies on a
Random, which is uncontrollable by design. Because ClientUserAuths
creates the Random, the tests are unable to inject a testable
instance, and so instead must bypass the ClientUserAuths methods that
interact with the Random.

PROBLEM 2

Some tests attempt to verify ClientUserAuths responsibilities only by
observing how a ClientUserAuths interacts with itself. These tests
verify only implementation details, and leave the actual responsibility
untested.

PROBLEM 3

ClientUserAuths.putSubject(subject, id) treats id == 0 as a request
to generate and assign an ID. The getNextID() method can generate ID
0, and so putSubject() might associate the subject with ID 0. If a
caller later attempts to replace the subject by calling
putSubject(replacement, 0), putSubject() will generate a new ID and
assign it to the replacement, rather than replacing the original
subject.

SOLUTION

1. (The first commit in this PR.) Extract and test a RandomSubjectIdGenerator that generates random IDs.
2. (The second commit in this PR.) Change existing tests so that they do not override production behavior, and so that they verify actual responsibilities. Update getNextID() to not generate ID 0.
3. (Subsequent PR.) Add tests for the now-testable responsibilities.

[jchen21]

If you would, you can refer to IDS_EXHAUSTED.

[demery-pivotal]

I think I'm going to leave this as -1 (but see below for an alternative). The reason is that there's already a weird, subtle coupling between the -1 here (where it means "IDs exhausted") and the -1 in putSubject() (where it means "please generate an ID").

I say "weird" because, though -1 has a different meaning in each place, both places must use the same value. That's weird.

If I change this one to IDS_EXHAUSTED, that hides the fact that we must use -1 in both places… which makes it no less weird, but even more subtle.

I'm not happy with the weirdness, the subtlety, or the coupling. But given that there's coupling and it's weird, I'm reluctant make it more subtle.

I considered making generateId() return an OptionalLong, which would eliminate the need for the generator to treat -1 as special in any way. I decided not to… out of a perhaps unfounded fear of allocating the OptionalLong on the heap. Should I reconsider that?

[jchen21]

What if idGenerator.generateId() return 0 again? Chances are very slim though.

[demery-pivotal]

The contract for an ID generator is to return unique IDs. If it can't do that, it must return -1 to indicate that it has exhausted all unique IDs. So a valid ID generator never returns the same ID twice (0 or otherwise) without first resetting itself and returning -1 to report ID exhaustion.

This contract is currently described only on the implementation of RandomSubjectIdGenerator. I will add it to the SubjectIdGenerator interface.

[jchen21]

What if idGenerator.generateId() return -1 again? Chances are very slim though.

[demery-pivotal]

The "contract" for SubjectIdGenerator is that it returns -1 only if it exhausts all available IDs. So the only way it will return -1 twice in a row only if it cannot generate any IDs in between the -1s.

I put "contract" in scare quotes because it's not written down anywhere. I should probably add a comment on SubjectIdGenerator describing the contract.

[demery-pivotal]

Based on @jchen21's comments, I'm going to re-implement the generator to return OptionalLong and to use OptionalLong.empty() to indicate "no more available unique IDs."