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

feat: Add configMaxAge to MParticleOptions #85

Merged
merged 2 commits into from
Jan 6, 2022

Conversation

willpassidomo
Copy link
Contributor

@willpassidomo willpassidomo commented Dec 21, 2021

(moved from internal repo)

Summary

Add public API for setting a max config staleness parameter. If the config is older than staleness limit, we will delete it instead of loading it from memory while we wait for the new config

Usage

val options = MParticleOptions.builder(context)
   .maxConfigAge(1000 * 60 * 60 * 24 * 7)                   <- set a max age of 1 week
   .build()

note: maxConfigAge value is DAYS SECONDS

API Behavior

  • not setting maxConfigAge will default it to null which means it will not take effect and the SDK will behave exactly how it currently does
  • setting it to a negative Integer will trigger a warning-level log message then proceed as if it was null (same as above)
  • setting it to 0 will result in the stored config being deleted every time the SDK starts up aka Kits et al we will behave like it's a fresh install every time
    - setting it to an Integer greater than 100 will trigger a warning-level log message reminding the client that the time unit is days, but still proceed with the value
  • setting it to a reasonable (<100) Integer will set that value as the config max-age in days seconds

Internal Behavior

  • we will check config age exactly once per app start in the ConfigManager constructor

although it is possible the ConfigManager may get called at other points in our code, no where else will it pass in a configMaxAge, so in those cases it would default to null/do nothing

  • staleness is determined by ("config timestamp" + "Milliseconds in a day" * configMaxAge) < System.currentTimeMillisso there is no rounding or anything, the config will be invalid
  • when a config is "deleted" for staleness reasons, we will also delete the stored ETag and stored If-Modified values.

Testing Plan

  • unit tests for MParticleOptions and ConfigManager
  • integration tests for maxConfigAge == 0 and "maxConfigAge not set" where we 1) start the SDK 2) wait for config to load 3) stop SDK 4) restart SDK 5) check stored config before the new one is fetched
  • integration test for maxConfig == (big #) and make sure it isn't deleted before it should
  • new listener in ComfigManager that will callback when a config is "loaded" and also indicate whether it was a "reload" or a new config...needed this so we had something to actually wait on when refreshing the config in test

//TODO
- mock time and add an integration test for maxConfig == (some number), change the internal time and verify it gets properly deleted

Master Issue

Closes https://mparticle.tpondemand.com/RestUI/Board.aspx#page=board/4842431717611364328&appConfig=eyJhY2lkIjoiMjM4RjY2OUU2RkYxNzFCMTVCMUU5Q0JCRDlFQzQyRTAifQ==&boardPopup=bug/71594/silent

@mchuangatmp mchuangatmp changed the base branch from main to development December 21, 2021 19:21
}
}

