Skip to content

Commit 3282885

Browse files
wilhuffa-maurice
wilhuff
authored and
a-maurice
committed
Convert ExceptionInternal to the new JNI framework
Also partially convert TransactionInternal since these are so intertwined. PiperOrigin-RevId: 334241429
1 parent ee4fa72 commit 3282885

9 files changed

+197
-218
lines changed

‎firestore/src/android/event_listener_android.cc

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -78,47 +78,49 @@ void EventListenerInternal::Initialize(jni::Loader& loader) {
7878
}
7979

8080
void EventListenerInternal::DocumentEventListenerNativeOnEvent(
81-
JNIEnv* env, jclass, jlong firestore_ptr, jlong listener_ptr, jobject value,
82-
jobject error) {
81+
JNIEnv* raw_env, jclass, jlong firestore_ptr, jlong listener_ptr,
82+
jobject value, jobject raw_error) {
8383
if (firestore_ptr == 0 || listener_ptr == 0) {
8484
return;
8585
}
8686
auto* listener =
8787
reinterpret_cast<EventListener<DocumentSnapshot>*>(listener_ptr);
8888
auto* firestore = reinterpret_cast<FirestoreInternal*>(firestore_ptr);
8989

90+
Env env(raw_env);
91+
Object error(raw_error);
9092
Error error_code = ExceptionInternal::GetErrorCode(env, error);
9193
std::string error_message = ExceptionInternal::ToString(env, error);
9294
if (error_code != Error::kErrorOk) {
9395
listener->OnEvent({}, error_code, error_message);
9496
return;
9597
}
9698

97-
DocumentSnapshot snapshot(new DocumentSnapshotInternal(firestore, value));
99+
auto snapshot = firestore->NewDocumentSnapshot(env, Object(value));
98100
listener->OnEvent(snapshot, error_code, error_message);
99101
}
100102

