Skip to content

Commit bff3229

Browse files
zach-klippensteinzach-klippenstein
authored
Merge pull request #291 from square/ray/downgrade-kotlin
Revert "Upgrade kotlin and coroutines dependencies to latest versions."
2 parents 75b7168 + e1bf9a8 commit bff3229

File tree

13 files changed

+66
-111
lines changed

13 files changed

+66
-111
lines changed

Diff for: ‎kotlin/build.gradle

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ buildscript {
3535
'hamcrest': '1.3',
3636
'intellijAnnotations': '13.0',
3737
'junit': '4.12',
38-
'kotlin': '1.3.30',
39-
'kotlinCoroutines': '1.2.0',
38+
'kotlin': '1.3.21',
39+
'kotlinCoroutines': '1.1.1',
4040
'ktlintPlugin': '5.1.0',
4141
'mavenPublishPlugin': '0.6.0',
4242
'mockito': '2.7.5',

Diff for: ‎kotlin/legacy/legacy-workflow-core/src/main/java/com/squareup/workflow/legacy/WorkflowOperators.kt

+4-21
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
package com.squareup.workflow.legacy
1919

20-
import kotlinx.coroutines.CancellationException
2120
import kotlinx.coroutines.CoroutineScope
2221
import kotlinx.coroutines.Deferred
2322
import kotlinx.coroutines.Dispatchers.Unconfined
@@ -94,20 +93,7 @@ fun <S1 : Any, S2 : Any, E : Any, O : Any> Workflow<S1, E, O>.switchMapState(
9493
// leave the consumeEach loop but the produce coroutine will wait for this child coroutine
9594
// to complete (structured concurrency) before closing the downstream channel.
9695
transformerJob = launch {
97-
try {
98-
transform(upstreamState).toChannel(downstreamChannel)
99-
} catch (e: Throwable) {
100-
// Actual errors should be propagated, cancellation should interrupt forwarding
101-
// the current transformed channel but allow the next one to continue.
102-
// However, there may be multiple nested CancellationExceptions wrapping the actual
103-
// exception, so we have to go digging.
104-
val causeChain = generateSequence(e) { it.cause }
105-
causeChain.firstOrNull { it !is CancellationException }
106-
?.let { realException ->
107-
downstreamChannel.close(realException)
108-
throw realException
109-
}
110-
}
96+
transform(upstreamState).toChannel(downstreamChannel)
11197
}
11298
}
11399
}
@@ -128,13 +114,10 @@ fun <S : Any, E : Any, O1 : Any, O2 : Any> Workflow<S, E, O1>.mapResult(
128114

129115
// Propagate cancellation upstream.
130116
transformedResult.invokeOnCompletion { cause ->
131-
this@mapResult.cancel(
132-
if (cause is CancellationException) cause else CancellationException(null, cause)
133-
)
134-
if (cause is CancellationException) {
117+
if (cause != null) {
118+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
119+
@Suppress("DEPRECATION")
135120
this@mapResult.cancel(cause)
136-
} else if (cause != null) {
137-
this@mapResult.cancel(CancellationException(null, cause))
138121
}
139122
}
140123

Diff for: ‎kotlin/legacy/legacy-workflow-core/src/test/java/com/squareup/workflow/legacy/CoroutineWorkflowTest.kt

+4-11
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import kotlinx.coroutines.CancellationException
2121
import kotlinx.coroutines.CoroutineName
2222
import kotlinx.coroutines.CoroutineScope
2323
import kotlinx.coroutines.Dispatchers.Unconfined
24-
import kotlinx.coroutines.cancel
2524
import kotlinx.coroutines.channels.ReceiveChannel
2625
import kotlinx.coroutines.channels.consume
2726
import kotlinx.coroutines.suspendCancellableCoroutine
@@ -174,7 +173,7 @@ class CoroutineWorkflowTest : CoroutineScope {
174173
workflow.sendEvent(Unit)
175174
}
176175

177-
@Test fun `block gets original cancellation reason - null cause`() {
176+
@Test fun `block gets original cancellation reason`() {
178177
lateinit var cancelReason: Throwable
179178
val workflow = workflow<Nothing, Nothing, Nothing> { _, _ ->
180179
suspendCancellableCoroutine<Nothing> { continuation ->
@@ -191,7 +190,7 @@ class CoroutineWorkflowTest : CoroutineScope {
191190
}
192191

193192
@Suppress("DEPRECATION")
194-
@Test fun `block gets original cancellation reason - non-null cause`() {
193+
@Test fun `block gets original cancellation reason - deprecated cancel`() {
195194
lateinit var cancelReason: Throwable
196195
val workflow = workflow<Nothing, Nothing, Nothing> { _, _ ->
197196
suspendCancellableCoroutine<Nothing> { continuation ->
@@ -204,16 +203,10 @@ class CoroutineWorkflowTest : CoroutineScope {
204203
workflow.cancel(ExpectedException)
205204

206205
assertTrue(cancelReason is CancellationException)
207-
// Search up the cause chain for the expected exception, since multiple CancellationExceptions
208-
// may be chained together first.
209-
val causeChain = generateSequence<Throwable>(cancelReason) { it.cause }
210-
assertEquals(
211-
1, causeChain.count { it === ExpectedException },
212-
"Expected cancellation exception cause chain to include ExpectedException."
213-
)
206+
assertEquals(ExpectedException, cancelReason.cause)
214207
}
215208

216209
private companion object {
217-
object ExpectedException : CancellationException()
210+
object ExpectedException : RuntimeException()
218211
}
219212
}

Diff for: ‎kotlin/legacy/legacy-workflow-core/src/test/java/com/squareup/workflow/legacy/WorkflowOperatorsTest.kt

+20-28
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,10 @@ import kotlinx.coroutines.channels.Channel
2424
import kotlinx.coroutines.channels.consume
2525
import kotlinx.coroutines.channels.produce
2626
import kotlinx.coroutines.isActive
27-
import kotlinx.coroutines.runBlocking
2827
import kotlinx.coroutines.suspendCancellableCoroutine
2928
import kotlin.test.BeforeTest
3029
import kotlin.test.Test
3130
import kotlin.test.assertEquals
32-
import kotlin.test.assertFails
3331
import kotlin.test.assertFailsWith
3432
import kotlin.test.assertFalse
3533
import kotlin.test.assertNull
@@ -83,7 +81,9 @@ class WorkflowOperatorsTest {
8381
it.toString()
8482
}
8583

86-
withAdaptedEvents.cancel(CancellationException(null, ExpectedException()))
84+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
85+
@Suppress("DEPRECATION")
86+
withAdaptedEvents.cancel(ExpectedException())
8787

8888
assertTrue(sourceCancellation is CancellationException)
8989
}
@@ -161,7 +161,9 @@ class WorkflowOperatorsTest {
161161
val withMappedStates = source.mapState { it }
162162

163163
assertNull(sourceCancellation)
164-
withMappedStates.cancel(CancellationException(null, ExpectedException()))
164+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
165+
@Suppress("DEPRECATION")
166+
withMappedStates.cancel(ExpectedException())
165167
assertTrue(sourceCancellation is CancellationException)
166168
}
167169

@@ -306,16 +308,7 @@ class WorkflowOperatorsTest {
306308
.consume {
307309
assertFalse(withMappedStates.isCompleted)
308310
withMappedStates.sendEvent(Unit)
309-
assertFails { runBlocking { receive() } }
310-
.also { error ->
311-
// Search up the cause chain for the expected exception, since multiple CancellationExceptions
312-
// may be chained together first.
313-
val causeChain = generateSequence(error) { it.cause }
314-
assertEquals(
315-
1, causeChain.count { it is ExpectedException },
316-
"Expected cancellation exception cause chain to include ExpectedException."
317-
)
318-
}
311+
assertFailsWith<ExpectedException> { poll() }
319312
// Exception is not sent through the result.
320313
assertEquals(Unit, withMappedStates.getCompleted())
321314
}
@@ -328,24 +321,17 @@ class WorkflowOperatorsTest {
328321
}
329322
val withMappedStates: Workflow<Int, Unit, Unit> = source.switchMapState {
330323
Channel<Int>().apply {
331-
cancel(CancellationException(null, ExpectedException()))
324+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
325+
@Suppress("DEPRECATION")
326+
cancel(ExpectedException())
332327
}
333328
}
334329

335330
withMappedStates.openSubscriptionToState()
336331
.consume {
337332
assertFalse(withMappedStates.isCompleted)
338333
withMappedStates.sendEvent(Unit)
339-
assertFails { runBlocking { receive() } }
340-
.also { error ->
341-
// Search up the cause chain for the expected exception, since multiple CancellationExceptions
342-
// may be chained together first.
343-
val causeChain = generateSequence(error) { it.cause }
344-
assertEquals(
345-
1, causeChain.count { it is ExpectedException },
346-
"Expected cancellation exception cause chain to include ExpectedException."
347-
)
348-
}
334+
assertFailsWith<ExpectedException> { poll() }
349335
// Exception is not sent through the result.
350336
assertEquals(Unit, withMappedStates.getCompleted())
351337
}
@@ -385,7 +371,9 @@ class WorkflowOperatorsTest {
385371
withMappedStates.sendEvent(Unit)
386372

387373
assertFalse(transformedChannel.isClosedForSend)
388-
this.cancel(CancellationException(null, ExpectedException()))
374+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
375+
@Suppress("DEPRECATION")
376+
this.cancel(ExpectedException())
389377
assertTrue(transformedChannel.isClosedForSend)
390378
}
391379
}
@@ -402,7 +390,9 @@ class WorkflowOperatorsTest {
402390
val withMappedStates = source.switchMapState { produce { send(it) } }
403391

404392
assertNull(sourceCancellation)
405-
withMappedStates.cancel(CancellationException(null, ExpectedException()))
393+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
394+
@Suppress("DEPRECATION")
395+
withMappedStates.cancel(ExpectedException())
406396
assertTrue(sourceCancellation is CancellationException)
407397
}
408398

@@ -460,7 +450,9 @@ class WorkflowOperatorsTest {
460450
val withMappedResult = source.mapResult { it }
461451

462452
assertNull(sourceCancellation)
463-
withMappedResult.cancel(CancellationException(null, ExpectedException()))
453+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
454+
@Suppress("DEPRECATION")
455+
withMappedResult.cancel(ExpectedException())
464456
assertTrue(sourceCancellation is CancellationException)
465457
}
466458
}

Diff for: ‎kotlin/legacy/legacy-workflow-rx2/src/main/java/com/squareup/workflow/legacy/rx2/EventChannel.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ fun <E : Any> ReceiveChannel<E>.asEventChannel() = object : EventChannel<E> {
123123
.also { selectionJob.cancel() }
124124
} catch (cancellation: CancellationException) {
125125
val cause = cancellation.cause
126-
if (cause == null || cause is CancellationException) {
126+
if (cause == null) {
127127
// The select was cancelled normally, which means the workflow was abandoned and we're
128128
// about to get unsubscribed from. Don't propagate the error, just never emit/return.
129129
suspendCoroutine<Nothing> { }

Diff for: ‎kotlin/legacy/legacy-workflow-rx2/src/test/java/com/squareup/workflow/legacy/rx2/WorkflowOperatorsTest.kt

+3-1
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ class WorkflowOperatorsTest {
7878
states.cancel()
7979

8080
assertThat(subscribeCount).isEqualTo(1)
81-
assertThat(disposeCount).isEqualTo(1)
81+
// For some reason the observable is actually disposed twice. Seems like a coroutines bug, but
82+
// Disposable.dispose() is an idempotent operation so it should be fine.
83+
assertThat(disposeCount).isEqualTo(2)
8284
}
8385
}

Diff for: ‎kotlin/samples/tictactoe/android/build.gradle

-5
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,6 @@ android {
2727
versionCode 1
2828
versionName "1.0.0"
2929
}
30-
31-
// See https://github.com/Kotlin/kotlinx.coroutines/issues/1064#issuecomment-479412940
32-
packagingOptions {
33-
exclude 'META-INF/atomicfu.kotlin_module'
34-
}
3530
}
3631

3732
dependencies {

Diff for: ‎kotlin/workflow-runtime/src/main/java/com/squareup/workflow/WorkflowHost.kt

+3-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import com.squareup.workflow.WorkflowHost.Update
2222
import com.squareup.workflow.internal.WorkflowId
2323
import com.squareup.workflow.internal.WorkflowNode
2424
import com.squareup.workflow.internal.id
25-
import kotlinx.coroutines.CancellationException
2625
import kotlinx.coroutines.cancel
2726
import kotlinx.coroutines.channels.ReceiveChannel
2827
import kotlinx.coroutines.channels.produce
@@ -150,7 +149,9 @@ internal fun <I : Any, O : Any, R : Any> WorkflowNode<I, *, O, R>.start(
150149
} catch (e: Throwable) {
151150
// For some reason the exception gets masked if we don't explicitly pass it to cancel the
152151
// producer coroutine ourselves here.
153-
coroutineContext.cancel(if (e is CancellationException) e else CancellationException(null, e))
152+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
153+
@Suppress("DEPRECATION")
154+
coroutineContext.cancel(e)
154155
throw e
155156
} finally {
156157
// There's a potential race condition if the producer coroutine is cancelled before it has a chance

Diff for: ‎kotlin/workflow-runtime/src/test/java/com/squareup/workflow/internal/ChannelUpdatesTest.kt

+4-11
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ package com.squareup.workflow.internal
2020
import com.squareup.workflow.util.ChannelUpdate
2121
import com.squareup.workflow.util.ChannelUpdate.Closed
2222
import com.squareup.workflow.util.ChannelUpdate.Value
23-
import kotlinx.coroutines.CancellationException
2423
import kotlinx.coroutines.Dispatchers
2524
import kotlinx.coroutines.GlobalScope
2625
import kotlinx.coroutines.async
@@ -83,22 +82,16 @@ class ChannelUpdatesTest {
8382

8483
@Test fun `handles error`() {
8584
val channel = Channel<String>()
86-
channel.cancel(CancellationException(null, ExpectedException()))
85+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
86+
@Suppress("DEPRECATION")
87+
channel.cancel(ExpectedException())
8788

88-
assertFailsWith<CancellationException> {
89+
assertFailsWith<ExpectedException> {
8990
runBlocking {
9091
select<ChannelUpdate<String>> {
9192
onChannelUpdate(channel) { it }
9293
}
9394
}
94-
}.also { error ->
95-
// Search up the cause chain for the expected exception, since multiple CancellationExceptions
96-
// may be chained together first.
97-
val causeChain = generateSequence<Throwable>(error) { it.cause }
98-
assertEquals(
99-
1, causeChain.count { it is ExpectedException },
100-
"Expected cancellation exception cause chain to include original cause."
101-
)
10295
}
10396
}
10497

Diff for: ‎kotlin/workflow-rx2/src/test/java/com/squareup/workflow/rx2/SubscriptionsTest.kt

+10-4
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ class SubscriptionsTest {
103103
}
104104

105105
assertEquals(1, workflow.subscriptions)
106-
assertEquals(1, workflow.disposals)
106+
// For some reason the observable is actually disposed twice. Seems like a coroutines bug, but
107+
// Disposable.dispose() is an idempotent operation so it should be fine.
108+
assertEquals(2, workflow.disposals)
107109
}
108110
}
109111

@@ -116,7 +118,9 @@ class SubscriptionsTest {
116118
}
117119

118120
assertEquals(1, workflow.subscriptions)
119-
assertEquals(1, workflow.disposals)
121+
// For some reason the observable is actually disposed twice. Seems like a coroutines bug, but
122+
// Disposable.dispose() is an idempotent operation so it should be fine.
123+
assertEquals(2, workflow.disposals)
120124
}
121125
}
122126

@@ -146,13 +150,15 @@ class SubscriptionsTest {
146150

147151
host.withNextRendering { setSubscribed ->
148152
assertEquals(1, workflow.subscriptions)
149-
assertEquals(1, workflow.disposals)
153+
// For some reason the observable is actually disposed twice. Seems like a coroutines bug,
154+
// but Disposable.dispose() is an idempotent operation so it should be fine.
155+
assertEquals(2, workflow.disposals)
150156
setSubscribed(true)
151157
}
152158

153159
host.withNextRendering {
154160
assertEquals(2, workflow.subscriptions)
155-
assertEquals(1, workflow.disposals)
161+
assertEquals(2, workflow.disposals)
156162
}
157163
}
158164
}

Diff for: ‎kotlin/workflow-testing/src/main/java/com/squareup/workflow/testing/WorkflowTesting.kt

+11-9
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import com.squareup.workflow.Snapshot
2121
import com.squareup.workflow.StatefulWorkflow
2222
import com.squareup.workflow.Workflow
2323
import com.squareup.workflow.WorkflowHost
24-
import kotlinx.coroutines.CancellationException
24+
import com.squareup.workflow.WorkflowHost.Factory
2525
import kotlinx.coroutines.Dispatchers
2626
import kotlinx.coroutines.InternalCoroutinesApi
2727
import kotlinx.coroutines.Job
@@ -97,7 +97,7 @@ fun <StateT : Any, OutputT : Any, RenderingT : Any>
9797
private fun <T, O : Any, R : Any> test(
9898
testBlock: (WorkflowTester<O, R>) -> T,
9999
baseContext: CoroutineContext,
100-
starter: (WorkflowHost.Factory) -> WorkflowHost<O, R>
100+
starter: (Factory) -> WorkflowHost<O, R>
101101
): T {
102102
val context = Dispatchers.Unconfined + baseContext + Job(parent = baseContext[Job])
103103
val host = WorkflowHost.Factory(context)
@@ -112,13 +112,15 @@ private fun <T, O : Any, R : Any> test(
112112
throw e
113113
} finally {
114114
if (error != null) {
115-
context.cancel(
116-
if (error is CancellationException) error else CancellationException(null, error)
117-
)
118-
val cancellationCause = context[Job]!!.getCancellationException()
119-
.cause
120-
if (cancellationCause != error && cancellationCause != null) {
121-
error.addSuppressed(cancellationCause)
115+
// TODO https://github.com/square/workflow/issues/188 Stop using parameterized cancel.
116+
@Suppress("DEPRECATION")
117+
val cancelled = context.cancel(error)
118+
if (!cancelled) {
119+
val cancellationCause = context[Job]!!.getCancellationException()
120+
.cause
121+
if (cancellationCause != error && cancellationCause != null) {
122+
error.addSuppressed(cancellationCause)
123+
}
122124
}
123125
} else {
124126
// Cancel the Job to ensure everything gets cleaned up.

0 commit comments

Comments
 (0)