From 0dc5c5f1f43701f396d2eb49bbabd2bfd2338cf4 Mon Sep 17 00:00:00 2001 From: Ryan Linton Date: Mon, 4 Mar 2024 19:07:53 -0800 Subject: [PATCH] Update bridge to handle long values (#43158) Summary: This adds support for 64 bit integer (long) values to the Android bridge. Per the wide gamut color [RFC](https://github.com/react-native-community/discussions-and-proposals/pull/738) Android encodes wide gamut colors as long values so we need to update the bridge to support 64 bit integers as well since these classes will soon receive those values from native. ## Changelog: [ANDROID] [ADDED] - Update bridge to handle long values Pull Request resolved: https://github.com/facebook/react-native/pull/43158 Test Plan: I added tests where I could for long types and truncation. I would like to add tests for ReadableNativeArray and ReadableNativeMap but I'm not sure how to go about mocking HybridData. Reviewed By: cipolleschi Differential Revision: D54276496 Pulled By: NickGerleman fbshipit-source-id: 1e71b5283f662748beef1bdb34d9c86099baecb0 --- .../ReactAndroid/api/ReactAndroid.api | 12 ++++ .../facebook/react/bridge/JavaOnlyArray.java | 15 ++++- .../facebook/react/bridge/JavaOnlyMap.java | 12 ++++ .../facebook/react/bridge/ReadableArray.java | 2 + .../facebook/react/bridge/ReadableMap.java | 2 + .../react/bridge/ReadableNativeArray.java | 5 ++ .../react/bridge/ReadableNativeMap.java | 5 ++ .../facebook/react/bridge/WritableArray.java | 2 + .../facebook/react/bridge/WritableMap.java | 2 + .../react/bridge/WritableNativeArray.java | 3 + .../react/bridge/WritableNativeMap.java | 3 + .../react/bridge/JavaOnlyArrayTest.kt | 20 +++++-- .../facebook/react/bridge/JavaOnlyMapTest.kt | 55 +++++++++++++++++++ 13 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyMapTest.kt diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index d1144df5d315b5..d2437b2c8a25a4 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -823,6 +823,7 @@ public class com/facebook/react/bridge/JavaOnlyArray : com/facebook/react/bridge public fun getDouble (I)D public fun getDynamic (I)Lcom/facebook/react/bridge/Dynamic; public fun getInt (I)I + public fun getLong (I)J public fun getMap (I)Lcom/facebook/react/bridge/ReadableMap; public fun getString (I)Ljava/lang/String; public fun getType (I)Lcom/facebook/react/bridge/ReadableType; @@ -833,6 +834,7 @@ public class com/facebook/react/bridge/JavaOnlyArray : com/facebook/react/bridge public fun pushBoolean (Z)V public fun pushDouble (D)V public fun pushInt (I)V + public fun pushLong (J)V public fun pushMap (Lcom/facebook/react/bridge/ReadableMap;)V public fun pushNull ()V public fun pushString (Ljava/lang/String;)V @@ -853,6 +855,7 @@ public class com/facebook/react/bridge/JavaOnlyMap : com/facebook/react/bridge/R public fun getDynamic (Ljava/lang/String;)Lcom/facebook/react/bridge/Dynamic; public fun getEntryIterator ()Ljava/util/Iterator; public fun getInt (Ljava/lang/String;)I + public fun getLong (Ljava/lang/String;)J public fun getMap (Ljava/lang/String;)Lcom/facebook/react/bridge/ReadableMap; public fun getString (Ljava/lang/String;)Ljava/lang/String; public fun getType (Ljava/lang/String;)Lcom/facebook/react/bridge/ReadableType; @@ -866,6 +869,7 @@ public class com/facebook/react/bridge/JavaOnlyMap : com/facebook/react/bridge/R public fun putBoolean (Ljava/lang/String;Z)V public fun putDouble (Ljava/lang/String;D)V public fun putInt (Ljava/lang/String;I)V + public fun putLong (Ljava/lang/String;J)V public fun putMap (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableMap;)V public fun putNull (Ljava/lang/String;)V public fun putString (Ljava/lang/String;Ljava/lang/String;)V @@ -1326,6 +1330,7 @@ public abstract interface class com/facebook/react/bridge/ReadableArray { public abstract fun getDouble (I)D public abstract fun getDynamic (I)Lcom/facebook/react/bridge/Dynamic; public abstract fun getInt (I)I + public abstract fun getLong (I)J public abstract fun getMap (I)Lcom/facebook/react/bridge/ReadableMap; public abstract fun getString (I)Ljava/lang/String; public abstract fun getType (I)Lcom/facebook/react/bridge/ReadableType; @@ -1341,6 +1346,7 @@ public abstract interface class com/facebook/react/bridge/ReadableMap { public abstract fun getDynamic (Ljava/lang/String;)Lcom/facebook/react/bridge/Dynamic; public abstract fun getEntryIterator ()Ljava/util/Iterator; public abstract fun getInt (Ljava/lang/String;)I + public abstract fun getLong (Ljava/lang/String;)J public abstract fun getMap (Ljava/lang/String;)Lcom/facebook/react/bridge/ReadableMap; public abstract fun getString (Ljava/lang/String;)Ljava/lang/String; public abstract fun getType (Ljava/lang/String;)Lcom/facebook/react/bridge/ReadableType; @@ -1365,6 +1371,7 @@ public class com/facebook/react/bridge/ReadableNativeArray : com/facebook/react/ public fun getDynamic (I)Lcom/facebook/react/bridge/Dynamic; public fun getInt (I)I public static fun getJNIPassCounter ()I + public fun getLong (I)J public synthetic fun getMap (I)Lcom/facebook/react/bridge/ReadableMap; public fun getMap (I)Lcom/facebook/react/bridge/ReadableNativeMap; public fun getString (I)Ljava/lang/String; @@ -1385,6 +1392,7 @@ public class com/facebook/react/bridge/ReadableNativeMap : com/facebook/react/br public fun getEntryIterator ()Ljava/util/Iterator; public fun getInt (Ljava/lang/String;)I public static fun getJNIPassCounter ()I + public fun getLong (Ljava/lang/String;)J public synthetic fun getMap (Ljava/lang/String;)Lcom/facebook/react/bridge/ReadableMap; public fun getMap (Ljava/lang/String;)Lcom/facebook/react/bridge/ReadableNativeMap; public fun getString (Ljava/lang/String;)Ljava/lang/String; @@ -1480,6 +1488,7 @@ public abstract interface class com/facebook/react/bridge/WritableArray : com/fa public abstract fun pushBoolean (Z)V public abstract fun pushDouble (D)V public abstract fun pushInt (I)V + public abstract fun pushLong (J)V public abstract fun pushMap (Lcom/facebook/react/bridge/ReadableMap;)V public abstract fun pushNull ()V public abstract fun pushString (Ljava/lang/String;)V @@ -1492,6 +1501,7 @@ public abstract interface class com/facebook/react/bridge/WritableMap : com/face public abstract fun putBoolean (Ljava/lang/String;Z)V public abstract fun putDouble (Ljava/lang/String;D)V public abstract fun putInt (Ljava/lang/String;I)V + public abstract fun putLong (Ljava/lang/String;J)V public abstract fun putMap (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableMap;)V public abstract fun putNull (Ljava/lang/String;)V public abstract fun putString (Ljava/lang/String;Ljava/lang/String;)V @@ -1503,6 +1513,7 @@ public class com/facebook/react/bridge/WritableNativeArray : com/facebook/react/ public fun pushBoolean (Z)V public fun pushDouble (D)V public fun pushInt (I)V + public fun pushLong (J)V public fun pushMap (Lcom/facebook/react/bridge/ReadableMap;)V public fun pushNull ()V public fun pushString (Ljava/lang/String;)V @@ -1516,6 +1527,7 @@ public class com/facebook/react/bridge/WritableNativeMap : com/facebook/react/br public fun putBoolean (Ljava/lang/String;Z)V public fun putDouble (Ljava/lang/String;D)V public fun putInt (Ljava/lang/String;I)V + public fun putLong (Ljava/lang/String;J)V public fun putMap (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableMap;)V public fun putNull (Ljava/lang/String;)V public fun putString (Ljava/lang/String;Ljava/lang/String;)V diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java index f81766a4c0243d..8f63d7a2add58d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyArray.java @@ -96,6 +96,11 @@ public int getInt(int index) { return ((Number) mBackingList.get(index)).intValue(); } + @Override + public long getLong(int index) { + return ((Number) mBackingList.get(index)).longValue(); + } + @Override public @Nullable String getString(int index) { return (String) mBackingList.get(index); @@ -129,7 +134,10 @@ public ReadableMap getMap(int index) { return ReadableType.Null; } else if (object instanceof Boolean) { return ReadableType.Boolean; - } else if (object instanceof Double || object instanceof Float || object instanceof Integer) { + } else if (object instanceof Double + || object instanceof Float + || object instanceof Integer + || object instanceof Long) { return ReadableType.Number; } else if (object instanceof String) { return ReadableType.String; @@ -156,6 +164,11 @@ public void pushInt(int value) { mBackingList.add(new Double(value)); } + @Override + public void pushLong(long value) { + mBackingList.add(value); + } + @Override public void pushString(@Nullable String value) { mBackingList.add(value); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java index ccce30a7d60e36..d54cb797fbc943 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaOnlyMap.java @@ -7,6 +7,8 @@ package com.facebook.react.bridge; +import static androidx.core.util.Preconditions.checkNotNull; + import androidx.annotation.NonNull; import androidx.annotation.Nullable; import java.util.HashMap; @@ -110,6 +112,11 @@ public int getInt(@NonNull String name) { return ((Number) mBackingMap.get(name)).intValue(); } + @Override + public long getLong(@NonNull String name) { + return ((Number) checkNotNull(mBackingMap.get(name))).longValue(); + } + @Override public String getString(@NonNull String name) { return (String) mBackingMap.get(name); @@ -190,6 +197,11 @@ public void putInt(@NonNull String key, int value) { mBackingMap.put(key, new Double(value)); } + @Override + public void putLong(@NonNull String key, long value) { + mBackingMap.put(key, value); + } + @Override public void putString(@NonNull String key, @Nullable String value) { mBackingMap.put(key, value); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java index 26a88911b30cae..9a5e84aaeae848 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableArray.java @@ -26,6 +26,8 @@ public interface ReadableArray { int getInt(int index); + long getLong(int index); + @NonNull String getString(int index); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java index a3bae65f7224c4..3dcc141a8cc232 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableMap.java @@ -29,6 +29,8 @@ public interface ReadableMap { int getInt(@NonNull String name); + long getLong(@NonNull String name); + @Nullable String getString(@NonNull String name); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java index 1a117a6b46928c..2f74df2334cac6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeArray.java @@ -97,6 +97,11 @@ public int getInt(int index) { return ((Double) getLocalArray()[index]).intValue(); } + @Override + public long getLong(int index) { + return ((Long) getLocalArray()[index]).longValue(); + } + @Override public @NonNull String getString(int index) { return (String) getLocalArray()[index]; diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java index 0ade85c10090f1..7e796f7cdaf7f5 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/ReadableNativeMap.java @@ -160,6 +160,11 @@ public int getInt(@NonNull String name) { return getValue(name, Double.class).intValue(); } + @Override + public long getLong(@NonNull String name) { + return getValue(name, Long.class).longValue(); + } + @Override public @Nullable String getString(@NonNull String name) { return getNullableValue(name, String.class); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java index 8ec699c3970ed9..0b7cb17f69487c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableArray.java @@ -20,6 +20,8 @@ public interface WritableArray extends ReadableArray { void pushInt(int value); + void pushLong(long value); + void pushString(@Nullable String value); void pushArray(@Nullable ReadableArray array); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java index 1f6ec6500970f1..a7ef7818f51bb5 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableMap.java @@ -21,6 +21,8 @@ public interface WritableMap extends ReadableMap { void putInt(@NonNull String key, int value); + void putLong(@NonNull String key, long value); + void putString(@NonNull String key, @Nullable String value); void putArray(@NonNull String key, @Nullable ReadableArray value); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java index 46a2c8d9e67e34..06ad5650e11c45 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeArray.java @@ -38,6 +38,9 @@ public WritableNativeArray() { @Override public native void pushInt(int value); + @Override + public native void pushLong(long value); + @Override public native void pushString(@Nullable String value); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java index 809cc9127f555e..228b3065de7a39 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/WritableNativeMap.java @@ -32,6 +32,9 @@ public class WritableNativeMap extends ReadableNativeMap implements WritableMap @Override public native void putInt(@NonNull String key, int value); + @Override + public native void putLong(@NonNull String key, long value); + @Override public native void putNull(@NonNull String key); diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyArrayTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyArrayTest.kt index 46fbb1f67668d5..33f749862dd192 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyArrayTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyArrayTest.kt @@ -15,15 +15,23 @@ class JavaOnlyArrayTest { @Test fun testGetType() { val values = - JavaOnlyArray.of(1, 2f, 3.0, "4", false, JavaOnlyArray.of(), JavaOnlyMap.of(), null) + JavaOnlyArray.of(1, 2f, 3.0, 4L, "5", false, JavaOnlyArray.of(), JavaOnlyMap.of(), null) assertThat(values.getType(0)).isEqualTo(ReadableType.Number) assertThat(values.getType(1)).isEqualTo(ReadableType.Number) assertThat(values.getType(2)).isEqualTo(ReadableType.Number) - assertThat(values.getType(3)).isEqualTo(ReadableType.String) - assertThat(values.getType(4)).isEqualTo(ReadableType.Boolean) - assertThat(values.getType(5)).isEqualTo(ReadableType.Array) - assertThat(values.getType(6)).isEqualTo(ReadableType.Map) - assertThat(values.getType(7)).isEqualTo(ReadableType.Null) + assertThat(values.getType(3)).isEqualTo(ReadableType.Number) + assertThat(values.getType(4)).isEqualTo(ReadableType.String) + assertThat(values.getType(5)).isEqualTo(ReadableType.Boolean) + assertThat(values.getType(6)).isEqualTo(ReadableType.Array) + assertThat(values.getType(7)).isEqualTo(ReadableType.Map) + assertThat(values.getType(8)).isEqualTo(ReadableType.Null) + } + + @Test + fun testLongValueNotTruncated() { + val values = JavaOnlyArray.of(1125899906842623L) + + assertThat(values.getLong(0)).isEqualTo(1125899906842623L) } } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyMapTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyMapTest.kt new file mode 100644 index 00000000000000..4596b712bbe7de --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/bridge/JavaOnlyMapTest.kt @@ -0,0 +1,55 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.bridge + +import org.assertj.core.api.Assertions.assertThat +import org.junit.Test + +/** Tests for [JavaOnlyMap] */ +class JavaOnlyMapTest { + @Test + fun testGetType() { + val values = + JavaOnlyMap.of( + "int", + 1, + "float", + 2f, + "double", + 3.0, + "long", + 4L, + "string", + "5", + "boolean", + false, + "array", + JavaOnlyArray.of(), + "map", + JavaOnlyMap.of(), + "null", + null) + + assertThat(values.getType("int")).isEqualTo(ReadableType.Number) + assertThat(values.getType("float")).isEqualTo(ReadableType.Number) + assertThat(values.getType("double")).isEqualTo(ReadableType.Number) + assertThat(values.getType("long")).isEqualTo(ReadableType.Number) + assertThat(values.getType("string")).isEqualTo(ReadableType.String) + assertThat(values.getType("boolean")).isEqualTo(ReadableType.Boolean) + assertThat(values.getType("array")).isEqualTo(ReadableType.Array) + assertThat(values.getType("map")).isEqualTo(ReadableType.Map) + assertThat(values.getType("null")).isEqualTo(ReadableType.Null) + } + + @Test + fun testLongValueNotTruncated() { + val values = JavaOnlyMap.of("long", 1125899906842623L) + + assertThat(values.getLong("long")).isEqualTo(1125899906842623L) + } +}