Jan 2017 dependency upgrade

[mikehearn]

Upgrade dependencies and centralise some more version numbers in the root gradle file.

Three dependencies had to be held back:

• Exposed: API changes.
• Quasar: new release appears to have a bug that causes incorrect validation failures. I raised it with the Quasar guys.
• Gradle: the issue with the Kotlin 1.1 vs 1.0 stdlib clashes is apparently being worked around by JetBrains and there will be a fix when Kotlin 1.1 final is released. So we just wait, here.

Not assigning a reviewer yet - waiting to see what CI makes of it as I didn't bother to run the integration tests locally.

[adagys]

AFAIK the pull request CI build only runs unit tests :)

[mikehearn]

It seems to have run some integration tests too, one of which revealed an Rx related problem. But I can't tell yet if it's a problem with the upgrade or a bug in our code. Investigating.

[rick-r3]

Mike, shall I take a look at what happened in Exposed and tackle it? I'm probably going to regret it, but...

[mikehearn]

You can if you like - depends how much you value being on the latest version of Exposed. It's cleanup week after all. But if it's a big change don't worry about it.

[mikehearn]

The CI failure appears to be an unrelated flake, I can't reproduce it but from the logs I got an idea of what might be going on and have a fix in another branch for it.

[mikehearn]

@rick-r3 Can you review this please?

[rick-r3]

Looks good but seems to hang on some RX stuff in CI. Looking at the exception, I'm guessing there's an expectation of back pressure implementation been added somewhere that impacts the database wrapper.

[rick-r3]

Good once CI is passing.

[shamsasari]

Can you also move the assertj dependency to be a compile one in test-utils. Last time I checked assertj was always present where test-utils was present.

[mikehearn]

Yes, it's this change: ReactiveX/RxJava#4225

It's a race of some kind. I can't always reproduce it.

Looking at the code it seems like an easy fix in this case, but we use PublishSubject in quite a few places and it seems like RxJava broke backwards compatibility pretty badly here: my reading is that attempting to emit on the subject when there are no observers will now always fail whereas before it would succeed just fine. That's a very big API break!

I am going to take out the Rx upgrade. It seems clear that they have changed the API in major ways that don't cause compile errors but can cause subtle failures at runtime. I think not just PublishSubject changed. So to upgrade we are going to have to understand what they've done (I didn't see any mention of this in the release notes even though they knew it'd break code) and then audit our usages to figure it out. I'll file a task in JIRA to do this, but I don't think we have any real reason to upgrade right now.

[mikehearn]

My bad - I was trying to read the upstream change from the diff and got confused. You can still emit on a subject with no subscribers. As the problem is intermittent, it will require some more debugging to find out where the missing requests are. I've still left the Rx upgrade out, we can handle it in a different PR.

[akarnokd]

There is another effect of the PublishSubject change, namely when you call onNext concurrently on it - which is forbidden by the Observer contract - it can mess up the internal state. Previously PublishSubject was write-through so a concurrent onNext's effect would materialize in the subsequent operator or consumer. A search lists 17 places where you use PublishSubject; I'd apply .toSerialized() on all places and check if the problem persist.

[mikehearn]

Thanks! That's pretty amazing support! :) 🍺

I think the issue is that we have a class called DatabaseTransactionWrappingSubscriber which groups subscribers together and then passes through events by itself, after wrapping the whole run in a database transaction. That class doesn't propagate backpressure. We're discussing the issue on Slack. I'm not sure why the problem is intermittent. In this case the observables really shouldn't be being accessed from multiple threads anyway; I have a separate change to fix that.