101103
/* static */
102-
void EventListenerInternal::QueryEventListenerNativeOnEvent(JNIEnv* env, jclass,
103-
jlong firestore_ptr,
104-
jlong listener_ptr,
105-
jobject value,
106-
jobject error) {
104+
void EventListenerInternal::QueryEventListenerNativeOnEvent(
105+
JNIEnv* raw_env, jclass, jlong firestore_ptr, jlong listener_ptr,
106+
jobject value, jobject raw_error) {
107107
if (firestore_ptr == 0 || listener_ptr == 0) {
108108
return;
109109
}
110110
auto* listener =
111111
reinterpret_cast<EventListener<QuerySnapshot>*>(listener_ptr);
112112
auto* firestore = reinterpret_cast<FirestoreInternal*>(firestore_ptr);
113113

114+
Env env(raw_env);
115+
Object error(raw_error);
114116
Error error_code = ExceptionInternal::GetErrorCode(env, error);
115117
std::string error_message = ExceptionInternal::ToString(env, error);
116118
if (error_code != Error::kErrorOk) {
117119
listener->OnEvent({}, error_code, error_message);
118120
return;
119121
}
120122

121-
QuerySnapshot snapshot(new QuerySnapshotInternal(firestore, value));
123+
auto snapshot = firestore->NewQuerySnapshot(env, Object(value));
122124
listener->OnEvent(snapshot, error_code, error_message);
123125
}
124126

‎firestore/src/android/exception_android.cc

Lines changed: 94 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -1,156 +1,136 @@
11
#include "firestore/src/android/exception_android.h"
22

3-
#include <cstring>
4-
5-
#include "firestore/src/android/util_android.h"
3+
#include "app/src/util_android.h"
4+
#include "firestore/src/jni/env.h"
5+
#include "firestore/src/jni/loader.h"
6+
#include "firestore/src/jni/throwable.h"
67

78
namespace firebase {
89
namespace firestore {
10+
namespace {
11+
12+
using jni::Constructor;
13+
using jni::Env;
14+
using jni::Local;
15+
using jni::Method;
16+
using jni::Object;
17+
using jni::StaticMethod;
18+
using jni::String;
19+
using jni::Throwable;
20+
21+
// FirebaseFirestoreException
22+
constexpr char kFirestoreExceptionClassName[] = PROGUARD_KEEP_CLASS
23+
"com/google/firebase/firestore/FirebaseFirestoreException";
24+
25+
Constructor<Throwable> kNewFirestoreException(
26+
"(Ljava/lang/String;"
27+
"Lcom/google/firebase/firestore/FirebaseFirestoreException$Code;)V");
28+
Method<Object> kGetCode(
29+
"getCode",
30+
"()Lcom/google/firebase/firestore/FirebaseFirestoreException$Code;");
31+
32+
jclass g_firestore_exception_class = nullptr;
33+
34+
// FirebaseFirestoreException$Code
35+
constexpr char kCodeClassName[] = PROGUARD_KEEP_CLASS
36+
"com/google/firebase/firestore/FirebaseFirestoreException$Code";
37+
38+
Method<int32_t> kValue("value", "()I");
39+
StaticMethod<Object> kFromValue(
40+
"fromValue",
41+
"(I)Lcom/google/firebase/firestore/FirebaseFirestoreException$Code;");
942

10-
// clang-format off
11-
#define FIRESTORE_EXCEPTION_METHODS(X) \
12-
X(Constructor, "<init>", \
13-
"(Ljava/lang/String;" \
14-
"Lcom/google/firebase/firestore/FirebaseFirestoreException$Code;)V"), \
15-
X(GetCode, "getCode", \
16-
"()Lcom/google/firebase/firestore/FirebaseFirestoreException$Code;")
17-
// clang-format on
18-
19-
METHOD_LOOKUP_DECLARATION(firestore_exception, FIRESTORE_EXCEPTION_METHODS)
20-
METHOD_LOOKUP_DEFINITION(
21-
firestore_exception,
22-
PROGUARD_KEEP_CLASS
23-
"com/google/firebase/firestore/FirebaseFirestoreException",
24-
FIRESTORE_EXCEPTION_METHODS)
25-
26-
// clang-format off
27-
#define FIRESTORE_EXCEPTION_CODE_METHODS(X) \
28-
X(Value, "value", "()I"), \
29-
X(FromValue, "fromValue", \
30-
"(I)Lcom/google/firebase/firestore/FirebaseFirestoreException$Code;", \
31-
util::kMethodTypeStatic)
32-
// clang-format on
33-
34-
METHOD_LOOKUP_DECLARATION(firestore_exception_code,
35-
FIRESTORE_EXCEPTION_CODE_METHODS)
36-
METHOD_LOOKUP_DEFINITION(
37-
firestore_exception_code,
38-
PROGUARD_KEEP_CLASS
39-
"com/google/firebase/firestore/FirebaseFirestoreException$Code",
40-
FIRESTORE_EXCEPTION_CODE_METHODS)
41-
42-
#define ILLEGAL_STATE_EXCEPTION_METHODS(X) X(Constructor, "<init>", "()V")
43-
44-
METHOD_LOOKUP_DECLARATION(illegal_state_exception,
45-
ILLEGAL_STATE_EXCEPTION_METHODS)
46-
METHOD_LOOKUP_DEFINITION(illegal_state_exception,
47-
PROGUARD_KEEP_CLASS "java/lang/IllegalStateException",
48-
ILLEGAL_STATE_EXCEPTION_METHODS)
43+
// IllegalStateException
44+
constexpr char kIllegalStateExceptionClassName[] =
45+
PROGUARD_KEEP_CLASS "java/lang/IllegalStateException";
46+
Constructor<Throwable> kNewIllegalStateException("()V");
47+
48+
jclass g_illegal_state_exception_class = nullptr;
49+
50+
} // namespace
4951

5052
/* static */
51-
Error ExceptionInternal::GetErrorCode(JNIEnv* env, jobject exception) {
52-
if (exception == nullptr) {
53+
void ExceptionInternal::Initialize(jni::Loader& loader) {
54+
g_firestore_exception_class = loader.LoadClass(
55+
kFirestoreExceptionClassName, kNewFirestoreException, kGetCode);
56+
57+
loader.LoadClass(kCodeClassName, kValue, kFromValue);
58+
59+
g_illegal_state_exception_class = loader.LoadClass(
60+
kIllegalStateExceptionClassName, kNewIllegalStateException);
61+
}
62+
63+
Error ExceptionInternal::GetErrorCode(Env& env, const Object& exception) {
64+
if (!exception) {
5365
return Error::kErrorOk;
5466
}
5567

56-
// Some of the Precondition failure is thrown as IllegalStateException instead
57-
// of a FirebaseFirestoreException. So we convert them into a more meaningful
58-
// code.
59-
if (env->IsInstanceOf(exception, illegal_state_exception::GetClass())) {
68+
if (IsIllegalStateException(env, exception)) {
69+
// Some of the Precondition failure is thrown as IllegalStateException
70+
// instead of a FirebaseFirestoreException. Convert those into a more
71+
// meaningful code.
6072
return Error::kErrorFailedPrecondition;
6173
} else if (!IsFirestoreException(env, exception)) {
6274
return Error::kErrorUnknown;
6375
}
6476

65-
jobject code = env->CallObjectMethod(
66-
exception,
67-
firestore_exception::GetMethodId(firestore_exception::kGetCode));
68-
jint code_value = env->CallIntMethod(
69-
code,
70-
firestore_exception_code::GetMethodId(firestore_exception_code::kValue));
71-
env->DeleteLocalRef(code);
72-
CheckAndClearJniExceptions(env);
73-
74-
if (code_value > Error::kErrorUnauthenticated ||
75-
code_value < Error::kErrorOk) {
77+
Local<Object> java_code = env.Call(exception, kGetCode);
78+
int32_t code = env.Call(java_code, kValue);
79+
80+
if (code > Error::kErrorUnauthenticated || code < Error::kErrorOk) {
7681
return Error::kErrorUnknown;
7782
}
78-
return static_cast<Error>(code_value);
83+
return static_cast<Error>(code);
7984
}
8085

81-
/* static */
82-
std::string ExceptionInternal::ToString(JNIEnv* env, jobject exception) {
83-
return util::GetMessageFromException(env, exception);
86+
std::string ExceptionInternal::ToString(Env& env, const Object& exception) {
87+
return util::GetMessageFromException(env.get(), exception.get());
8488
}
8589

86-
/* static */
87-
jthrowable ExceptionInternal::Create(JNIEnv* env, Error code,
88-
const char* message) {
90+
Local<Throwable> ExceptionInternal::Create(Env& env, Error code,
91+
const std::string& message) {
8992
if (code == Error::kErrorOk) {
90-
return nullptr;
93+
return {};
9194
}
92-
// FirebaseFirestoreException requires message to be non-empty. If the caller
93-
// does not bother to give details, we assign an arbitrary message here.
94-
if (message == nullptr || strlen(message) == 0) {
95-
message = "Unknown Exception";
95+
96+
Local<String> java_message;
97+
if (message.empty()) {
98+
// FirebaseFirestoreException requires message to be non-empty. If the
99+
// caller does not bother to give details, we assign an arbitrary message
100+
// here.
101+
java_message = env.NewStringUtf("Unknown Exception");
102+
} else {
103+
java_message = env.NewStringUtf(message);
96104
}
97105

98-
jstring exception_message = env->NewStringUTF(message);
99-
jobject exception_code =
100-
env->CallStaticObjectMethod(firestore_exception_code::GetClass(),
101-
firestore_exception_code::GetMethodId(
102-
firestore_exception_code::kFromValue),
103-
static_cast<jint>(code));
104-
jthrowable result = static_cast<jthrowable>(env->NewObject(
105-
firestore_exception::GetClass(),
106-
firestore_exception::GetMethodId(firestore_exception::kConstructor),
107-
exception_message, exception_code));
108-
env->DeleteLocalRef(exception_message);
109-
env->DeleteLocalRef(exception_code);
110-
CheckAndClearJniExceptions(env);
111-
return result;
106+
Local<Object> java_code = env.Call(kFromValue, static_cast<int32_t>(code));
107+
return env.New(kNewFirestoreException, java_message, java_code);
112108
}
113109

114-
/* static */
115-
jthrowable ExceptionInternal::Wrap(JNIEnv* env, jthrowable exception) {
110+
Local<Throwable> ExceptionInternal::Wrap(Env& env,
111+
Local<Throwable>&& exception) {
116112
if (IsFirestoreException(env, exception)) {
117-
return static_cast<jthrowable>(env->NewLocalRef(exception));
113+
return Move(exception);
118114
} else {
119115
return Create(env, GetErrorCode(env, exception),
120116
ToString(env, exception).c_str());
121117
}
122118
}
123119

124-
/* static */
125-
bool ExceptionInternal::IsFirestoreException(JNIEnv* env, jobject exception) {
126-
return env->IsInstanceOf(exception, firestore_exception::GetClass());
120+
bool ExceptionInternal::IsFirestoreException(Env& env,
121+
const Object& exception) {
122+
return env.IsInstanceOf(exception, g_firestore_exception_class);
127123
}
128124

129-
/* static */
130-
bool ExceptionInternal::IsAnyExceptionThrownByFirestore(JNIEnv* env,
131-
jobject exception) {
132-
return IsFirestoreException(env, exception) ||
133-
env->IsInstanceOf(exception, illegal_state_exception::GetClass());
134-
}
135-
136-
/* static */
137-
bool ExceptionInternal::Initialize(App* app) {
138-
JNIEnv* env = app->GetJNIEnv();
139-
jobject activity = app->activity();
140-
bool result = firestore_exception::CacheMethodIds(env, activity) &&
141-
firestore_exception_code::CacheMethodIds(env, activity) &&
142-
illegal_state_exception::CacheMethodIds(env, activity);
143-
util::CheckAndClearJniExceptions(env);
144-
return result;
125+
bool ExceptionInternal::IsIllegalStateException(Env& env,
126+
const Object& exception) {
127+
return env.IsInstanceOf(exception, g_illegal_state_exception_class);
145128
}
146129

147-
/* static */
148-
void ExceptionInternal::Terminate(App* app) {
149-
JNIEnv* env = app->GetJNIEnv();
150-
firestore_exception::ReleaseClass(env);
151-
firestore_exception_code::ReleaseClass(env);
152-
illegal_state_exception::ReleaseClass(env);
153-
util::CheckAndClearJniExceptions(env);
130+
bool ExceptionInternal::IsAnyExceptionThrownByFirestore(
131+
Env& env, const Object& exception) {
132+
return IsFirestoreException(env, exception) ||
133+
IsIllegalStateException(env, exception);
154134
}
155135

156136
} // namespace firestore

‎firestore/src/android/exception_android.h

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,34 +4,37 @@
44
#include <string>
55

66
#include "app/src/include/firebase/app.h"
7-
#include "app/src/util_android.h"
7+
#include "firestore/src/jni/jni_fwd.h"
88
#include "firebase/firestore/firestore_errors.h"
99

1010
namespace firebase {
1111
namespace firestore {
1212

1313
class ExceptionInternal {
1414
public:
15-
static Error GetErrorCode(JNIEnv* env, jobject exception);
16-
static std::string ToString(JNIEnv* env, jobject exception);
15+
static void Initialize(jni::Loader& loader);
1716

18-
static jthrowable Create(JNIEnv* env, Error code, const char* message);
19-
static jthrowable Wrap(JNIEnv* env, jthrowable exception);
17+
static Error GetErrorCode(jni::Env& env, const jni::Object& exception);
18+
static std::string ToString(jni::Env& env, const jni::Object& exception);
19+
20+
static jni::Local<jni::Throwable> Create(jni::Env& env, Error code,
21+
const std::string& message);
22+
static jni::Local<jni::Throwable> Wrap(
23+
jni::Env& env, jni::Local<jni::Throwable>&& exception);
2024

2125
/** Returns true if the given object is a FirestoreException. */
22-
static bool IsFirestoreException(JNIEnv* env, jobject exception);
26+
static bool IsFirestoreException(jni::Env& env, const jni::Object& exception);
27+
28+
/** Returns true if the given object is an IllegalStateException. */
29+
static bool IsIllegalStateException(jni::Env& env,
30+
const jni::Object& exception);
2331

2432
/**
2533
* Returns true if the given object is a FirestoreException or any other type
2634
* of exception thrown by a Firestore API.
2735
*/
28-
static bool IsAnyExceptionThrownByFirestore(JNIEnv* env, jobject exception);
29-
30-
private:
31-
friend class FirestoreInternal;
32-
33-
static bool Initialize(App* app);
34-
static void Terminate(App* app);
36+
static bool IsAnyExceptionThrownByFirestore(jni::Env& env,
37+
const jni::Object& exception);
3538
};
3639

3740
} // namespace firestore

‎firestore/src/android/firestore_android.cc

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ bool FirestoreInternal::Initialize(App* app) {
175175

176176
if (!( // Call Initialize on each Firestore internal class.
177177
FieldValueInternal::Initialize(app) &&
178-
ExceptionInternal::Initialize(app) &&
179178
TransactionInternal::Initialize(app) && Wrapper::Initialize(app) &&
180179
// Initialize those embedded Firestore internal classes.
181180
InitializeEmbeddedClasses(app, loader))) {
@@ -202,6 +201,7 @@ bool FirestoreInternal::Initialize(App* app) {
202201
DocumentReferenceInternal::Initialize(loader);
203202
DocumentSnapshotInternal::Initialize(loader);
204203
EventListenerInternal::Initialize(loader);
204+
ExceptionInternal::Initialize(loader);
205205
FieldPathConverter::Initialize(loader);
206206
GeoPointInternal::Initialize(loader);
207207
ListenerRegistrationInternal::Initialize(loader);
@@ -242,7 +242,6 @@ void FirestoreInternal::ReleaseClasses(App* app) {
242242

243243
// Call Terminate on each Firestore internal class.
244244
FieldValueInternal::Terminate(app);
245-
ExceptionInternal::Terminate(app);
246245
TransactionInternal::Terminate(app);
247246
Wrapper::Terminate(app);
248247
}
@@ -465,6 +464,13 @@ Query FirestoreInternal::NewQuery(jni::Env& env,
465464
return Query(new QueryInternal(mutable_this(), query.get()));
466465
}
467466

467+
QuerySnapshot FirestoreInternal::NewQuerySnapshot(
468+
jni::Env& env, const jni::Object& snapshot) const {
469+
if (!env.ok() || !snapshot) return {};
470+
return QuerySnapshot(
471+
new QuerySnapshotInternal(mutable_this(), snapshot.get()));
472+
}
473+
468474
/* static */
469475
void Firestore::set_log_level(LogLevel log_level) {
470476
// "Verbose" and "debug" map to logging enabled.

0 commit comments

Comments
 (0)