API improvements for ServerEnvironment

[dmitrykuzmin]

1. ServerEnvironment.use(...) should now only be usable in a context like

   ServerEnvironment.when(SomeEnv.class)
                    .use(envType -> storageFactory())
                    .use(envType -> delivery()) // ...

   That is, the use method won't require EnvironmentType as a second argument anymore (superseded by when(...)), and will accept lazily-initialized values instead of plain objects. The envType is the active environment type supplied for logging or other purposes.

   The old use(T, EnvironmentType) API should be deprecated.

2. ServerEnvironment should supply a new public method type() which will return the currently active EnvironmentType.

   This way, the user code will be able to easily access the info about the currently active environment when the ServerEnvironment is already configured.

3. @Internal-ize the Environment.register(...) and improve EnvironmentType documentation.
   It is not immediately apparent what exactly to do with a custom EnvironmentType once you've created it.
   For example, with the current implementation, it's possible to employ Environment.register(EnvironmentType) to register a custom env type which actually accepts some dependencies on creation which help to deduce whether it's enabled or not. And then it would be logical to check for such env presence with when(...), but this would throw an exception.

[alexander-yevsyukov]

@armiol @dmitrykuzmin how about having it shorter for the configuration phase?

ServerEnvironment.when(SomeEnv.class)
                 .use(envType -> storageFactory())
                 .use(envType -> delivery()) // ...

[dmitrykuzmin]

I personally support this change.

[armiol]

@alexander-yevsyukov LGTM.

[alexander-yevsyukov]

@armiol @dmitrykuzmin

Because of limitation of Java generics, we cannot have multiple use(Function<T, R>) at the same level. So, the following is not possible:

ServerEnvironment.when(SomeEnv.class)
                 .use(envType -> storageFactory())
                 .use(envType -> delivery())  // ...

We can have:

ServerEnvironment.when(SomeEnv.class)
                 .useStorageFactory(envType -> storageFactory())
                 .useDelivery(envType -> delivery())  // ...

Or, abandon the idea of passing functions to the use() API at all.

Thoughts?

[alexander-yevsyukov]

Also we can have use() API with typed lambdas, but because of that we'd have to require users to implement configuration functions returning those typed lambdas. E.g.

private ServerEnvironment.DeliveryFn deliveryFn(Delivery valueFromFunction) {
    return t -> valueFromFunction;
}

or cast it like this:

when(currentType).use((ServerEnvironment.DeliveryFn) t -> valueFromFunction);

This is non-trivial, to say the least, and makes things much more harder to understand.

[alexander-yevsyukov]

Summing up, I'll add useThis(), useThat() family of methods that would accept Fn<This> and Fn<That>.

[alexander-yevsyukov]

@dmitrykuzmin, we need to expose Environment.register(Class) because it is linked to the way Environment.is(Class) used and documented in the API. The fact that ServerEnvironment does the registration automatically for us is a happy coincidence.

I'm going to improve the documentation so that it's easier to understand and give it to you for review later, hopefully, this week.

[alexander-yevsyukov]

As discussed with @armiol, the following construct is problematic:

ServerEnvironment.when(Staging.class).run((envType) -> configureStaging())
 	         .when(Production.class).run((envType) -> configureProduction());

First of all, it looks as if the code under run() is going to be executed when the environment changes its type. This is so because what we specify in the combination of when().use() is going to be given when it's actually needed and if the type is what we put in when().

The original intent was to execute the function under run() only once, if when() is “true”, and not every time when the environment hypothetically turns into the given state once again.

To avoid the confusion, we tried to rename when() to ifIs(), and run to then(), having this:

ServerEnvironment.ifIs(Staging.class).then((envType) -> configureStaging())
 	         .ifIs(Production.class).then((envType) -> configureProduction());

This construct is worse than combination of two if(). It's harder to read and in overall looks like as over-engineering.

In conclusion, we're not doing the item no.3 (when()/run()) from the original issue request because the same effect can be achieved by if() clauses.