Convert Timestamp to the new JNI framework

PiperOrigin-RevId: 329814232

Author: wilhuff

firestore/src/android/field_value_android.cc

@@ -99,10 +99,9 @@ FieldValueInternal::FieldValueInternal(double value)
 
 FieldValueInternal::FieldValueInternal(Timestamp value)
     : cached_type_(Type::kTimestamp) {
-  JNIEnv* env = firestore_->app()->GetJNIEnv();
-  jobject obj = TimestampInternal::TimestampToJavaTimestamp(env, value);
-  obj_ = env->NewGlobalRef(obj);
-  env->DeleteLocalRef(obj);
+  Env env;
+  Local<TimestampInternal> obj = TimestampInternal::Create(env, value);
+  obj_ = env.get()->NewGlobalRef(obj.get());
 }
 
 FieldValueInternal::FieldValueInternal(std::string value)
@@ -194,7 +193,7 @@ Type FieldValueInternal::type() const {
     cached_type_ = Type::kDouble;
     return Type::kDouble;
   }
-  if (env->IsInstanceOf(obj_, TimestampInternal::GetClass())) {
+  if (env->IsInstanceOf(obj_, TimestampInternal::GetClass().get())) {
     cached_type_ = Type::kTimestamp;
     return Type::kTimestamp;
   }
@@ -271,17 +270,17 @@ double FieldValueInternal::double_value() const {
 }
 
 Timestamp FieldValueInternal::timestamp_value() const {
-  JNIEnv* env = firestore_->app()->GetJNIEnv();
+  Env env;
 
   // Make sure this instance is of correct type.
   if (cached_type_ == Type::kNull) {
-    FIREBASE_ASSERT(env->IsInstanceOf(obj_, TimestampInternal::GetClass()));
+    FIREBASE_ASSERT(env.IsInstanceOf(obj_, TimestampInternal::GetClass()));
     cached_type_ = Type::kTimestamp;
   } else {
     FIREBASE_ASSERT(cached_type_ == Type::kTimestamp);
   }
 
-  return TimestampInternal::JavaTimestampToTimestamp(env, obj_);
+  return TimestampInternal(obj_).ToPublic(env);
 }
 
 std::string FieldValueInternal::string_value() const {

firestore/src/android/firestore_android.cc

@@ -170,7 +170,6 @@ bool FirestoreInternal::Initialize(App* app) {
           ListenerRegistrationInternal::Initialize(app) &&
           QueryInternal::Initialize(app) &&
           QuerySnapshotInternal::Initialize(app) &&
-          TimestampInternal::Initialize(app) &&
           TransactionInternal::Initialize(app) && Wrapper::Initialize(app) &&
           // Initialize those embedded Firestore internal classes.
           InitializeEmbeddedClasses(app))) {
@@ -204,6 +203,7 @@ bool FirestoreInternal::Initialize(App* app) {
     SettingsInternal::Initialize(loader);
     SnapshotMetadataInternal::Initialize(loader);
     SourceInternal::Initialize(loader);
+    TimestampInternal::Initialize(loader);
     WriteBatchInternal::Initialize(loader);
     if (!loader.ok()) {
       ReleaseClasses(app);
@@ -258,7 +258,6 @@ void FirestoreInternal::ReleaseClasses(App* app) {
   ListenerRegistrationInternal::Terminate(app);
   QueryInternal::Terminate(app);
   QuerySnapshotInternal::Terminate(app);
-  TimestampInternal::Terminate(app);
   TransactionInternal::Terminate(app);
   Wrapper::Terminate(app);
 }

firestore/src/android/timestamp_android.cc

@@ -1,65 +1,44 @@
 #include "firestore/src/android/timestamp_android.h"
 
-#include <stdint.h>
-
-#include "app/src/util_android.h"
-#include "firestore/src/android/util_android.h"
+#include "firestore/src/jni/env.h"
+#include "firestore/src/jni/loader.h"
 
 namespace firebase {
 namespace firestore {
+namespace {
 
-// clang-format off
-#define TIMESTAMP_METHODS(X)                                     \
-  X(Constructor, "<init>", "(JI)V", util::kMethodTypeInstance),  \
-  X(GetSeconds, "getSeconds", "()J"),                            \
-  X(GetNanoseconds, "getNanoseconds", "()I")
-// clang-format on
+using jni::Class;
+using jni::Constructor;
+using jni::Env;
+using jni::Local;
+using jni::Method;
 
-METHOD_LOOKUP_DECLARATION(timestamp, TIMESTAMP_METHODS)
-METHOD_LOOKUP_DEFINITION(timestamp,
-                         PROGUARD_KEEP_CLASS "com/google/firebase/Timestamp",
-                         TIMESTAMP_METHODS)
+constexpr char kClassName[] =
+    PROGUARD_KEEP_CLASS "com/google/firebase/Timestamp";
+Constructor<TimestampInternal> kConstructor("(JI)V");
+Method<int64_t> kGetSeconds("getSeconds", "()J");
+Method<int32_t> kGetNanoseconds("getNanoseconds", "()I");
 
-/* static */
-jobject TimestampInternal::TimestampToJavaTimestamp(
-    JNIEnv* env, const Timestamp& timestamp) {
-  jobject result = env->NewObject(
-      timestamp::GetClass(), timestamp::GetMethodId(timestamp::kConstructor),
-      static_cast<jlong>(timestamp.seconds()),
-      static_cast<jint>(timestamp.nanoseconds()));
-  CheckAndClearJniExceptions(env);
-  return result;
-}
+jclass g_clazz = nullptr;
+
+}  // namespace
 
-/* static */
-Timestamp TimestampInternal::JavaTimestampToTimestamp(JNIEnv* env,
-                                                      jobject obj) {
-  jlong seconds =
-      env->CallLongMethod(obj, timestamp::GetMethodId(timestamp::kGetSeconds));
-  jint nanoseconds = env->CallIntMethod(
-      obj, timestamp::GetMethodId(timestamp::kGetNanoseconds));
-  CheckAndClearJniExceptions(env);
-  return Timestamp{static_cast<int64_t>(seconds),
-                   static_cast<int32_t>(nanoseconds)};
+void TimestampInternal::Initialize(jni::Loader& loader) {
+  g_clazz =
+      loader.LoadClass(kClassName, kConstructor, kGetSeconds, kGetNanoseconds);
 }
 
-/* static */
-jclass TimestampInternal::GetClass() { return timestamp::GetClass(); }
+Class TimestampInternal::GetClass() { return Class(g_clazz); }
 
-/* static */
-bool TimestampInternal::Initialize(App* app) {
-  JNIEnv* env = app->GetJNIEnv();
-  jobject activity = app->activity();
-  bool result = timestamp::CacheMethodIds(env, activity);
-  util::CheckAndClearJniExceptions(env);
-  return result;
+Local<TimestampInternal> TimestampInternal::Create(Env& env,
+                                                   const Timestamp& timestamp) {
+  return env.New(kConstructor, timestamp.seconds(), timestamp.nanoseconds());
 }
 
-/* static */
-void TimestampInternal::Terminate(App* app) {
-  JNIEnv* env = app->GetJNIEnv();
-  timestamp::ReleaseClass(env);
-  util::CheckAndClearJniExceptions(env);
+Timestamp TimestampInternal::ToPublic(Env& env) const {
+  int64_t seconds = env.Call(*this, kGetSeconds);
+  int32_t nanoseconds = env.Call(*this, kGetNanoseconds);
+  return Timestamp(seconds, nanoseconds);
 }
 
 }  // namespace firestore

firestore/src/android/timestamp_android.h

@@ -1,38 +1,30 @@
 #ifndef FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_TIMESTAMP_ANDROID_H_
 #define FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_TIMESTAMP_ANDROID_H_
 
-#include <jni.h>
-
-#include "app/src/include/firebase/app.h"
+#include "firestore/src/jni/jni_fwd.h"
+#include "firestore/src/jni/object.h"
 #include "firebase/firestore/timestamp.h"
 
 namespace firebase {
 namespace firestore {
 
-// This is the non-wrapper Android implementation of Timestamp. Since Timestamp
-// has most of their method inlined, we use it directly instead of wrapping
-// around a Java Timestamp object. We still need the helper functions to convert
-// between the two types. In addition, we also need proper initializer and
-// terminator for the Java class cache/uncache.
-class TimestampInternal {
+/** A C++ proxy for a Java `Timestamp`. */
+class TimestampInternal : public jni::Object {
  public:
   using ApiType = Timestamp;
 
-  // Convert a C++ Timestamp into a Java Timestamp.
-  static jobject TimestampToJavaTimestamp(JNIEnv* env,
-                                          const Timestamp& timestamp);
+  using jni::Object::Object;
 
-  // Convert a Java Timestamp into a C++ Timestamp.
-  static Timestamp JavaTimestampToTimestamp(JNIEnv* env, jobject obj);
+  static void Initialize(jni::Loader& loader);
 
-  // Gets the class object of Java Timestamp class.
-  static jclass GetClass();
+  static jni::Class GetClass();
 
- private:
-  friend class FirestoreInternal;
+  /** Creates a C++ proxy for a Java `Timestamp` object. */
+  static jni::Local<TimestampInternal> Create(jni::Env& env,
+                                              const Timestamp& timestamp);
 
-  static bool Initialize(App* app);
-  static void Terminate(App* app);
+  /** Converts a Java `Timestamp` into a public C++ `Timestamp`. */
+  Timestamp ToPublic(jni::Env& env) const;
 };
 
 }  // namespace firestore

firestore/src/jni/env.h

@@ -158,6 +158,9 @@ class Env {
   Local<Class> GetObjectClass(const Object& object);
 
   bool IsInstanceOf(const Object& object, const Class& clazz);
+  bool IsInstanceOf(jobject object, const Class& clazz) {
+    return IsInstanceOf(Object(object), clazz);
+  }
 
   bool IsSameObject(const Object& object1, const Object& object2);
 

firestore/src/jni/loader.h

@@ -91,9 +91,10 @@ class Loader {
    * Loads a Java class and all its members in a single invocation.
    */
   template <typename... Members>
-  void LoadClass(const char* name, Members&&... members) {
-    LoadClass(name);
+  jclass LoadClass(const char* name, Members&&... members) {
+    jclass result = LoadClass(name);
     LoadAll(Forward<Members>(members)...);
+    return result;
   }
 
   /**

firestore/src/tests/android/timestamp_android_test.cc

@@ -1,26 +1,26 @@
 #include "firestore/src/android/timestamp_android.h"
 
-#include <jni.h>
-
+#include "firestore/src/jni/env.h"
 #include "firestore/src/tests/firestore_integration_test.h"
-#include "testing/base/public/gmock.h"
 #include "gtest/gtest.h"
 #include "firebase/firestore/timestamp.h"
 
 namespace firebase {
 namespace firestore {
+namespace {
+
+using jni::Env;
 
-TEST_F(FirestoreIntegrationTest, Converter) {
-  JNIEnv* env = app()->GetJNIEnv();
+using TimestampTest = FirestoreIntegrationTest;
 
-  const Timestamp timestamp{1234, 5678};
-  jobject java_timestamp =
-      TimestampInternal::TimestampToJavaTimestamp(env, timestamp);
-  EXPECT_EQ(timestamp,
-            TimestampInternal::JavaTimestampToTimestamp(env, java_timestamp));
+TEST_F(TimestampTest, Converts) {
+  Env env;
 
-  env->DeleteLocalRef(java_timestamp);
+  Timestamp timestamp{1234, 5678};
+  auto java_timestamp = TimestampInternal::Create(env, timestamp);
+  EXPECT_EQ(timestamp, java_timestamp.ToPublic(env));
 }
 
+}  // namespace
 }  // namespace firestore
 }  // namespace firebase