Feat: Network Rule #7

canonall · 2023-03-21T08:37:36Z

The network rule requires two parameters: NetworkState and networkTimeoutDuration. The NetworkState parameter specifies the desired state of the network that needs to be satisfied, while the networkTimeoutDuration parameter specifies the duration to observe changes in the network state.

The network rule follows the flow outlined below:

If the network state is not satisfied during networkTimeoutDuration:
- NetworkRuleTimeout will be emitted.
- DidCancel will be emitted.
If the network state is satisfied during networkTimeoutDuration:
- NetworkRuleSatisfied will be emitted and NetworkRuleTimeout@ scope will be cancelled.
- WillRun will be emitted.
- If the job is completed within jobTimeout:
  - result will be emitted which can be one of three events: DidSucceed, DidFail, DidFailOnRemove
  - Otherwise JobTimeout and DidCancel will be emitted.

Changes in the code:

define network states (WIFI, MOBILE, NONE)
observe changes in the network state
move withTimeout() to JobQueue
add more tests
some clean-up

…are already added

- use StateFlow - take NetworkManager as parameter

…kRuleTimeout

benjohnde

I like the implementation overall @canonall, well done!

benjohnde · 2023-03-22T14:51:25Z

src/androidMain/kotlin/com/liftric/job/queue/JobQueue.kt

-    serializers,
-    configuration,
-    store
+    serializers = serializers,


I like the use of named arguments.

benjohnde · 2023-03-22T14:55:43Z

src/commonMain/kotlin/com/liftric/job/queue/Job.kt


-    constructor(task: Task, info: JobInfo) : this (UUIDFactory.create(), info, task, Clock.System.now())
+    constructor(task: Task, info: JobInfo) : this(
+        UUIDFactory.create(),


Not that important, you could also use named arguments here :)

benjohnde · 2023-03-22T14:59:22Z

src/commonMain/kotlin/com/liftric/job/queue/JobQueue.kt

-                        listener.emit(result)
+                        var shouldRunJob = false
+                        try {
+                            withTimeout(job.info.networkRuleTimeout) NetworkRuleTimeout@{


Just curious why the withTimeout, my understanding is that NetworkListener.currentNetworkState gives instant feedback.

withTimeout(job.info.networkRuleTimeout) NetworkRuleTimeout@{ networkListener.currentNetworkState.collect { currentNetworkState -> val isNetworkRuleSatisfied = networkListener.isNetworkRuleSatisfied( jobInfo = job.info, currentNetworkState = currentNetworkState ) if (isNetworkRuleSatisfied) { shouldRunJob = true jobEventListener.emit(JobEvent.NetworkRuleSatisfied(job)) [email protected]("Network rule satisfied") } } }

The aim of using the withTimeout function was to ensure that the network requirement is met within a specified time frame. With each emission of the currentNetworkState, the 'isNetworkRuleSatisfied' flag is checked, instantly. If the flag is true, the NetworkRuleTimeout scope is canceled. If the flag is false, the currentNetworkState continues to be observed until the timeout expires. Without withTimeout the job would never be canceled until the network requirement is met, which I thought not desirable.

benjohnde · 2023-03-22T15:02:38Z

src/androidMain/kotlin/com/liftric/job/queue/NetworkListener.kt

+            networkManager
+                .observeNetworkConnection()
+                .collect { currentNetworkState ->
+                    _currentNetworkState.emit(


If I did not misunderstand your implementation, you could also dismiss the when block and directly write:

_currentNetworkState.emit(currentNetworkState ?: NetworkState.NONE)

benjohnde · 2023-03-22T15:02:50Z

src/iosMain/kotlin/com/liftric/job/queue/NetworkListener.kt

+        networkManager.startMonitoring()
+        scope.launch {
+            networkManager.network.collectLatest { currentNetworkState ->
+                _currentNetworkState.emit(


If I did not misunderstand your implementation, you could also dismiss the when block and directly write:

_currentNetworkState.emit(currentNetworkState)

Small note: in this case you could simplify it with (same applies to the android side):

_currentNetworkState.emit(it)

Actually, I don't like using it. I choose the long names most of the time 🙃

benjohnde · 2023-03-22T15:04:55Z

src/androidMain/kotlin/com/liftric/job/queue/NetworkManager.kt

+
+    private val connectionPublisher = MutableStateFlow(getCurrentNetworkConnection())
+
+    fun observeNetworkConnection(): Flow<NetworkState?> = connectionPublisher


Do I understand the correctly that on iOS we basically observe the connection, hence the Flow could emit changes and on Android we only get the state at the time of the function invocation?

Well, I just realized that now and I guess the answer is yes. I will do the following change:

Suggested change

fun observeNetworkConnection(): Flow<NetworkState?> = connectionPublisher

val network: Flow<NetworkState?> = connectionPublisher

and then listen it just as in IOS in Android NetworkListener

override fun observeNetworkState() { scope.launch { networkManager .network .collectLatest { currentNetworkState -> _currentNetworkState.emit(currentNetworkState ?: NetworkState.NONE) } } }

benjohnde · 2023-03-22T15:07:23Z

src/androidMain/kotlin/com/liftric/job/queue/NetworkListener.kt

+
+
+    // not needed for Android
+    override fun stopMonitoring() {}


We could either implement that in a way that the state gets refreshed every 5 seconds (or in any other interval) to mimic the iOS behaviour a bit. But this is not subject to be implemented instantly but rather - if I understood everything right - to keep it as an issue for future improvements.

Actually, I can just create a job variable and assign it in observeNetworkState. Then, I can simply cancel it. I will add that for now. I had a wrong thinking at first. I will also do the same for ios.

Suggested change

override fun stopMonitoring() {}

override fun stopMonitoring() {

job?.cancel()

}

benjohnde · 2023-03-22T15:08:39Z

src/androidMain/kotlin/com/liftric/job/queue/NetworkManager.kt

+            preAndroidMNetworkConnection(connectivityManager)
+        }
+
+    @TargetApi(Build.VERSION_CODES.M)


Lovely to have it backwards compatible. We should internally discuss which API level makes sense for Flowify. I would rather start with Android 6 or 7 to get rid of some old chunks. But nice that you've implemented it with old versions in mind! Just leave it as is at the moment, we may remove it later on!

benjohnde · 2023-03-22T15:16:07Z

src/commonMain/kotlin/com/liftric/job/queue/JobQueue.kt

@@ -135,18 +145,49 @@ abstract class AbstractJobQueue(
                job.delegate = delegate


I guess somewhere in the while(isActive) loop we need to invoke NetworkListener.observeNetworkState(). The invocation is currently missing in the code :)

🥴 sir yes sir

canonall · 2023-03-22T11:18:31Z

src/commonTest/kotlin/com/liftric/job/queue/JobQueueTests.kt

+        }
+
+        queue.schedule(TestData(id), ::TestTask) {
+            minRequiredNetwork(NetworkState.NONE, 3.seconds)


As there is no real or simulated device to observe the network, the default value for the network is always NetworkState.NONE. If the minRequiredNetwork is also set to NetworkState.NONE, the test will pass. However, in any other case, the test will fail. Any ideas to deal with this?

I think that was because I did not call observeNetworkState as you mentioned. Now, it behaves normally. The test environment network is set to MOBILE I guess. But I do not know how to change it to WIFI if I need to.

canonall · 2023-03-23T09:10:50Z

src/commonMain/kotlin/com/liftric/job/queue/JobQueue.kt

-                        listener.emit(result)
+                        var shouldRunJob = false
+                        try {
+                            withTimeout(job.info.networkRuleTimeout) NetworkRuleTimeout@{


withTimeout(job.info.networkRuleTimeout) NetworkRuleTimeout@{ networkListener.currentNetworkState.collect { currentNetworkState -> val isNetworkRuleSatisfied = networkListener.isNetworkRuleSatisfied( jobInfo = job.info, currentNetworkState = currentNetworkState ) if (isNetworkRuleSatisfied) { shouldRunJob = true jobEventListener.emit(JobEvent.NetworkRuleSatisfied(job)) [email protected]("Network rule satisfied") } } }

The aim of using the withTimeout function was to ensure that the network requirement is met within a specified time frame. With each emission of the currentNetworkState, the 'isNetworkRuleSatisfied' flag is checked, instantly. If the flag is true, the NetworkRuleTimeout scope is canceled. If the flag is false, the currentNetworkState continues to be observed until the timeout expires. Without withTimeout the job would never be canceled until the network requirement is met, which I thought not desirable.

canonall · 2023-03-23T09:19:57Z

src/iosMain/kotlin/com/liftric/job/queue/NetworkListener.kt

+        networkManager.startMonitoring()
+        scope.launch {
+            networkManager.network.collectLatest { currentNetworkState ->
+                _currentNetworkState.emit(


Actually, I don't like using it. I choose the long names most of the time 🙃

canonall · 2023-03-23T09:37:25Z

src/androidMain/kotlin/com/liftric/job/queue/NetworkManager.kt

+
+    private val connectionPublisher = MutableStateFlow(getCurrentNetworkConnection())
+
+    fun observeNetworkConnection(): Flow<NetworkState?> = connectionPublisher


Well, I just realized that now and I guess the answer is yes. I will do the following change:

Suggested change

fun observeNetworkConnection(): Flow<NetworkState?> = connectionPublisher

val network: Flow<NetworkState?> = connectionPublisher

and then listen it just as in IOS in Android NetworkListener

override fun observeNetworkState() { scope.launch { networkManager .network .collectLatest { currentNetworkState -> _currentNetworkState.emit(currentNetworkState ?: NetworkState.NONE) } } }

canonall · 2023-03-23T09:44:22Z

src/commonMain/kotlin/com/liftric/job/queue/JobQueue.kt

@@ -135,18 +145,49 @@ abstract class AbstractJobQueue(
                job.delegate = delegate


🥴 sir yes sir

canonall · 2023-03-23T10:07:40Z

src/androidMain/kotlin/com/liftric/job/queue/NetworkListener.kt

+
+
+    // not needed for Android
+    override fun stopMonitoring() {}


Actually, I can just create a job variable and assign it in observeNetworkState. Then, I can simply cancel it. I will add that for now. I had a wrong thinking at first. I will also do the same for ios.

Suggested change

override fun stopMonitoring() {}

override fun stopMonitoring() {

job?.cancel()

}

canonall added 12 commits March 20, 2023 10:05

add konnection library

0055332

implement NetworkListener using Konnection

308600d

add NetworkListener to JobQueue

ca34720

implement NetworkRule

4cf22f8

update tests

bd96487

move network check inside willRun

65b1a20

move withTimeout to JobQueue

8efa999

update network rule test

2635030

remove Konnection library

d206da3

implement NetworkManager without Konnection and update NetworkListener

4932108

code clean-up

a9c940f

fix flaky tests

a9c7358

canonall changed the title ~~Feat: Network Rule~~ Draft: Feat: Network Rule Mar 21, 2023

canonall requested a review from benjohnde March 21, 2023 08:40

canonall added 11 commits March 21, 2023 09:44

add missing actual keyword for ios NetworkManager

51f2b87

@SuppressLint("MissingPermission") - AndroidManifest and permissions …

e1d8a13

…are already added

use abstract NetworkListener

208effc

observe network changes in the queue

0bb1b79

update NetworkListener:

bac0add

- use StateFlow - take NetworkManager as parameter

add new JobEvents

95f862d

refactor NetworkException to NetworkRuleTimeoutException

185bed6

wait for network rule to be satisfied 15 seconds, otherwise cancel it

6ea1857

update test

ea76420

chore: rename timeout to jobTimeout and allow user to define a networ…

d92cc9d

…kRuleTimeout

chore: fix tests

4b5b720

canonall changed the title ~~Draft: Feat: Network Rule~~ Feat: Network Rule Mar 22, 2023

benjohnde suggested changes Mar 22, 2023

View reviewed changes

canonall added 3 commits March 23, 2023 09:49

refactor: use named arguments and rename some variables

164ce28

call observeNetworkState() before running the job

5377d9d

refactor NetworkListeners and android network manager

56801aa

canonall commented May 4, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Network Rule #7

Feat: Network Rule #7

canonall commented Mar 21, 2023 •

edited

Loading

benjohnde left a comment

benjohnde Mar 22, 2023

benjohnde Mar 22, 2023

benjohnde Mar 22, 2023

canonall Mar 23, 2023

benjohnde Mar 22, 2023

benjohnde Mar 22, 2023

benjohnde Mar 22, 2023

canonall Mar 23, 2023

benjohnde Mar 22, 2023

canonall Mar 23, 2023

benjohnde Mar 22, 2023

canonall Mar 23, 2023

benjohnde Mar 22, 2023

benjohnde Mar 22, 2023

canonall Mar 23, 2023

canonall Mar 22, 2023

canonall Mar 23, 2023

canonall Mar 23, 2023

canonall Mar 23, 2023

canonall Mar 23, 2023

canonall Mar 23, 2023

canonall Mar 23, 2023


		private val connectionPublisher = MutableStateFlow(getCurrentNetworkConnection())

		fun observeNetworkConnection(): Flow<NetworkState?> = connectionPublisher

	fun observeNetworkConnection(): Flow<NetworkState?> = connectionPublisher
	val network: Flow<NetworkState?> = connectionPublisher

		@@ -135,18 +145,49 @@ abstract class AbstractJobQueue(
		job.delegate = delegate

Feat: Network Rule #7

Are you sure you want to change the base?

Feat: Network Rule #7

Conversation

canonall commented Mar 21, 2023 • edited Loading

benjohnde left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

canonall commented Mar 21, 2023 •

edited

Loading