Skip to content

Commit 5ff343a

Browse files
authored
Fixes for the App Check Android testapp (#1281)
* Fixes for the App Check Android testapp * Formatting
1 parent 9c20916 commit 5ff343a

File tree

3 files changed

+49
-2
lines changed

3 files changed

+49
-2
lines changed

app_check/integration_test/build.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ android {
6868
"-DFIREBASE_INCLUDE_FUNCTIONS=ON",
6969
"-DFIREBASE_INCLUDE_FIRESTORE=ON"
7070
}
71+
multiDexEnabled true
7172
}
7273
externalNativeBuild.cmake {
7374
path 'CMakeLists.txt'
@@ -79,6 +80,9 @@ android {
7980
proguardFile file('proguard.pro')
8081
}
8182
}
83+
lintOptions {
84+
abortOnError false
85+
}
8286
}
8387

8488
apply from: "$gradle.firebase_cpp_sdk_dir/Android/firebase_dependencies.gradle"

app_check/integration_test/src/integration_test.cc

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -754,9 +754,24 @@ TEST_F(FirebaseAppCheckTest, TestDatabaseFailure) {
754754
firebase::database::DatabaseReference ref = CreateWorkingPath();
755755
const char* test_name = test_info_->name();
756756
firebase::Future<void> f = ref.Child(test_name).SetValue("test");
757+
#if FIREBASE_PLATFORM_ANDROID
758+
// On Android the future will not complete. This is present in the
759+
// underlying Android SDK, so work around it here.
760+
ASSERT_EQ(f.status(), firebase::kFutureStatusPending);
761+
std::this_thread::sleep_for(std::chrono::milliseconds(2000));
762+
ASSERT_EQ(f.status(), firebase::kFutureStatusPending);
763+
// Likewise, removal won't finish, so don't wait for it.
764+
if (!database_cleanup_.empty()) {
765+
LogDebug("Cleaning up Database...");
766+
for (int i = 0; i < database_cleanup_.size(); ++i) {
767+
database_cleanup_[i].RemoveValue();
768+
}
769+
database_cleanup_.clear();
770+
}
771+
#else
757772
WaitForCompletion(f, "SetString", firebase::database::kErrorDisconnected);
758-
759773
CleanupDatabase(firebase::database::kErrorOperationFailed);
774+
#endif
760775
}
761776

762777
TEST_F(FirebaseAppCheckTest, TestDatabaseCreateWorkingPath) {

app_check/src/android/app_check_android.cc

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,19 @@ METHOD_LOOKUP_DEFINITION(app_check,
5858
"com/google/firebase/appcheck/FirebaseAppCheck",
5959
APP_CHECK_METHODS)
6060

61+
// clang-format off
62+
#define DEFAULT_APP_CHECK_IMPL_METHODS(X) \
63+
X(ResetAppCheckState, "resetAppCheckState", "()V")
64+
// clang-format on
65+
66+
METHOD_LOOKUP_DECLARATION(default_app_check_impl,
67+
DEFAULT_APP_CHECK_IMPL_METHODS)
68+
METHOD_LOOKUP_DEFINITION(
69+
default_app_check_impl,
70+
PROGUARD_KEEP_CLASS
71+
"com/google/firebase/appcheck/internal/DefaultFirebaseAppCheck",
72+
DEFAULT_APP_CHECK_IMPL_METHODS)
73+
6174
// clang-format off
6275
#define JNI_APP_CHECK_PROVIDER_FACTORY_METHODS(X) \
6376
X(Constructor, "<init>", "(JJ)V")
@@ -159,11 +172,13 @@ bool CacheAppCheckMethodIds(
159172
FIREBASE_ARRAYSIZE(kNativeJniAppCheckListenerMethods)))) {
160173
return false;
161174
}
162-
return app_check::CacheMethodIds(env, activity);
175+
return app_check::CacheMethodIds(env, activity) &&
176+
default_app_check_impl::CacheMethodIds(env, activity);
163177
}
164178

165179
void ReleaseAppCheckClasses(JNIEnv* env) {
166180
app_check::ReleaseClass(env);
181+
default_app_check_impl::ReleaseClass(env);
167182
jni_provider_factory::ReleaseClass(env);
168183
jni_provider::ReleaseClass(env);
169184
jni_app_check_listener::ReleaseClass(env);
@@ -359,6 +374,19 @@ AppCheckInternal::~AppCheckInternal() {
359374
env->DeleteGlobalRef(j_app_check_listener_);
360375
}
361376
if (app_check_impl_ != nullptr) {
377+
// The Android App Check library holds onto the provider,
378+
// which can be a problem if it tries to call up to C++
379+
// after being deleted. So we use a hidden function meant
380+
// for testing purposes to clear out the App Check state,
381+
// to prevent this. Note, this assumes that the java object
382+
// is a DefaultFirebaseAppCheck (instead of a FirebaseAppCheck)
383+
// which is currently true, but may not be in the future.
384+
// We will have to rely on tests to detect if this changes.
385+
env->CallVoidMethod(app_check_impl_,
386+
default_app_check_impl::GetMethodId(
387+
default_app_check_impl::kResetAppCheckState));
388+
FIREBASE_ASSERT(!util::CheckAndClearJniExceptions(env));
389+
362390
env->DeleteGlobalRef(app_check_impl_);
363391
}
364392

0 commit comments

Comments
 (0)