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.

Sign up for GitHub

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

Jump to bottom

Move Firebase Database URL to Android secrets #2717

Merged
merged 6 commits into from
Oct 11, 2023
Merged

Move Firebase Database URL to Android secrets #2717

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
853e8fa
Remove Firebase URL from google-services template
avazirna Jul 18, 2023
de11e75
Add Firebase URL to android secrets
avazirna Jul 18, 2023
d4c7e62
Merge branch 'master' into move_firebase_url_to_android_secrets
avazirna Oct 3, 2023
5b2b0b7
Revert "Remove Firebase URL from google-services template"
avazirna Oct 5, 2023
6328989
Inject Firebase URL into firebase config file
avazirna Oct 6, 2023
f3039d0
Merge branch 'master' into move_firebase_url_to_android_secrets
avazirna Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,29 +160,31 @@ ext {
HQ_API_USERNAME = project.properties['HQ_API_USERNAME'] ?: ""
HQ_API_PASSWORD = project.properties['HQ_API_PASSWORD'] ?: ""
TEST_BUILD_TYPE = project.properties['TEST_BUILD_TYPE'] ?: "debug"
FIREBASE_DATABASE_URL = project.properties['FIREBASE_DATABASE_URL'] ?: ""
}

afterEvaluate {
// Hack to get assets to show up in robolectric tests; try to eventually remove this
preCommcareDebugUnitTestBuild.dependsOn mergeCommcareDebugAssets
processStandaloneDebugGoogleServices.dependsOn injectGoogleServicesAPIKey
processStandaloneReleaseGoogleServices.dependsOn injectGoogleServicesAPIKey
processLtsDebugGoogleServices.dependsOn injectGoogleServicesAPIKey
processLtsReleaseGoogleServices.dependsOn injectGoogleServicesAPIKey
processCommcareDebugGoogleServices.dependsOn injectGoogleServicesAPIKey
processCommcareReleaseGoogleServices.dependsOn injectGoogleServicesAPIKey
processStandaloneDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile
processStandaloneReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile
processLtsDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile
processLtsReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile
processCommcareDebugGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile
processCommcareReleaseGoogleServices.dependsOn injectPropertiesIntoFirebaseConfigFile
}

/**
* https://discuss.gradle.org/t/gradle-7-0-seems-to-take-an-overzealous-approach-to-inter-task-dependencies/39656/2
* Gradle 7.0 treats any copy tasks as having an implicit dependencies with each other.
*/
task injectGoogleServicesAPIKey {
description = 'Injects the Google Services API key into the google-services.json file at runtime'
task injectPropertiesIntoFirebaseConfigFile {
description = 'Injects properties into the google-services.json file at runtime'

copy {
from 'templates/google-services.json'
filter(ReplaceTokens, tokens: [apiKey: project.ext.GOOGLE_SERVICES_API_KEY])
filter(ReplaceTokens, tokens: [apiKey: project.ext.GOOGLE_SERVICES_API_KEY,
firebaseDatabaseUrl: project.ext.FIREBASE_DATABASE_URL])
into projectDir
}
}
Expand Down Expand Up @@ -272,7 +274,7 @@ android {

buildConfigField "String", "HQ_API_USERNAME", "\"${project.ext.HQ_API_USERNAME}\""
buildConfigField "String", "HQ_API_PASSWORD", "\"${project.ext.HQ_API_PASSWORD}\""

buildConfigField "String", "FIREBASE_DATABASE_URL", "\"${project.ext.FIREBASE_DATABASE_URL}\""
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this feeds into google-services.json ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't, it will come from ~/.gradle/gradle.properties now

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't firebase need it to be in google-services.json ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the tests I ran, it doesn't seems that there aren't services relying on it. I can re-run them just to make sure.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you add what tests you run to the safety story for this PR ?

Copy link
Contributor

Choose a reason for hiding this comment

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

we are adding FIREBASE_DATABASE_URL as a build config field but never utilising this field. As such I am struggling to understand the approach behind this change in how Firebase will utilise it from the build config ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment it seems that there is no use for it, so we could actually remove it. But because the initial reasoning was just to 'move' it, I decided to put it there. Should we just remove it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't understand the need to move or delete it and a bit hesitant to go ahead with it without testing all Firebase services we use. The url is a public url and is behind authentication, since we already have the project_id visible in google services json anyone can anyway construct this url easily with format https://{project_id}.firebaseio.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, that was also my thinking, with the project ID it's easy to put the url together. Is there any particular Firebase we should be concerned about? I can run additional tests.

testInstrumentationRunner "org.commcare.CommCareJUnitRunner"
}

Expand Down
2 changes: 1 addition & 1 deletion app/templates/google-services.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"project_info": {
"project_number": "768065119425",
"firebase_url": "https://commcare-a57e4.firebaseio.com",
"firebase_url": "@firebaseDatabaseUrl@",
"project_id": "commcare-a57e4",
"storage_bucket": "commcare-a57e4.appspot.com"
},
Expand Down