Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Take into account well-known config to disable WorkManager network constraint #8662

Merged
merged 14 commits into from Oct 13, 2023

Conversation

bmarty
Copy link
Member

@bmarty bmarty commented Oct 9, 2023

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

  • Read new optional boolean from .well-know file "io.element.disable_network_constraint": true (@giomfo note that this is a boolean, not a String, let me know if it's OK for you)
  • If the value is true, the network constraint will not be added when build worker for the WorkManager.

Motivation and context

Be able to run the application in air gapped mode.

Screenshots / GIFs

Tests

  • Simulate .well-know configuration (hack the code) and observe that the network constraint is not add to the worker constraint.

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@bmarty bmarty requested a review from giomfo October 9, 2023 10:10
Copy link
Member

@giomfo giomfo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmarty these changes look good to me. I reviewed and ran some tests
Did you test with an actual wellknown response including the new param?

@bmarty
Copy link
Member Author

bmarty commented Oct 9, 2023

Did you test with an actual wellknown response including the new param?

No, I just tested the case by hacking the code.

@bmarty bmarty marked this pull request as ready for review October 10, 2023 08:43
@bmarty bmarty requested a review from ganfra October 10, 2023 08:44
Copy link
Contributor

@ganfra ganfra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

bmarty and others added 3 commits October 10, 2023 10:48
…AILED

e: file:///home/runner/actions-runner/_work/element-android/element-android/matrix-sdk-android/src/test/java/org/matrix/android/sdk/internal/session/pushers/DefaultPushersServiceTest.kt:55:5 No value passed for parameter 'homeServerCapabilitiesDataSource'
… ensure that the config is retrieved.

The add pusher worker can be configured before the .well-known config is retrieved.
@bmarty bmarty force-pushed the feature/bma/noNetworkConstraint branch from 8aab4a9 to d4c6a46 Compare October 12, 2023 16:14
Copy link
Member

@giomfo giomfo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I did some tests with a local HS and a custom wellknown response: OK

@bmarty bmarty merged commit 8bfd5f7 into develop Oct 13, 2023
13 of 14 checks passed
@bmarty bmarty deleted the feature/bma/noNetworkConstraint branch October 13, 2023 20:03
@sonarcloud
Copy link

sonarcloud bot commented Oct 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

warning The version of Java (11.0.20.1) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants