Add config store

[kvosper]

Cherry-picked from previous branch to styx-1.0-dev

[mikkokar]

CyclicBarrier seems more appropriate for this use case.

[kvosper]

I don't know what difference that would make.

[mikkokar]

It would be using only one synchronising primitive instead of two. That cuts the complexity by 50% ;-).

[mikkokar]

I think this is conceptually much simpler:

    public void listensOnSeparateThread2() {
        final CyclicBarrier barrier = new CyclicBarrier(2);

        configStore.watch("foo", String.class)
                .subscribe(value -> {
                        await(barrier);
                });

        configStore.set("foo", "bar");
        await(barrier);
    }

[kvosper]

OK.

[mikkokar]

As per our unwritten naming conventions, utility classes names like this would have their name pluralised.

That is, the class name should be Latches.

[kvosper]

It's not a utility class. Utility classes have only static methods. This is a normal class.

[mikkokar]

Right of course. Now I see that. I should have seen that straight away.

But why to implement these await methods as a composed class?

By making it a public test support class you are promoting its use in other tests. But then again the Latch class is not type compatible with CountDownLatch. Sooner or later some parts of the codebase will be using Latch while other parts stick to CountDownLatch. After that we will need to bridge the two classes at the interface level.

But all we really want is to re-propagate checked exceptions as runtime exceptions in await method.

For these reasons, please convert this to a utility class with await as a static utility method.

[kvosper]

I'll look into this.

[mikkokar]

Right of course. Now I see that. I should have seen that straight away.

But why to implement these await methods as a composed class?

By making it a public test support class you are promoting its use in other tests. But then again the Latch class is not type compatible with CountDownLatch. Sooner or later some parts of the codebase will be using Latch while other parts stick to CountDownLatch. After that we will need to bridge the two classes at the interface level.

But all we really want is to re-propagate checked exceptions as runtime exceptions in await method.

For these reasons, please convert this to a utility class with await as a static utility method.

[mikkokar]

Surely we need to re-throw something from the InterruptedException case? Otherwise await just returns and the calling code carries on as if nothing happened.

[dvlato]

Sure I failed to notice that we had stopped throwing an Exception here. My advice is that we just let InterruptedException propagate...

[kvosper]

Unfortunately, Observable.subscribe takes an Action1 as its argument, which will not allow InterruptedException to be thrown.

[dvlato]

We could have a method that receives (and returns) a lambda and catches/propagates the exception instead of this.

[mikkokar]

I think the problem is with the lambda expressions. Without this utility you would be forced to try/catch inside lambda expressions.

[kvosper]

As Mikko says, the problem is that InterruptedException is a checked exception, and the interfaces our lambdas are implementing do not allow checked exceptions to be thrown.

[dvlato]

Oh, sure. I think some of the code inside the tests uses the Latch outside a lambda.

Then my suggestion of using a method that receives a lambda (with InterruptedException in the signature) and propagates InterruptedException as a RuntimeException (but keeping the Thread.interrupt() ) makes more sense than wrapping specific classes.

[mikkokar]

Hi @dvlato. It no longer wraps a class. It is just a static helper method that accepts a CountDownLatch as an argument. IMO it should just set the interrupted flag and propagate an InterruptedException wrapped inside a RuntimeException.

[dvlato]

Sure I failed to notice that we had stopped throwing an Exception here. My advice is that we just let InterruptedException propagate...