core: Validate policy name in defaultLoadBalancingPolicy() immediately

[manmohak07]

Fixes #12695

Move the load-balancing policy name validation from the build phase
to the defaultLoadBalancingPolicy() setter itself, so that an
invalid policy name throws IllegalArgumentException immediately
at the call site.

Problem

Currently, calling .defaultLoadBalancingPolicy("round_robinn") (typo)
stores the bad string. The error only surfaces later during
channel construction when AutoConfiguredLoadBalancerFactory creates a
FixedPickerLoadBalancerProvider with TRANSIENT_FAILURE status.

Solution

The defaultLoadBalancingPolicy() setter in ManagedChannelImplBuilder
now validates the provided policy name against
LoadBalancerRegistry.getDefaultRegistry() immediately. If no provider
is found, it throws IllegalArgumentException with a descriptive
message.

Changes

• ManagedChannelImplBuilder.java: Add registry lookup and
  validation in defaultLoadBalancingPolicy()
• ManagedChannelBuilder.java: Update Javadoc to document the
  new IllegalArgumentException behavior
• ManagedChannelImplBuilderTest.java: Register a fake
  LoadBalancerProvider for test isolation and add test for unregistered
  policy name

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: manmohak07 / name: Mohak Agrawal (439218a, 72c8cbb, f6b9459)

[manmohak07]

1. In ManagedChannelBuilder.java I think if you just pass in LoadBalancerRegistry.getDefaultRegistry().getProvider(policy) to the Preconditions check, that will work. If not, make the variable final. You can also use Preconditions.checkNotNull. While it's still correct, provider.getPolicyName() will give the same result as policy assuming the policy was already registered (when setting the default).
2. If you are going to use FAKE_POLICY_NAME then it may be beneficial to use it everywhere "magic_balancer" is in the test file.
3. I believe the test you wrote is correct, however, it doesn't actually use the rest of the code you wrote; you wrote the test with provider name "unregistered_balancer" rather than "magic_balancer".
4. It may be possible to use 3 lines to set up the mock (use the mock statement, then two doReturn statements for the mock), while keeping the registration and deregistration in the same normal method. The way you have it right now will create the mock for each of the tests rather than for the required normal test.

Hi, thanks for the input. With respect to what you said about the tests, I think we can make these changes:

• Remove FAKE_POLICY_NAME and fakeProvider.

@Test
  public void defaultLoadBalancingPolicy_normal() {
    LoadBalancerProvider mockProvider = mock(LoadBalancerProvider.class);
    doReturn(true).when(mockProvider).isAvailable();
    doReturn("magic_balancer").when(mockProvider).getPolicyName();
    LoadBalancerRegistry.getDefaultRegistry().register(mockProvider);
    try {
      assertSame(builder, builder.defaultLoadBalancingPolicy("magic_balancer"));
      assertEquals("magic_balancer", builder.defaultLbPolicy);
    } finally {
      LoadBalancerRegistry.getDefaultRegistry().deregister(mockProvider);
    }
  }

• Use "magic_balancer" directly.

@Test
  public void defaultLoadBalancingPolicy_unregisteredPolicy() {
    assertThrows(
        IllegalArgumentException.class,
        () -> builder.defaultLoadBalancingPolicy("magic_balancer")
    );
  }

I am not sure about the first point you mentioned. @shivaspeaks Could you please provide suggestions on the existing changes and new ones mentioned here? It would help a lot.