-
Notifications
You must be signed in to change notification settings - Fork 14
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
Feature/real time flags (#5) #21
Merged
matthewelwell
merged 9 commits into
Flagsmith:feat/real-time
from
foresightmobile:feature/real-time-flags-squashed
Sep 11, 2023
Merged
Feature/real time flags (#5) #21
matthewelwell
merged 9 commits into
Flagsmith:feat/real-time
from
foresightmobile:feature/real-time-flags-squashed
Sep 11, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Compiles and test, need to add some tests then get it into a sample app * Should be testing but not getting the errors through Fuse so can't run into the logic * Probably gone as far as I can with 2.x fuel, let's try the 3.x * Move to Retrofit - seems to be going well so far, test runs * Tidying up, setTrait test not working * All the tests are passing so will finish the retrofit migration * Updated some of the logic and added setTraits * Checkpoint commit before trying generic converter * Generics working fine * All passing for flags and such with the new generic caching * Mostly swapped to Retrofit, now need to do the analytics * Analytics now over to retrofit * Add caching for the getFlags endpoint * Get rid of the last of Fuel * Another clear-out and all working fine on the tests * Now using Retrofit cache, remove the old stuff * Now just using HTTP caching * Delete the old caching logic * Finishing off, should be done for defaults and caching * Remove unneeded todo * Remove some more code * Still just playing around with it * Move cache configuration to its own data class * Tidy up the cache config and the tests * Update the comments * Now covers the caching tests * Tidy up some more of the tests * Some more tidying up * Default to caching disabled * Last few PR comments * Split the read and write timeout for HTTP * Initial basic implementation, let's try to get things hooked up to the server * Seems to be generally working * Checkpoint commit, seems to be generally working now just need to get the flags on update * Checkpoint commit before making the changes OK'd by Matthew to move the update clock into the event service * Ensure that the event source just reconnects if it loses the connection * Events and timers now all hooked-up and working in the manual integration test * Got the integration test working * Tidy everything up and move sensitive data to environment variables * Add a new test to cover the event stream going through a reconnect cycle * Added test for the live stream of flags, tidied up the imports and various thing, changed the logic a bit for when we need to do updates from events
FlagsmithClient/src/test/java/com/flagsmith/RealTimeUpdatesIntegrationTests.kt
Outdated
Show resolved
Hide resolved
…egrationTests.kt Co-authored-by: Matthew Elwell <[email protected]>
…re obvious what's going on if we don't configure properly
…/foresightmobile/flagsmith-kotlin-android-client into feature/real-time-flags-squashed
matthewelwell
added a commit
that referenced
this pull request
Nov 9, 2023
* Feature/real time flags (#5) * Compiles and test, need to add some tests then get it into a sample app * Should be testing but not getting the errors through Fuse so can't run into the logic * Probably gone as far as I can with 2.x fuel, let's try the 3.x * Move to Retrofit - seems to be going well so far, test runs * Tidying up, setTrait test not working * All the tests are passing so will finish the retrofit migration * Updated some of the logic and added setTraits * Checkpoint commit before trying generic converter * Generics working fine * All passing for flags and such with the new generic caching * Mostly swapped to Retrofit, now need to do the analytics * Analytics now over to retrofit * Add caching for the getFlags endpoint * Get rid of the last of Fuel * Another clear-out and all working fine on the tests * Now using Retrofit cache, remove the old stuff * Now just using HTTP caching * Delete the old caching logic * Finishing off, should be done for defaults and caching * Remove unneeded todo * Remove some more code * Still just playing around with it * Move cache configuration to its own data class * Tidy up the cache config and the tests * Update the comments * Now covers the caching tests * Tidy up some more of the tests * Some more tidying up * Default to caching disabled * Last few PR comments * Split the read and write timeout for HTTP * Initial basic implementation, let's try to get things hooked up to the server * Seems to be generally working * Checkpoint commit, seems to be generally working now just need to get the flags on update * Checkpoint commit before making the changes OK'd by Matthew to move the update clock into the event service * Ensure that the event source just reconnects if it loses the connection * Events and timers now all hooked-up and working in the manual integration test * Got the integration test working * Tidy everything up and move sensitive data to environment variables * Add a new test to cover the event stream going through a reconnect cycle * Added test for the live stream of flags, tidied up the imports and various thing, changed the logic a bit for when we need to do updates from events * Update FlagsmithClient/src/test/java/com/flagsmith/RealTimeUpdatesIntegrationTests.kt Co-authored-by: Matthew Elwell <[email protected]> * Update environment variables in the github actions * Add some error checking on the environment variables so it's a bit more obvious what's going on if we don't configure properly * Noddy change to get the tests to run again * Try printing out unsuccessful responses in the integration tests * Push more more non-empty checks * Feature/real time flags (#5) (#21) * Feature/real time flags (#5) * Compiles and test, need to add some tests then get it into a sample app * Should be testing but not getting the errors through Fuse so can't run into the logic * Probably gone as far as I can with 2.x fuel, let's try the 3.x * Move to Retrofit - seems to be going well so far, test runs * Tidying up, setTrait test not working * All the tests are passing so will finish the retrofit migration * Updated some of the logic and added setTraits * Checkpoint commit before trying generic converter * Generics working fine * All passing for flags and such with the new generic caching * Mostly swapped to Retrofit, now need to do the analytics * Analytics now over to retrofit * Add caching for the getFlags endpoint * Get rid of the last of Fuel * Another clear-out and all working fine on the tests * Now using Retrofit cache, remove the old stuff * Now just using HTTP caching * Delete the old caching logic * Finishing off, should be done for defaults and caching * Remove unneeded todo * Remove some more code * Still just playing around with it * Move cache configuration to its own data class * Tidy up the cache config and the tests * Update the comments * Now covers the caching tests * Tidy up some more of the tests * Some more tidying up * Default to caching disabled * Last few PR comments * Split the read and write timeout for HTTP * Initial basic implementation, let's try to get things hooked up to the server * Seems to be generally working * Checkpoint commit, seems to be generally working now just need to get the flags on update * Checkpoint commit before making the changes OK'd by Matthew to move the update clock into the event service * Ensure that the event source just reconnects if it loses the connection * Events and timers now all hooked-up and working in the manual integration test * Got the integration test working * Tidy everything up and move sensitive data to environment variables * Add a new test to cover the event stream going through a reconnect cycle * Added test for the live stream of flags, tidied up the imports and various thing, changed the logic a bit for when we need to do updates from events * Update FlagsmithClient/src/test/java/com/flagsmith/RealTimeUpdatesIntegrationTests.kt Co-authored-by: Matthew Elwell <[email protected]> * Update environment variables in the github actions * Add some error checking on the environment variables so it's a bit more obvious what's going on if we don't configure properly * Noddy change to get the tests to run again * Try printing out unsuccessful responses in the integration tests * Push more more non-empty checks --------- Co-authored-by: Matthew Elwell <[email protected]> * Split the actions into push and pull-request versions * Remove the old script * Update the timeouts on the integration tests and make them a little more robust * Fix up some merge issues * Use the flow value directly in the unit tests and simplify the flow updates in the SDK so they're always passive * Quick update to see if we're getting an error back when pushing the flag * Bump to 4c runner * Move to Strings as the server prefers them. Increase the timeouts and make the check loops more CPU friendly * Updates to cover most of the review comments * Make some of the Flagsmith class external again, move the test-only portions of the API into the test folder * Suppress a case warning in the test code * Remove integration test env vars now that we know that they passed with them included * Exclude integration tests in the Kover run * Disable coverage on PR workflow * Remove some duplicated logic * Update construction of event source URL * Add some constants and rearrange a bit of code * Return the env key interceptor to its original position * Various updates to the integration tests to cover Matthew's comments * Tidied up the integration tests, also added a bit more to one of the timeouts as it was taking a while for the realtime updates to come though * Tidied some of the println() statements into the asserts, commented on the reasoning for some of the timeouts and delays --------- Co-authored-by: Gareth Reese <[email protected]> Co-authored-by: Gareth Reese <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds the real time flags feature as described in #18 and Flagsmith/flagsmith#1498
Fixes #18
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Because the real-time updates are dependent upon the the operation of the SSE functionality I found it best to write end-to-end integrations test for this feature. They test the main 'update' scenario, as well as ensuring that the SSE connection reconnects. It
Test Configuration:
As these are integration tests they need some environment variables to connect to an account, feature and value. These are at the top of the integration test and follow the format seen in the existing documentation:
To skip the integration tests you can run the
gradle
command as shown in the build.gradle:Checklist:
Changes
Compiles and test, need to add some tests then get it into a sample app
Should be testing but not getting the errors through Fuse so can't run into the logic
Probably gone as far as I can with 2.x fuel, let's try the 3.x
Move to Retrofit - seems to be going well so far, test runs
Tidying up, setTrait test not working
All the tests are passing so will finish the retrofit migration
Updated some of the logic and added setTraits
Checkpoint commit before trying generic converter
Generics working fine
All passing for flags and such with the new generic caching
Mostly swapped to Retrofit, now need to do the analytics
Analytics now over to retrofit
Add caching for the getFlags endpoint
Get rid of the last of Fuel
Another clear-out and all working fine on the tests
Now using Retrofit cache, remove the old stuff
Now just using HTTP caching
Delete the old caching logic
Finishing off, should be done for defaults and caching
Remove unneeded todo
Remove some more code
Still just playing around with it
Move cache configuration to its own data class
Tidy up the cache config and the tests
Update the comments
Now covers the caching tests
Tidy up some more of the tests
Some more tidying up
Default to caching disabled
Last few PR comments
Split the read and write timeout for HTTP
Initial basic implementation, let's try to get things hooked up to the server
Seems to be generally working
Checkpoint commit, seems to be generally working now just need to get the flags on update
Checkpoint commit before making the changes OK'd by Matthew to move the update clock into the event service
Ensure that the event source just reconnects if it loses the connection
Events and timers now all hooked-up and working in the manual integration test
Got the integration test working
Tidy everything up and move sensitive data to environment variables
Add a new test to cover the event stream going through a reconnect cycle
Added test for the live stream of flags, tidied up the imports and various thing, changed the logic a bit for when we need to do updates from events