Skip to content

Commit ee4fa72

Browse files
wilhuffa-maurice
authored andcommitted
Clean up exception naming
Rename FirebaseFirestoreExceptionInternal to ExceptionInternal. Rename methods in ExceptionInternal to better reflect their function. PiperOrigin-RevId: 334219916
1 parent d3fe671 commit ee4fa72

8 files changed

+75
-80
lines changed

firestore/CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,14 @@ set(android_SRCS
7171
src/android/document_snapshot_android.h
7272
src/android/event_listener_android.cc
7373
src/android/event_listener_android.h
74+
src/android/exception_android.cc
75+
src/android/exception_android.h
7476
src/android/field_path_android.cc
7577
src/android/field_path_android.h
7678
src/android/field_path_portable.cc
7779
src/android/field_path_portable.h
7880
src/android/field_value_android.cc
7981
src/android/field_value_android.h
80-
src/android/firebase_firestore_exception_android.cc
81-
src/android/firebase_firestore_exception_android.h
8282
src/android/firestore_android.cc
8383
src/android/firestore_android.h
8484
src/android/geo_point_android.cc

firestore/src/android/event_listener_android.cc

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include "app/src/include/firebase/internal/common.h"
88
#include "app/src/util_android.h"
99
#include "firestore/src/android/document_snapshot_android.h"
10-
#include "firestore/src/android/firebase_firestore_exception_android.h"
10+
#include "firestore/src/android/exception_android.h"
1111
#include "firestore/src/android/query_snapshot_android.h"
1212
#include "firestore/src/android/util_android.h"
1313
#include "firestore/src/common/util.h"
@@ -87,10 +87,8 @@ void EventListenerInternal::DocumentEventListenerNativeOnEvent(
8787
reinterpret_cast<EventListener<DocumentSnapshot>*>(listener_ptr);
8888
auto* firestore = reinterpret_cast<FirestoreInternal*>(firestore_ptr);
8989

90-
Error error_code =
91-
FirebaseFirestoreExceptionInternal::ToErrorCode(env, error);
92-
std::string error_message =
93-
FirebaseFirestoreExceptionInternal::ToString(env, error);
90+
Error error_code = ExceptionInternal::GetErrorCode(env, error);
91+
std::string error_message = ExceptionInternal::ToString(env, error);
9492
if (error_code != Error::kErrorOk) {
9593
listener->OnEvent({}, error_code, error_message);
9694
return;
@@ -113,10 +111,8 @@ void EventListenerInternal::QueryEventListenerNativeOnEvent(JNIEnv* env, jclass,
113111
reinterpret_cast<EventListener<QuerySnapshot>*>(listener_ptr);
114112
auto* firestore = reinterpret_cast<FirestoreInternal*>(firestore_ptr);
115113

116-
Error error_code =
117-
FirebaseFirestoreExceptionInternal::ToErrorCode(env, error);
118-
std::string error_message =
119-
FirebaseFirestoreExceptionInternal::ToString(env, error);
114+
Error error_code = ExceptionInternal::GetErrorCode(env, error);
115+
std::string error_message = ExceptionInternal::ToString(env, error);
120116
if (error_code != Error::kErrorOk) {
121117
listener->OnEvent({}, error_code, error_message);
122118
return;

firestore/src/android/firebase_firestore_exception_android.cc renamed to firestore/src/android/exception_android.cc

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#include "firestore/src/android/firebase_firestore_exception_android.h"
1+
#include "firestore/src/android/exception_android.h"
22

33
#include <cstring>
44

@@ -48,8 +48,7 @@ METHOD_LOOKUP_DEFINITION(illegal_state_exception,
4848
ILLEGAL_STATE_EXCEPTION_METHODS)
4949

5050
/* static */
51-
Error FirebaseFirestoreExceptionInternal::ToErrorCode(JNIEnv* env,
52-
jobject exception) {
51+
Error ExceptionInternal::GetErrorCode(JNIEnv* env, jobject exception) {
5352
if (exception == nullptr) {
5453
return Error::kErrorOk;
5554
}
@@ -59,7 +58,7 @@ Error FirebaseFirestoreExceptionInternal::ToErrorCode(JNIEnv* env,
5958
// code.
6059
if (env->IsInstanceOf(exception, illegal_state_exception::GetClass())) {
6160
return Error::kErrorFailedPrecondition;
62-
} else if (!IsInstance(env, exception)) {
61+
} else if (!IsFirestoreException(env, exception)) {
6362
return Error::kErrorUnknown;
6463
}
6564

@@ -80,14 +79,13 @@ Error FirebaseFirestoreExceptionInternal::ToErrorCode(JNIEnv* env,
8079
}
8180

8281
/* static */
83-
std::string FirebaseFirestoreExceptionInternal::ToString(JNIEnv* env,
84-
jobject exception) {
82+
std::string ExceptionInternal::ToString(JNIEnv* env, jobject exception) {
8583
return util::GetMessageFromException(env, exception);
8684
}
8785

8886
/* static */
89-
jthrowable FirebaseFirestoreExceptionInternal::ToException(
90-
JNIEnv* env, Error code, const char* message) {
87+
jthrowable ExceptionInternal::Create(JNIEnv* env, Error code,
88+
const char* message) {
9189
if (code == Error::kErrorOk) {
9290
return nullptr;
9391
}
@@ -114,31 +112,29 @@ jthrowable FirebaseFirestoreExceptionInternal::ToException(
114112
}
115113

116114
/* static */
117-
jthrowable FirebaseFirestoreExceptionInternal::ToException(
118-
JNIEnv* env, jthrowable exception) {
119-
if (IsInstance(env, exception)) {
115+
jthrowable ExceptionInternal::Wrap(JNIEnv* env, jthrowable exception) {
116+
if (IsFirestoreException(env, exception)) {
120117
return static_cast<jthrowable>(env->NewLocalRef(exception));
121118
} else {
122-
return ToException(env, ToErrorCode(env, exception),
123-
ToString(env, exception).c_str());
119+
return Create(env, GetErrorCode(env, exception),
120+
ToString(env, exception).c_str());
124121
}
125122
}
126123

127124
/* static */
128-
bool FirebaseFirestoreExceptionInternal::IsInstance(JNIEnv* env,
129-
jobject exception) {
125+
bool ExceptionInternal::IsFirestoreException(JNIEnv* env, jobject exception) {
130126
return env->IsInstanceOf(exception, firestore_exception::GetClass());
131127
}
132128

133129
/* static */
134-
bool FirebaseFirestoreExceptionInternal::IsFirestoreException(
135-
JNIEnv* env, jobject exception) {
136-
return IsInstance(env, exception) ||
130+
bool ExceptionInternal::IsAnyExceptionThrownByFirestore(JNIEnv* env,
131+
jobject exception) {
132+
return IsFirestoreException(env, exception) ||
137133
env->IsInstanceOf(exception, illegal_state_exception::GetClass());
138134
}
139135

140136
/* static */
141-
bool FirebaseFirestoreExceptionInternal::Initialize(App* app) {
137+
bool ExceptionInternal::Initialize(App* app) {
142138
JNIEnv* env = app->GetJNIEnv();
143139
jobject activity = app->activity();
144140
bool result = firestore_exception::CacheMethodIds(env, activity) &&
@@ -149,7 +145,7 @@ bool FirebaseFirestoreExceptionInternal::Initialize(App* app) {
149145
}
150146

151147
/* static */
152-
void FirebaseFirestoreExceptionInternal::Terminate(App* app) {
148+
void ExceptionInternal::Terminate(App* app) {
153149
JNIEnv* env = app->GetJNIEnv();
154150
firestore_exception::ReleaseClass(env);
155151
firestore_exception_code::ReleaseClass(env);
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
#ifndef FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_EXCEPTION_ANDROID_H_
2+
#define FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_EXCEPTION_ANDROID_H_
3+
4+
#include <string>
5+
6+
#include "app/src/include/firebase/app.h"
7+
#include "app/src/util_android.h"
8+
#include "firebase/firestore/firestore_errors.h"
9+
10+
namespace firebase {
11+
namespace firestore {
12+
13+
class ExceptionInternal {
14+
public:
15+
static Error GetErrorCode(JNIEnv* env, jobject exception);
16+
static std::string ToString(JNIEnv* env, jobject exception);
17+
18+
static jthrowable Create(JNIEnv* env, Error code, const char* message);
19+
static jthrowable Wrap(JNIEnv* env, jthrowable exception);
20+
21+
/** Returns true if the given object is a FirestoreException. */
22+
static bool IsFirestoreException(JNIEnv* env, jobject exception);
23+
24+
/**
25+
* Returns true if the given object is a FirestoreException or any other type
26+
* of exception thrown by a Firestore API.
27+
*/
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);
35+
};
36+
37+
} // namespace firestore
38+
} // namespace firebase
39+
40+
#endif // FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_EXCEPTION_ANDROID_H_

firestore/src/android/firebase_firestore_exception_android.h

Lines changed: 0 additions & 32 deletions
This file was deleted.

firestore/src/android/firestore_android.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717
#include "firestore/src/android/document_reference_android.h"
1818
#include "firestore/src/android/document_snapshot_android.h"
1919
#include "firestore/src/android/event_listener_android.h"
20+
#include "firestore/src/android/exception_android.h"
2021
#include "firestore/src/android/field_path_android.h"
2122
#include "firestore/src/android/field_value_android.h"
22-
#include "firestore/src/android/firebase_firestore_exception_android.h"
2323
#include "firestore/src/android/geo_point_android.h"
2424
#include "firestore/src/android/lambda_event_listener.h"
2525
#include "firestore/src/android/lambda_transaction_function.h"
@@ -175,7 +175,7 @@ bool FirestoreInternal::Initialize(App* app) {
175175

176176
if (!( // Call Initialize on each Firestore internal class.
177177
FieldValueInternal::Initialize(app) &&
178-
FirebaseFirestoreExceptionInternal::Initialize(app) &&
178+
ExceptionInternal::Initialize(app) &&
179179
TransactionInternal::Initialize(app) && Wrapper::Initialize(app) &&
180180
// Initialize those embedded Firestore internal classes.
181181
InitializeEmbeddedClasses(app, loader))) {
@@ -242,7 +242,7 @@ void FirestoreInternal::ReleaseClasses(App* app) {
242242

243243
// Call Terminate on each Firestore internal class.
244244
FieldValueInternal::Terminate(app);
245-
FirebaseFirestoreExceptionInternal::Terminate(app);
245+
ExceptionInternal::Terminate(app);
246246
TransactionInternal::Terminate(app);
247247
Wrapper::Terminate(app);
248248
}

firestore/src/android/promise_android.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include "app/src/reference_counted_future_impl.h"
88
#include "app/src/util_android.h"
99
#include "firestore/src/android/document_snapshot_android.h"
10-
#include "firestore/src/android/firebase_firestore_exception_android.h"
10+
#include "firestore/src/android/exception_android.h"
1111
#include "firestore/src/android/firestore_android.h"
1212
#include "firestore/src/android/query_snapshot_android.h"
1313
#include "firestore/src/jni/env.h"
@@ -115,7 +115,7 @@ class Promise {
115115
switch (result_code) {
116116
case util::kFutureResultFailure:
117117
// When failed, result is the exception raised.
118-
error_code = FirebaseFirestoreExceptionInternal::ToErrorCode(
118+
error_code = ExceptionInternal::GetErrorCode(
119119
this->firestore_->app()->GetJNIEnv(), result);
120120
break;
121121
case util::kFutureResultCancelled:

firestore/src/android/transaction_android.cc

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@
88
#include "app/src/include/firebase/internal/common.h"
99
#include "app/src/util_android.h"
1010
#include "firestore/src/android/document_reference_android.h"
11+
#include "firestore/src/android/exception_android.h"
1112
#include "firestore/src/android/field_path_android.h"
1213
#include "firestore/src/android/field_value_android.h"
13-
#include "firestore/src/android/firebase_firestore_exception_android.h"
1414
#include "firestore/src/android/set_options_android.h"
1515
#include "firestore/src/jni/env.h"
1616
#include "firestore/src/jni/hash_map.h"
@@ -114,15 +114,13 @@ DocumentSnapshot TransactionInternal::Get(const DocumentReference& document,
114114
util::CheckAndClearJniExceptions(env);
115115
if (exception != nullptr) {
116116
if (error_code != nullptr) {
117-
*error_code =
118-
FirebaseFirestoreExceptionInternal::ToErrorCode(env, exception);
117+
*error_code = ExceptionInternal::GetErrorCode(env, exception);
119118
}
120119
if (error_message != nullptr) {
121-
*error_message =
122-
FirebaseFirestoreExceptionInternal::ToString(env, exception);
120+
*error_message = ExceptionInternal::ToString(env, exception);
123121
}
124122

125-
if (!FirebaseFirestoreExceptionInternal::IsInstance(env, exception)) {
123+
if (!ExceptionInternal::IsFirestoreException(env, exception)) {
126124
// We would only preserve exception if it is not
127125
// FirebaseFirestoreException. The user should decide whether to raise the
128126
// error or let the transaction succeed.
@@ -163,10 +161,8 @@ void TransactionInternal::PreserveException(jthrowable exception) {
163161
}
164162

165163
JNIEnv* env = firestore_->app()->GetJNIEnv();
166-
if (FirebaseFirestoreExceptionInternal::IsFirestoreException(env,
167-
exception)) {
168-
jobject firestore_exception =
169-
FirebaseFirestoreExceptionInternal::ToException(env, exception);
164+
if (ExceptionInternal::IsAnyExceptionThrownByFirestore(env, exception)) {
165+
jobject firestore_exception = ExceptionInternal::Wrap(env, exception);
170166
*first_exception_ =
171167
static_cast<jthrowable>(env->NewGlobalRef(firestore_exception));
172168
env->DeleteLocalRef(firestore_exception);
@@ -234,8 +230,7 @@ jobject TransactionInternal::TransactionFunctionNativeApply(
234230
cpp_transaction.internal_->ClearException();
235231
return first_exception;
236232
} else {
237-
return FirebaseFirestoreExceptionInternal::ToException(env, code,
238-
message.c_str());
233+
return ExceptionInternal::Create(env, code, message.c_str());
239234
}
240235
}
241236

0 commit comments

Comments
 (0)