void saveConfigJson(JSONObject json, String etag, String lastModified) throws JSONException {
Copy link
Member

Choose a reason for hiding this comment

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

I like moving the responsibility to persist these here. That said - there is a slight behavior change in that we used to check if they were null. This might be better - whereby if they are null we will pass null into putString below - but then we will also get null back out whereas behavior maybe we wouldn't have? Is there any concern when these are null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thats a good call-out, it is a change in existing behavior. When I came across the previous logic, it looked like a bug to me, although not a particularly bad one. If we fetch "config1" which returns with "etag1", then the next time around we fetch "config2" which returns without an etag, on the next request we will be using the stale "etag1" in our request header which may lead the server to believe our config is stale-er than it actually is. Net-net, the way I read it, the previous behavior may lead to us fetching more configs than we need too (the extra should have been 304s).

That said, I don't see any concern over this change. Sans any comments or notes on some unexpected behavior we were accounting for when we originally implemented this, this new version seems more "correct"

Set a maximum age threshold for the locally cached config. When a store config surpasses the threshold, it will be purged on the next SDK start, and the SDK will operate without a config until one is returned successfully from the server. configMaxAge is set on startup, in seconds, by calling MParticleOptions.Builder#configMaxAge(Long)
@willpassidomo willpassidomo merged commit eaadccd into development Jan 6, 2022
@willpassidomo willpassidomo deleted the feat/71594-config-staleness branch January 6, 2022 16:38
github-actions bot pushed a commit that referenced this pull request Jan 6, 2022
# [5.34.0](v5.33.2...v5.34.0) (2022-01-06)

### Bug Fixes

* Add additional checks for AndroidId ([#87](#87)) ([1eceea3](1eceea3))
* Change configMaxAge type from Long to int ([#91](#91)) ([14ad4b5](14ad4b5))

### Features

* Add configMaxAge to MParticleOptions ([#85](#85)) ([eaadccd](eaadccd))
@mparticle-automation
Copy link
Collaborator

🎉 This PR is included in version 5.34.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit to mchuangatmp/mparticle-android-sdk that referenced this pull request Mar 23, 2022
# [5.18.0](v5.17.0...v5.18.0) (2022-03-23)

### Bug Fixes

* Add additional checks for AndroidId ([mParticle#87](https:/mchuangatmp/mparticle-android-sdk/issues/87)) ([1eceea3](1eceea3))
* add dry run to fix daily cron health check job ([mParticle#95](https:/mchuangatmp/mparticle-android-sdk/issues/95)) ([92c443f](92c443f))
* bad formatting in release.config.js ([mParticle#75](https:/mchuangatmp/mparticle-android-sdk/issues/75)) ([253c9a3](253c9a3))
* bugfix some lint issues ([3177500](3177500))
* Change configMaxAge type from Long to int ([mParticle#91](https:/mchuangatmp/mparticle-android-sdk/issues/91)) ([14ad4b5](14ad4b5))
* downgrade error log to debug ([mParticle#102](https:/mchuangatmp/mparticle-android-sdk/issues/102)) ([293ce35](293ce35))
* Fix cross platforms for Android forked PRs when checking out code ([mParticle#97](https:/mchuangatmp/mparticle-android-sdk/issues/97)) ([90b2760](90b2760))
* Hardcode macos version for Github Actions ([de1f552](de1f552))
* keep package names in proguard to prevent collisions ([mParticle#109](https:/mchuangatmp/mparticle-android-sdk/issues/109)) ([a8df4fe](a8df4fe))
* post new release versions to github ([707c9ec](707c9ec))
* remove Appsee submodule ([8a172b4](8a172b4))
* remove bintray url from kits ([323d57d](323d57d))
* Remove broken Sonatype plugin dependency ([41e86a1](41e86a1))
* remove deprecated kahuna kit for semantic release ([9defb08](9defb08))
* test commit ([e1fb76e](e1fb76e))
* Transitive dependency, remove version range for localbroadcastmanager ([5f94984](5f94984))
* Treat duplicate Push tokens as background tokens if no current Session ([63eb698](63eb698))
* typo in GA4 kit name ([mParticle#112](https:/mchuangatmp/mparticle-android-sdk/issues/112)) ([45f9770](45f9770))
* typo in release.config.js ([mParticle#76](https:/mchuangatmp/mparticle-android-sdk/issues/76)) ([cf30ab8](cf30ab8))
* update java docs syntax after gradle upgrade ([mParticle#98](https:/mchuangatmp/mparticle-android-sdk/issues/98)) ([9a8e22e](9a8e22e))
* update with proper kit configuration ([mParticle#113](https:/mchuangatmp/mparticle-android-sdk/issues/113)) ([7496077](7496077))

### Features

* Add configMaxAge to MParticleOptions ([mParticle#85](https:/mchuangatmp/mparticle-android-sdk/issues/85)) ([eaadccd](eaadccd))
* add in ktlintcheck ([mParticle#108](https:/mchuangatmp/mparticle-android-sdk/issues/108)) ([f75c5b6](f75c5b6))
* add in lint error check for kits ([81307ae](81307ae))
* add in semantic release work ([8e26f79](8e26f79))
* add in sonatype work ([6e73455](6e73455))
* add support for firebase messaging v22.0.0 ([0facc42](0facc42))
* add upload bypass support to logscreen ([177a5d5](177a5d5))
* Android id disabled default true ([mParticle#89](https:/mchuangatmp/mparticle-android-sdk/issues/89)) ([7fc0217](7fc0217))
* Implement event upload bypass option ([3d8a967](3d8a967))
* remove deprecated methods, AppConfig ([mParticle#100](https:/mchuangatmp/mparticle-android-sdk/issues/100)) ([a5a934a](a5a934a))
* split configuration into core and kits ([mParticle#103](https:/mchuangatmp/mparticle-android-sdk/issues/103)) ([766e0d3](766e0d3))
* switch crossplatform tests to run on target repo due to token permission scope ([b7e946a](b7e946a))
* test gpg key ([3239c02](3239c02))
* turn on lint errors ([9c98535](9c98535))
* Update Submodules ([b634d90](b634d90))
* Update Submodules ([eb4573f](eb4573f))
* Update Submodules ([45bfd95](45bfd95))
* Update Submodules ([9aa5077](9aa5077))
* Update Submodules ([d584a3b](d584a3b))
* Update Submodules ([055c896](055c896))
* Update Submodules ([6b2d6be](6b2d6be))
* Update Submodules ([ed75216](ed75216))
* Update Submodules ([216c775](216c775))
* Update Submodules ([1826382](1826382))
* Update Submodules ([1cf5869](1cf5869))
* Update Submodules ([e46a282](e46a282))
* Update Submodules ([4a34f66](4a34f66))
* Update Submodules ([4918847](4918847))
* upgrade to AGP 7.X ([267bf36](267bf36))

### Reverts

* Rollback Kotlin upgrade ([7deee2e](7deee2e))
github-actions bot pushed a commit to mchuangatmp/mparticle-android-sdk that referenced this pull request Mar 23, 2022
# [5.18.0](v5.17.0...v5.18.0) (2022-03-23)

### Bug Fixes

* Add additional checks for AndroidId ([mParticle#87](https:/mchuangatmp/mparticle-android-sdk/issues/87)) ([1eceea3](1eceea3))
* add dry run to fix daily cron health check job ([mParticle#95](https:/mchuangatmp/mparticle-android-sdk/issues/95)) ([92c443f](92c443f))
* bad formatting in release.config.js ([mParticle#75](https:/mchuangatmp/mparticle-android-sdk/issues/75)) ([253c9a3](253c9a3))
* bugfix some lint issues ([3177500](3177500))
* Change configMaxAge type from Long to int ([mParticle#91](https:/mchuangatmp/mparticle-android-sdk/issues/91)) ([14ad4b5](14ad4b5))
* downgrade error log to debug ([mParticle#102](https:/mchuangatmp/mparticle-android-sdk/issues/102)) ([293ce35](293ce35))
* Fix cross platforms for Android forked PRs when checking out code ([mParticle#97](https:/mchuangatmp/mparticle-android-sdk/issues/97)) ([90b2760](90b2760))
* Hardcode macos version for Github Actions ([de1f552](de1f552))
* keep package names in proguard to prevent collisions ([mParticle#109](https:/mchuangatmp/mparticle-android-sdk/issues/109)) ([a8df4fe](a8df4fe))
* post new release versions to github ([707c9ec](707c9ec))
* remove Appsee submodule ([8a172b4](8a172b4))
* remove bintray url from kits ([323d57d](323d57d))
* Remove broken Sonatype plugin dependency ([41e86a1](41e86a1))
* remove deprecated kahuna kit for semantic release ([9defb08](9defb08))
* test commit ([1283aba](1283aba))
* Transitive dependency, remove version range for localbroadcastmanager ([5f94984](5f94984))
* Treat duplicate Push tokens as background tokens if no current Session ([63eb698](63eb698))
* typo in GA4 kit name ([mParticle#112](https:/mchuangatmp/mparticle-android-sdk/issues/112)) ([45f9770](45f9770))
* typo in release.config.js ([mParticle#76](https:/mchuangatmp/mparticle-android-sdk/issues/76)) ([cf30ab8](cf30ab8))
* update java docs syntax after gradle upgrade ([mParticle#98](https:/mchuangatmp/mparticle-android-sdk/issues/98)) ([9a8e22e](9a8e22e))
* update with proper kit configuration ([mParticle#113](https:/mchuangatmp/mparticle-android-sdk/issues/113)) ([7496077](7496077))

### Features

* Add configMaxAge to MParticleOptions ([mParticle#85](https:/mchuangatmp/mparticle-android-sdk/issues/85)) ([eaadccd](eaadccd))
* add in ktlintcheck ([mParticle#108](https:/mchuangatmp/mparticle-android-sdk/issues/108)) ([f75c5b6](f75c5b6))
* add in lint error check for kits ([81307ae](81307ae))
* add in semantic release work ([8e26f79](8e26f79))
* add in sonatype work ([6e73455](6e73455))
* add support for firebase messaging v22.0.0 ([0facc42](0facc42))
* add upload bypass support to logscreen ([177a5d5](177a5d5))
* Android id disabled default true ([mParticle#89](https:/mchuangatmp/mparticle-android-sdk/issues/89)) ([7fc0217](7fc0217))
* Implement event upload bypass option ([3d8a967](3d8a967))
* remove deprecated methods, AppConfig ([mParticle#100](https:/mchuangatmp/mparticle-android-sdk/issues/100)) ([a5a934a](a5a934a))
* split configuration into core and kits ([mParticle#103](https:/mchuangatmp/mparticle-android-sdk/issues/103)) ([766e0d3](766e0d3))
* switch crossplatform tests to run on target repo due to token permission scope ([b7e946a](b7e946a))
* test gpg key ([dad7acd](dad7acd))
* turn on lint errors ([9c98535](9c98535))
* Update Submodules ([b634d90](b634d90))
* Update Submodules ([eb4573f](eb4573f))
* Update Submodules ([45bfd95](45bfd95))
* Update Submodules ([9aa5077](9aa5077))
* Update Submodules ([d584a3b](d584a3b))
* Update Submodules ([055c896](055c896))
* Update Submodules ([6b2d6be](6b2d6be))
* Update Submodules ([ed75216](ed75216))
* Update Submodules ([216c775](216c775))
* Update Submodules ([1826382](1826382))
* Update Submodules ([1cf5869](1cf5869))
* Update Submodules ([e46a282](e46a282))
* Update Submodules ([4a34f66](4a34f66))
* Update Submodules ([4918847](4918847))
* upgrade to AGP 7.X ([267bf36](267bf36))

### Reverts

* Rollback Kotlin upgrade ([7deee2e](7deee2e))
github-actions bot pushed a commit to mchuangatmp/mparticle-android-sdk that referenced this pull request Mar 23, 2022
# [5.18.0](v5.17.0...v5.18.0) (2022-03-23)

### Bug Fixes

* Add additional checks for AndroidId ([mParticle#87](https:/mchuangatmp/mparticle-android-sdk/issues/87)) ([1eceea3](1eceea3))
* add dry run to fix daily cron health check job ([mParticle#95](https:/mchuangatmp/mparticle-android-sdk/issues/95)) ([92c443f](92c443f))
* bad formatting in release.config.js ([mParticle#75](https:/mchuangatmp/mparticle-android-sdk/issues/75)) ([253c9a3](253c9a3))
* bugfix some lint issues ([3177500](3177500))
* Change configMaxAge type from Long to int ([mParticle#91](https:/mchuangatmp/mparticle-android-sdk/issues/91)) ([14ad4b5](14ad4b5))
* downgrade error log to debug ([mParticle#102](https:/mchuangatmp/mparticle-android-sdk/issues/102)) ([293ce35](293ce35))
* Fix cross platforms for Android forked PRs when checking out code ([mParticle#97](https:/mchuangatmp/mparticle-android-sdk/issues/97)) ([90b2760](90b2760))
* Hardcode macos version for Github Actions ([de1f552](de1f552))
* keep package names in proguard to prevent collisions ([mParticle#109](https:/mchuangatmp/mparticle-android-sdk/issues/109)) ([a8df4fe](a8df4fe))
* post new release versions to github ([707c9ec](707c9ec))
* remove Appsee submodule ([8a172b4](8a172b4))
* remove bintray url from kits ([323d57d](323d57d))
* Remove broken Sonatype plugin dependency ([41e86a1](41e86a1))
* remove deprecated kahuna kit for semantic release ([9defb08](9defb08))
* test commit ([5fdbf9e](5fdbf9e))
* Transitive dependency, remove version range for localbroadcastmanager ([5f94984](5f94984))
* Treat duplicate Push tokens as background tokens if no current Session ([63eb698](63eb698))
* typo in GA4 kit name ([mParticle#112](https:/mchuangatmp/mparticle-android-sdk/issues/112)) ([45f9770](45f9770))
* typo in release.config.js ([mParticle#76](https:/mchuangatmp/mparticle-android-sdk/issues/76)) ([cf30ab8](cf30ab8))
* update java docs syntax after gradle upgrade ([mParticle#98](https:/mchuangatmp/mparticle-android-sdk/issues/98)) ([9a8e22e](9a8e22e))
* update with proper kit configuration ([mParticle#113](https:/mchuangatmp/mparticle-android-sdk/issues/113)) ([7496077](7496077))

### Features

* Add configMaxAge to MParticleOptions ([mParticle#85](https:/mchuangatmp/mparticle-android-sdk/issues/85)) ([eaadccd](eaadccd))
* add in ktlintcheck ([mParticle#108](https:/mchuangatmp/mparticle-android-sdk/issues/108)) ([f75c5b6](f75c5b6))
* add in lint error check for kits ([81307ae](81307ae))
* add in semantic release work ([8e26f79](8e26f79))
* add in sonatype work ([6e73455](6e73455))
* add support for firebase messaging v22.0.0 ([0facc42](0facc42))
* add upload bypass support to logscreen ([177a5d5](177a5d5))
* Android id disabled default true ([mParticle#89](https:/mchuangatmp/mparticle-android-sdk/issues/89)) ([7fc0217](7fc0217))
* Implement event upload bypass option ([3d8a967](3d8a967))
* remove deprecated methods, AppConfig ([mParticle#100](https:/mchuangatmp/mparticle-android-sdk/issues/100)) ([a5a934a](a5a934a))
* split configuration into core and kits ([mParticle#103](https:/mchuangatmp/mparticle-android-sdk/issues/103)) ([766e0d3](766e0d3))
* switch crossplatform tests to run on target repo due to token permission scope ([b7e946a](b7e946a))
* test gpg key ([6f782b6](6f782b6))
* turn on lint errors ([9c98535](9c98535))
* Update Submodules ([b634d90](b634d90))
* Update Submodules ([eb4573f](eb4573f))
* Update Submodules ([45bfd95](45bfd95))
* Update Submodules ([9aa5077](9aa5077))
* Update Submodules ([d584a3b](d584a3b))
* Update Submodules ([055c896](055c896))
* Update Submodules ([6b2d6be](6b2d6be))
* Update Submodules ([ed75216](ed75216))
* Update Submodules ([216c775](216c775))
* Update Submodules ([1826382](1826382))
* Update Submodules ([1cf5869](1cf5869))
* Update Submodules ([e46a282](e46a282))
* Update Submodules ([4a34f66](4a34f66))
* Update Submodules ([4918847](4918847))
* upgrade to AGP 7.X ([267bf36](267bf36))

### Reverts

* Rollback Kotlin upgrade ([7deee2e](7deee2e))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants