diff --git a/realm/realm-library/src/androidTest/java/io/realm/ManagedRealmListForValueTests.java b/realm/realm-library/src/androidTest/java/io/realm/ManagedRealmListForValueTests.java index 1867051ec5..30fc07eba3 100644 --- a/realm/realm-library/src/androidTest/java/io/realm/ManagedRealmListForValueTests.java +++ b/realm/realm-library/src/androidTest/java/io/realm/ManagedRealmListForValueTests.java @@ -18,7 +18,6 @@ import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -57,6 +56,7 @@ import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -1072,7 +1072,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(1, changes.getInsertions().length); assertEquals(0, changes.getDeletions().length); assertEquals(0, changes.getChanges().length); @@ -1091,7 +1092,6 @@ public void onChange(RealmList collection, OrderedCollectionChangeSet ch } @Test - @Ignore("unexpected fail. Unexpected insertion index.") @RunTestInLooperThread public void changeListener_forAddAt() { Realm realm = looperThread.getRealm(); @@ -1122,7 +1122,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(1, changes.getInsertions().length); assertEquals(0, changes.getDeletions().length); assertEquals(0, changes.getChanges().length); @@ -1141,7 +1142,6 @@ public void onChange(RealmList collection, OrderedCollectionChangeSet ch } @Test - @Ignore("unexpected fail. Listener never be called.") @RunTestInLooperThread public void changeListener_forSet() { Realm realm = looperThread.getRealm(); @@ -1172,7 +1172,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(0, changes.getInsertions().length); assertEquals(0, changes.getDeletions().length); assertEquals(1, changes.getChanges().length); @@ -1191,7 +1192,6 @@ public void onChange(RealmList collection, OrderedCollectionChangeSet ch } @Test - @Ignore("unexpected fail. Unexpected deletion index.") @RunTestInLooperThread public void changeListener_forRemoveAt() { Realm realm = looperThread.getRealm(); @@ -1222,7 +1222,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(0, changes.getInsertions().length); assertEquals(1, changes.getDeletions().length); assertEquals(0, changes.getChanges().length); @@ -1233,7 +1234,11 @@ public void onChange(RealmList collection, OrderedCollectionChangeSet ch realm.beginTransaction(); //noinspection unchecked - assertEquals(generateValue(listType, 100), list.remove(1)); + if (listType == BINARY_LIST) { + assertArrayEquals((byte[]) generateValue(listType, 100), (byte[]) list.remove(1)); + } else { + assertEquals(generateValue(listType, 100), list.remove(1)); + } realm.commitTransaction(); assertEquals(2, listenerCalledCount.get()); @@ -1241,9 +1246,14 @@ public void onChange(RealmList collection, OrderedCollectionChangeSet ch } @Test - @Ignore("unexpected fail. Unexpected deletion index.") @RunTestInLooperThread public void changeListener_forRemoveObject() { + if (listType == BINARY_LIST) { + // 'removeAll()' never remove byte array element since 'equals()' never return true against byte array. + looperThread.testComplete(); + return; + } + Realm realm = looperThread.getRealm(); realm.executeTransaction(new Realm.Transaction() { @Override @@ -1255,7 +1265,7 @@ public void execute(Realm realm) { //noinspection unchecked list.add(generateValue(listType, 0)); //noinspection unchecked - list.add(generateValue(listType, 100)); + list.add(generateValue(listType, 101)); //noinspection unchecked list.add(generateValue(listType, 200)); } @@ -1272,8 +1282,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { - assertEquals(1, changes.getInsertions().length); + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(0, changes.getInsertions().length); assertEquals(1, changes.getDeletions().length); assertEquals(0, changes.getChanges().length); @@ -1284,7 +1294,7 @@ public void onChange(RealmList collection, OrderedCollectionChangeSet ch realm.beginTransaction(); //noinspection unchecked - assertTrue(list.remove(generateValue(listType, 100))); + assertTrue(list.remove(generateValue(listType, 101))); realm.commitTransaction(); assertEquals(2, listenerCalledCount.get()); @@ -1328,7 +1338,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(0, changes.getInsertions().length); assertEquals(listType == BOOLEAN_LIST ? 3 : 2, changes.getDeletions().length); assertEquals(0, changes.getChanges().length); @@ -1351,7 +1362,6 @@ public void onChange(RealmList collection, OrderedCollectionChangeSet ch } @Test - @Ignore("unexpected fail. Unexpected deletion index.") @RunTestInLooperThread public void changeListener_forDeleteAt() { Realm realm = looperThread.getRealm(); @@ -1382,7 +1392,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(0, changes.getInsertions().length); assertEquals(1, changes.getDeletions().length); assertEquals(0, changes.getChanges().length); @@ -1431,7 +1442,8 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { + assertNotNull(changes); assertEquals(0, changes.getInsertions().length); assertEquals(3, changes.getDeletions().length); assertEquals(0, changes.getChanges().length); @@ -1473,7 +1485,7 @@ public void onChange(RealmList element) { //noinspection unchecked list.addChangeListener(new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { fail(); } }); @@ -1523,7 +1535,7 @@ public void onChange(RealmList element) { OrderedRealmCollectionChangeListener> listener2 = new OrderedRealmCollectionChangeListener>() { @Override - public void onChange(RealmList collection, OrderedCollectionChangeSet changes) { + public void onChange(RealmList collection, @Nullable OrderedCollectionChangeSet changes) { assertEquals(0, listenerCalledCount.getAndIncrement()); looperThread.testComplete(); } diff --git a/realm/realm-library/src/main/cpp/io_realm_internal_Collection.cpp b/realm/realm-library/src/main/cpp/io_realm_internal_Collection.cpp index 34731cedcf..e9b6330a90 100644 --- a/realm/realm-library/src/main/cpp/io_realm_internal_Collection.cpp +++ b/realm/realm-library/src/main/cpp/io_realm_internal_Collection.cpp @@ -14,49 +14,22 @@ * limitations under the License. */ -#include #include "io_realm_internal_Collection.h" #include #include #include +#include "java_class_global_def.hpp" #include "java_sort_descriptor.hpp" +#include "observable_collection_wrapper.hpp" #include "util.hpp" -#include "java_class_global_def.hpp" - -#include "jni_util/java_class.hpp" -#include "jni_util/java_global_weak_ref.hpp" -#include "jni_util/java_method.hpp" using namespace realm; using namespace realm::jni_util; using namespace realm::_impl; -// We need to control the life cycle of Results, weak ref of Java Collection object and the NotificationToken. -// Wrap all three together, so when the Java Collection object gets GCed, all three of them will be invalidated. -struct ResultsWrapper { - JavaGlobalWeakRef m_collection_weak_ref; - NotificationToken m_notification_token; - Results m_results; - - ResultsWrapper(Results& results) - : m_collection_weak_ref() - , m_notification_token() - , m_results(std::move(results)) - { - } - - ResultsWrapper(ResultsWrapper&&) = delete; - ResultsWrapper& operator=(ResultsWrapper&&) = delete; - - ResultsWrapper(ResultsWrapper const&) = delete; - ResultsWrapper& operator=(ResultsWrapper const&) = delete; - - ~ResultsWrapper() - { - } -}; +typedef ObservableCollectionWrapper ResultsWrapper; static void finalize_results(jlong ptr); @@ -104,7 +77,8 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeCreateResultsFro { TR_ENTER() try { - auto& list = *reinterpret_cast(list_ptr); + auto& list_wrapper = *reinterpret_cast*>(list_ptr); + auto& list = list_wrapper.collection(); auto shared_realm = *(reinterpret_cast(shared_realm_ptr)); Results results = j_sort_desc ? list.sort(JavaSortDescriptor(env, j_sort_desc).sort_descriptor()) : @@ -122,7 +96,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeCreateSnapshot(J TR_ENTER_PTR(native_ptr); try { auto wrapper = reinterpret_cast(native_ptr); - auto snapshot_results = wrapper->m_results.snapshot(); + auto snapshot_results = wrapper->collection().snapshot(); auto snapshot_wrapper = new ResultsWrapper(snapshot_results); return reinterpret_cast(snapshot_wrapper); } @@ -137,7 +111,7 @@ JNIEXPORT jboolean JNICALL Java_io_realm_internal_Collection_nativeContains(JNIE try { auto wrapper = reinterpret_cast(native_ptr); auto row = reinterpret_cast(native_row_ptr); - size_t index = wrapper->m_results.index_of(RowExpr(*row)); + size_t index = wrapper->collection().index_of(RowExpr(*row)); return to_jbool(index != not_found); } CATCH_STD(); @@ -150,7 +124,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeGetRow(JNIEnv* e TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - auto row = wrapper->m_results.get(static_cast(index)); + auto row = wrapper->collection().get(static_cast(index)); return reinterpret_cast(new Row(std::move(row))); } CATCH_STD() @@ -162,7 +136,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeFirstRow(JNIEnv* TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - auto optional_row = wrapper->m_results.first(); + auto optional_row = wrapper->collection().first(); if (optional_row) { return reinterpret_cast(new Row(std::move(optional_row.value()))); } @@ -176,7 +150,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeLastRow(JNIEnv* TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - auto optional_row = wrapper->m_results.last(); + auto optional_row = wrapper->collection().last(); if (optional_row) { return reinterpret_cast(new Row(std::move(optional_row.value()))); } @@ -190,7 +164,7 @@ JNIEXPORT void JNICALL Java_io_realm_internal_Collection_nativeClear(JNIEnv* env TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - wrapper->m_results.clear(); + wrapper->collection().clear(); } CATCH_STD() } @@ -200,7 +174,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeSize(JNIEnv* env TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - return static_cast(wrapper->m_results.size()); + return static_cast(wrapper->collection().size()); } CATCH_STD() return 0; @@ -217,13 +191,13 @@ JNIEXPORT jobject JNICALL Java_io_realm_internal_Collection_nativeAggregate(JNIE Optional value; switch (agg_func) { case io_realm_internal_Collection_AGGREGATE_FUNCTION_MINIMUM: - value = wrapper->m_results.min(index); + value = wrapper->collection().min(index); break; case io_realm_internal_Collection_AGGREGATE_FUNCTION_MAXIMUM: - value = wrapper->m_results.max(index); + value = wrapper->collection().max(index); break; case io_realm_internal_Collection_AGGREGATE_FUNCTION_AVERAGE: { - Optional value_count(wrapper->m_results.average(index)); + Optional value_count(wrapper->collection().average(index)); if (value_count) { value = Optional(Mixed(value_count.value())); } @@ -233,7 +207,7 @@ JNIEXPORT jobject JNICALL Java_io_realm_internal_Collection_nativeAggregate(JNIE break; } case io_realm_internal_Collection_AGGREGATE_FUNCTION_SUM: - value = wrapper->m_results.sum(index); + value = wrapper->collection().sum(index); break; default: REALM_UNREACHABLE(); @@ -267,7 +241,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeSort(JNIEnv* env TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - auto sorted_result = wrapper->m_results.sort(JavaSortDescriptor(env, j_sort_desc).sort_descriptor()); + auto sorted_result = wrapper->collection().sort(JavaSortDescriptor(env, j_sort_desc).sort_descriptor()); return reinterpret_cast(new ResultsWrapper(sorted_result)); } CATCH_STD() @@ -281,7 +255,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeDistinct(JNIEnv* try { auto wrapper = reinterpret_cast(native_ptr); auto distinct_result = - wrapper->m_results.distinct(JavaSortDescriptor(env, j_distinct_desc).distinct_descriptor()); + wrapper->collection().distinct(JavaSortDescriptor(env, j_distinct_desc).distinct_descriptor()); return reinterpret_cast(new ResultsWrapper(distinct_result)); } CATCH_STD() @@ -293,38 +267,9 @@ JNIEXPORT void JNICALL Java_io_realm_internal_Collection_nativeStartListening(JN { TR_ENTER_PTR(native_ptr) - static JavaClass os_results_class(env, "io/realm/internal/Collection"); - static JavaMethod notify_change_listeners(env, os_results_class, "notifyChangeListeners", "(J)V"); - try { auto wrapper = reinterpret_cast(native_ptr); - if (!wrapper->m_collection_weak_ref) { - wrapper->m_collection_weak_ref = JavaGlobalWeakRef(env, instance); - } - - auto cb = [=](CollectionChangeSet const& changes, std::exception_ptr err) { - // OS will call all notifiers' callback in one run, so check the Java exception first!! - if (env->ExceptionCheck()) - return; - - if (err) { - try { - std::rethrow_exception(err); - } - catch (const std::exception& e) { - realm::jni_util::Log::e("Caught exception in collection change callback %1", e.what()); - return; - } - } - - wrapper->m_collection_weak_ref.call_with_local_ref(env, [&](JNIEnv* local_env, jobject collection_obj) { - local_env->CallVoidMethod( - collection_obj, notify_change_listeners, - reinterpret_cast(changes.empty() ? 0 : new CollectionChangeSet(changes))); - }); - }; - - wrapper->m_notification_token = wrapper->m_results.add_notification_callback(cb); + wrapper->start_listening(env, instance); } CATCH_STD() } @@ -335,7 +280,7 @@ JNIEXPORT void JNICALL Java_io_realm_internal_Collection_nativeStopListening(JNI try { auto wrapper = reinterpret_cast(native_ptr); - wrapper->m_notification_token = {}; + wrapper->stop_listening(); } CATCH_STD() } @@ -352,7 +297,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeWhere(JNIEnv* en try { auto wrapper = reinterpret_cast(native_ptr); - auto table_view = wrapper->m_results.get_tableview(); + auto table_view = wrapper->collection().get_tableview(); Query* query = new Query(table_view.get_parent(), std::unique_ptr(new TableView(std::move(table_view)))); return reinterpret_cast(query); @@ -369,7 +314,7 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_Collection_nativeIndexOf(JNIEnv* auto wrapper = reinterpret_cast(native_ptr); auto row = reinterpret_cast(row_native_ptr); - return static_cast(wrapper->m_results.index_of(RowExpr(*row))); + return static_cast(wrapper->collection().index_of(RowExpr(*row))); } CATCH_STD() return npos; @@ -380,7 +325,7 @@ JNIEXPORT jboolean JNICALL Java_io_realm_internal_Collection_nativeDeleteLast(JN TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - auto row = wrapper->m_results.last(); + auto row = wrapper->collection().last(); if (row && row->is_attached()) { row->move_last_over(); return JNI_TRUE; @@ -396,7 +341,7 @@ JNIEXPORT jboolean JNICALL Java_io_realm_internal_Collection_nativeDeleteFirst(J try { auto wrapper = reinterpret_cast(native_ptr); - auto row = wrapper->m_results.first(); + auto row = wrapper->collection().first(); if (row && row->is_attached()) { row->move_last_over(); return JNI_TRUE; @@ -413,7 +358,7 @@ JNIEXPORT void JNICALL Java_io_realm_internal_Collection_nativeDelete(JNIEnv* en try { auto wrapper = reinterpret_cast(native_ptr); - auto row = wrapper->m_results.get(index); + auto row = wrapper->collection().get(index); if (row.is_attached()) { row.move_last_over(); } @@ -426,7 +371,7 @@ JNIEXPORT jboolean JNICALL Java_io_realm_internal_Collection_nativeIsValid(JNIEn TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - return wrapper->m_results.is_valid(); + return wrapper->collection().is_valid(); } CATCH_STD() return JNI_FALSE; @@ -437,7 +382,7 @@ JNIEXPORT jbyte JNICALL Java_io_realm_internal_Collection_nativeGetMode(JNIEnv* TR_ENTER_PTR(native_ptr) try { auto wrapper = reinterpret_cast(native_ptr); - switch (wrapper->m_results.get_mode()) { + switch (wrapper->collection().get_mode()) { case Results::Mode::Empty: return io_realm_internal_Collection_MODE_EMPTY; case Results::Mode::Table: diff --git a/realm/realm-library/src/main/cpp/io_realm_internal_OsList.cpp b/realm/realm-library/src/main/cpp/io_realm_internal_OsList.cpp index 53c1804b7a..ccee1c9720 100644 --- a/realm/realm-library/src/main/cpp/io_realm_internal_OsList.cpp +++ b/realm/realm-library/src/main/cpp/io_realm_internal_OsList.cpp @@ -20,6 +20,7 @@ #include #include +#include "observable_collection_wrapper.hpp" #include "java_accessor.hpp" #include "java_exception_def.hpp" #include "jni_util/java_exception_thrower.hpp" @@ -29,42 +30,44 @@ using namespace realm; using namespace realm::util; using namespace realm::_impl; +typedef ObservableCollectionWrapper ListWrapper; + namespace { void finalize_list(jlong ptr) { TR_ENTER_PTR(ptr) - delete reinterpret_cast(ptr); + delete reinterpret_cast(ptr); } inline void add_value(JNIEnv* env, jlong list_ptr, Any&& value) { - auto& list = *reinterpret_cast(list_ptr); + auto& wrapper = *reinterpret_cast(list_ptr); JavaAccessorContext context(env); - list.add(context, value); + wrapper.collection().add(context, value); } inline void insert_value(JNIEnv* env, jlong list_ptr, jlong pos, Any&& value) { - auto& list = *reinterpret_cast(list_ptr); + auto& wrapper = *reinterpret_cast(list_ptr); JavaAccessorContext context(env); - list.insert(context, pos, value); + wrapper.collection().insert(context, pos, value); } inline void set_value(JNIEnv* env, jlong list_ptr, jlong pos, Any&& value) { - auto& list = *reinterpret_cast(list_ptr); + auto& wrapper = *reinterpret_cast(list_ptr); JavaAccessorContext context(env); - list.set(context, pos, value); + wrapper.collection().set(context, pos, value); } // Check nullable earlier https://github.com/realm/realm-object-store/issues/544 inline void check_nullable(JNIEnv* env, jlong list_ptr, jobject jobject_ptr = nullptr) { - auto& list = *reinterpret_cast(list_ptr); - if (!jobject_ptr && !is_nullable(list.get_type())) { + auto& wrapper = *reinterpret_cast(list_ptr); + if (!jobject_ptr && !is_nullable(wrapper.collection().get_type())) { THROW_JAVA_EXCEPTION(env, JavaExceptionDef::IllegalArgument, "This 'RealmList' is not nullable. A non-null value is expected."); } @@ -92,10 +95,11 @@ JNIEXPORT jlongArray JNICALL Java_io_realm_internal_OsList_nativeCreate(JNIEnv* auto& shared_realm = *reinterpret_cast(shared_realm_ptr); jlong ret[2]; - auto list_ptr = new List(shared_realm, *row.get_table(), column_index, row.get_index()); - ret[0] = reinterpret_cast(list_ptr); + List list(shared_realm, *row.get_table(), column_index, row.get_index()); + ListWrapper* wrapper_ptr = new ListWrapper(list); + ret[0] = reinterpret_cast(wrapper_ptr); - if (list_ptr->get_type() == PropertyType::Object) { + if (wrapper_ptr->collection().get_type() == PropertyType::Object) { LinkViewRef link_view_ref(row.get_linklist(column_index)); Table* target_table_ptr = &(link_view_ref)->get_target_table(); @@ -124,8 +128,8 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_OsList_nativeGetRow(JNIEnv* env, TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - auto row = list.get(column_index); + auto& wrapper = *reinterpret_cast(list_ptr); + auto row = wrapper.collection().get(column_index); return reinterpret_cast(new Row(std::move(row))); } CATCH_STD() @@ -138,8 +142,8 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeAddRow(JNIEnv* env, j TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.add(static_cast(target_row_index)); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().add(static_cast(target_row_index)); } CATCH_STD() } @@ -150,8 +154,8 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeInsertRow(JNIEnv* env TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.insert(static_cast(pos), static_cast(target_row_index)); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().insert(static_cast(pos), static_cast(target_row_index)); } CATCH_STD() } @@ -162,8 +166,8 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeSetRow(JNIEnv* env, j TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.set(static_cast(pos), static_cast(target_row_index)); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().set(static_cast(pos), static_cast(target_row_index)); } CATCH_STD() } @@ -174,8 +178,8 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeMove(JNIEnv* env, jcl TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.move(source_index, target_index); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().move(source_index, target_index); } CATCH_STD() } @@ -185,8 +189,8 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeRemove(JNIEnv* env, j TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.remove(index); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().remove(index); } CATCH_STD() } @@ -196,8 +200,8 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeRemoveAll(JNIEnv* env TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.remove_all(); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().remove_all(); } CATCH_STD() } @@ -207,8 +211,8 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_OsList_nativeSize(JNIEnv* env, jc TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - return list.size(); + auto& wrapper = *reinterpret_cast(list_ptr); + return wrapper.collection().size(); } CATCH_STD() return 0; @@ -219,8 +223,8 @@ JNIEXPORT jlong JNICALL Java_io_realm_internal_OsList_nativeGetQuery(JNIEnv* env TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - auto query = list.get_query(); + auto& wrapper = *reinterpret_cast(list_ptr); + auto query = wrapper.collection().get_query(); return reinterpret_cast(new Query(std::move(query))); } CATCH_STD() @@ -232,8 +236,8 @@ JNIEXPORT jboolean JNICALL Java_io_realm_internal_OsList_nativeIsValid(JNIEnv* e TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - return list.is_valid(); + auto& wrapper = *reinterpret_cast(list_ptr); + return wrapper.collection().is_valid(); } CATCH_STD() return JNI_FALSE; @@ -244,8 +248,8 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeDelete(JNIEnv* env, j TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.delete_at(S(index)); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().delete_at(S(index)); } CATCH_STD() } @@ -255,8 +259,31 @@ JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeDeleteAll(JNIEnv* env TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); - list.delete_all(); + auto& wrapper = *reinterpret_cast(list_ptr); + wrapper.collection().delete_all(); + } + CATCH_STD() +} + +JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeStartListening(JNIEnv* env, jobject instance, + jlong native_ptr) +{ + TR_ENTER_PTR(native_ptr) + + try { + auto wrapper = reinterpret_cast(native_ptr); + wrapper->start_listening(env, instance); + } + CATCH_STD() +} + +JNIEXPORT void JNICALL Java_io_realm_internal_OsList_nativeStopListening(JNIEnv* env, jobject, jlong native_ptr) +{ + TR_ENTER_PTR(native_ptr) + + try { + auto wrapper = reinterpret_cast(native_ptr); + wrapper->stop_listening(); } CATCH_STD() } @@ -514,9 +541,9 @@ JNIEXPORT jobject JNICALL Java_io_realm_internal_OsList_nativeGetValue(JNIEnv* e { TR_ENTER_PTR(list_ptr) try { - auto& list = *reinterpret_cast(list_ptr); + auto& wrapper = *reinterpret_cast(list_ptr); JavaAccessorContext context(env); - return any_cast(list.get(context, pos)); + return any_cast(wrapper.collection().get(context, pos)); } CATCH_STD() diff --git a/realm/realm-library/src/main/cpp/observable_collection_wrapper.hpp b/realm/realm-library/src/main/cpp/observable_collection_wrapper.hpp new file mode 100644 index 0000000000..ed53c62c9f --- /dev/null +++ b/realm/realm-library/src/main/cpp/observable_collection_wrapper.hpp @@ -0,0 +1,105 @@ +/* + * Copyright 2017 Realm Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef REALM_JNI_IMPL_OBSERVABLE_COLLECTION_WRAPPER_HPP +#define REALM_JNI_IMPL_OBSERVABLE_COLLECTION_WRAPPER_HPP + +#include "jni_util/java_class.hpp" +#include "jni_util/java_global_weak_ref.hpp" +#include "jni_util/java_method.hpp" +#include "jni_util/log.hpp" + +namespace realm { +namespace _impl { + +// Wrapper of Object Store List & Results. +// We need to control the life cycle of Results/List, weak ref of Java Collection object and the NotificationToken. +// Wrap all three together, so when the Java Collection object gets GCed, all three of them will be invalidated. +template +class ObservableCollectionWrapper { +public: + ObservableCollectionWrapper(T& collection) + : m_collection_weak_ref() + , m_notification_token() + , m_collection(std::move(collection)) + { + } + + ~ObservableCollectionWrapper() = default; + + ObservableCollectionWrapper(ObservableCollectionWrapper&&) = delete; + ObservableCollectionWrapper& operator=(ObservableCollectionWrapper&&) = delete; + ObservableCollectionWrapper(ObservableCollectionWrapper const&) = delete; + ObservableCollectionWrapper& operator=(ObservableCollectionWrapper const&) = delete; + + T& collection() + { + return m_collection; + }; + void start_listening(JNIEnv* env, jobject j_collection_object); + void stop_listening(); + +private: + jni_util::JavaGlobalWeakRef m_collection_weak_ref; + NotificationToken m_notification_token; + T m_collection; +}; + +template +void ObservableCollectionWrapper::start_listening(JNIEnv* env, jobject j_collection_object) +{ + static jni_util::JavaClass os_results_class(env, "io/realm/internal/ObservableCollection"); + static jni_util::JavaMethod notify_change_listeners(env, os_results_class, "notifyChangeListeners", "(J)V"); + + if (!m_collection_weak_ref) { + m_collection_weak_ref = jni_util::JavaGlobalWeakRef(env, j_collection_object); + } + + auto cb = [=](CollectionChangeSet const& changes, std::exception_ptr err) { + // OS will call all notifiers' callback in one run, so check the Java exception first!! + if (env->ExceptionCheck()) + return; + + if (err) { + try { + std::rethrow_exception(err); + } + catch (const std::exception& e) { + realm::jni_util::Log::e("Caught exception in collection change callback %1", e.what()); + return; + } + } + + m_collection_weak_ref.call_with_local_ref(env, [&](JNIEnv* local_env, jobject collection_obj) { + local_env->CallVoidMethod( + collection_obj, notify_change_listeners, + reinterpret_cast(changes.empty() ? 0 : new CollectionChangeSet(changes))); + }); + }; + + m_notification_token = m_collection.add_notification_callback(cb); +} + +template +void ObservableCollectionWrapper::stop_listening() +{ + m_notification_token = {}; +} + +} // namespace realm +} // namespace _impl + +#endif // REALM_JNI_IMPL_OBSERVABLE_COLLECTION_WRAPPER_HPP diff --git a/realm/realm-library/src/main/java/io/realm/OrderedRealmCollectionChangeListener.java b/realm/realm-library/src/main/java/io/realm/OrderedRealmCollectionChangeListener.java index b9a5261c85..24216e776d 100644 --- a/realm/realm-library/src/main/java/io/realm/OrderedRealmCollectionChangeListener.java +++ b/realm/realm-library/src/main/java/io/realm/OrderedRealmCollectionChangeListener.java @@ -16,6 +16,8 @@ package io.realm; +import javax.annotation.Nullable; + /** * {@link OrderedRealmCollectionChangeListener} can be registered with a {@link RealmResults} to receive a notification * with a {@link OrderedCollectionChangeSet} to describe the details of what have been changed in the collection from @@ -36,5 +38,5 @@ public interface OrderedRealmCollectionChangeListener { * @param changeSet object with information about which rows in the collection were added, removed or modified. * {@code null} is returned the first time an async query is completed. */ - void onChange(T t, OrderedCollectionChangeSet changeSet); + void onChange(T t, @Nullable OrderedCollectionChangeSet changeSet); } diff --git a/realm/realm-library/src/main/java/io/realm/RealmList.java b/realm/realm-library/src/main/java/io/realm/RealmList.java index 282b94f510..248bf8039b 100644 --- a/realm/realm-library/src/main/java/io/realm/RealmList.java +++ b/realm/realm-library/src/main/java/io/realm/RealmList.java @@ -63,7 +63,6 @@ public class RealmList extends AbstractList implements OrderedRealmCollect private static final String ALLOWED_ONLY_FOR_REALM_MODEL_ELEMENT_MESSAGE = "This feature is available only when the element type is implementing RealmModel."; public static final String REMOVE_OUTSIDE_TRANSACTION_ERROR = "Objects can only be removed from inside a write transaction."; - private final io.realm.internal.Collection collection; @Nullable protected Class clazz; @Nullable @@ -73,6 +72,8 @@ public class RealmList extends AbstractList implements OrderedRealmCollect private final ManagedListOperator osListOperator; final protected BaseRealm realm; private List unmanagedList; + // Used for listeners on RealmList + private io.realm.internal.Collection osResults; /** * Creates a RealmList in unmanaged mode, where the elements are not controlled by a Realm. @@ -82,7 +83,6 @@ public class RealmList extends AbstractList implements OrderedRealmCollect * Use {@link io.realm.Realm#copyToRealm(Iterable)} to properly persist its elements in Realm. */ public RealmList() { - collection = null; realm = null; osListOperator = null; unmanagedList = new ArrayList<>(); @@ -103,7 +103,6 @@ public RealmList(E... objects) { if (objects == null) { throw new IllegalArgumentException("The objects argument cannot be null"); } - collection = null; realm = null; osListOperator = null; unmanagedList = new ArrayList<>(objects.length); @@ -118,14 +117,12 @@ public RealmList(E... objects) { * @param realm reference to Realm containing the data. */ RealmList(Class clazz, OsList osList, BaseRealm realm) { - this.collection = new io.realm.internal.Collection(realm.sharedRealm, osList, null); this.clazz = clazz; osListOperator = getOperator(realm, osList, clazz, null); this.realm = realm; } RealmList(String className, OsList osList, BaseRealm realm) { - this.collection = new io.realm.internal.Collection(realm.sharedRealm, osList, null); this.realm = realm; this.className = className; osListOperator = getOperator(realm, osList, null, className); @@ -965,7 +962,11 @@ private void checkForAddRemoveListener(@Nullable Object listener, boolean checkL */ public void addChangeListener(OrderedRealmCollectionChangeListener> listener) { checkForAddRemoveListener(listener, true); - collection.addListener(this, listener); + if (osListOperator.forRealmModel()) { + getOrCreateOsResultsForListener().addListener(this, listener); + } else { + osListOperator.getOsList().addListener(this, listener); + } } /** @@ -978,7 +979,11 @@ public void addChangeListener(OrderedRealmCollectionChangeListener> */ public void removeChangeListener(OrderedRealmCollectionChangeListener> listener) { checkForAddRemoveListener(listener, true); - collection.removeListener(this, listener); + if (osListOperator.forRealmModel()) { + getOrCreateOsResultsForListener().removeListener(this, listener); + } else { + osListOperator.getOsList().removeListener(this, listener); + } } /** @@ -1016,7 +1021,11 @@ public void removeChangeListener(OrderedRealmCollectionChangeListener> listener) { checkForAddRemoveListener(listener, true); - collection.addListener(this, listener); + if (osListOperator.forRealmModel()) { + getOrCreateOsResultsForListener().addListener(this, listener); + } else { + osListOperator.getOsList().addListener(this, listener); + } } /** @@ -1029,7 +1038,11 @@ public void addChangeListener(RealmChangeListener> listener) { */ public void removeChangeListener(RealmChangeListener> listener) { checkForAddRemoveListener(listener, true); - collection.removeListener(this, listener); + if (osListOperator.forRealmModel()) { + getOrCreateOsResultsForListener().removeListener(this, listener); + } else { + osListOperator.getOsList().removeListener(this, listener); + } } /** @@ -1040,7 +1053,11 @@ public void removeChangeListener(RealmChangeListener> listener) { */ public void removeAllChangeListeners() { checkForAddRemoveListener(null, false); - collection.removeAllListeners(); + if (osListOperator.forRealmModel()) { + getOrCreateOsResultsForListener().removeAllListeners(); + } else { + osListOperator.getOsList().removeAllListeners(); + } } // Custom RealmList iterator. @@ -1260,6 +1277,20 @@ private ManagedListOperator getOperator(BaseRealm realm, OsList osList, @Null } throw new IllegalArgumentException("Unexpected value class: " + clazz.getName()); } + + // TODO: Object Store is not able to merge change set for links list. Luckily since we were still using LinkView + // when ship the fine grain notifications, the listener on RealmList is actually added to a OS Results which is + // created from the link view. OS Results is computing the change set by comparing the old/new collection. So it + // will give the right results if you remove all elements from a RealmList then add all them back and add one more + // new element. By right results it means the change set only include one insertion. But if the listener is on the + // OS List, the change set will include all ranges of th list. So we keep the old behaviour for + // RealmList for now. See https://github.com/realm/realm-object-store/issues/541 + private io.realm.internal.Collection getOrCreateOsResultsForListener() { + if (osResults == null) { + this.osResults = new io.realm.internal.Collection(realm.sharedRealm, osListOperator.getOsList(), null); + } + return osResults; + } } /** diff --git a/realm/realm-library/src/main/java/io/realm/internal/Collection.java b/realm/realm-library/src/main/java/io/realm/internal/Collection.java index d46e7b8538..df0b2c6cf0 100644 --- a/realm/realm-library/src/main/java/io/realm/internal/Collection.java +++ b/realm/realm-library/src/main/java/io/realm/internal/Collection.java @@ -22,7 +22,6 @@ import javax.annotation.Nullable; -import io.realm.OrderedCollectionChangeSet; import io.realm.OrderedRealmCollectionChangeListener; import io.realm.RealmChangeListener; @@ -31,68 +30,11 @@ * Java wrapper of Object Store Results class. * It is the backend of binding's query results and back links. */ -@Keep -public class Collection implements NativeObject { +public class Collection implements NativeObject, ObservableCollection { private static final String CLOSED_REALM_MESSAGE = "This Realm instance has already been closed, making it unusable."; - private static class CollectionObserverPair extends ObserverPairList.ObserverPair { - public CollectionObserverPair(T observer, Object listener) { - super(observer, listener); - } - - public void onChange(T observer, OrderedCollectionChangeSet changes) { - if (listener instanceof OrderedRealmCollectionChangeListener) { - //noinspection unchecked - ((OrderedRealmCollectionChangeListener) listener).onChange(observer, changes); - } else if (listener instanceof RealmChangeListener) { - //noinspection unchecked - ((RealmChangeListener) listener).onChange(observer); - } else { - throw new RuntimeException("Unsupported listener type: " + listener); - } - } - } - - private static class RealmChangeListenerWrapper implements OrderedRealmCollectionChangeListener { - private final RealmChangeListener listener; - - RealmChangeListenerWrapper(RealmChangeListener listener) { - this.listener = listener; - } - - @Override - public void onChange(T collection, OrderedCollectionChangeSet changes) { - listener.onChange(collection); - } - - @Override - public boolean equals(Object obj) { - return obj instanceof RealmChangeListenerWrapper && - listener == ((RealmChangeListenerWrapper) obj).listener; - } - - @Override - public int hashCode() { - return listener.hashCode(); - } - } - - private static class Callback implements ObserverPairList.Callback { - private final OrderedCollectionChangeSet changeSet; - - Callback(OrderedCollectionChangeSet changeSet) { - this.changeSet = changeSet; - } - - @Override - public void onCalled(CollectionObserverPair pair, Object observer) { - //noinspection unchecked - pair.onChange(observer, changeSet); - } - } - // Custom Collection iterator. It ensures that we only iterate on a Realm collection that hasn't changed. public static abstract class Iterator implements java.util.Iterator { Collection iteratorCollection; @@ -517,8 +459,8 @@ public boolean isValid() { } // Called by JNI - @SuppressWarnings("unused") - private void notifyChangeListeners(long nativeChangeSetPtr) { + @Override + public void notifyChangeListeners(long nativeChangeSetPtr) { if (nativeChangeSetPtr == 0 && isLoaded()) { return; } diff --git a/realm/realm-library/src/main/java/io/realm/internal/ObservableCollection.java b/realm/realm-library/src/main/java/io/realm/internal/ObservableCollection.java new file mode 100644 index 0000000000..7004df192c --- /dev/null +++ b/realm/realm-library/src/main/java/io/realm/internal/ObservableCollection.java @@ -0,0 +1,72 @@ +package io.realm.internal; + +import javax.annotation.Nullable; + +import io.realm.OrderedCollectionChangeSet; +import io.realm.OrderedRealmCollectionChangeListener; +import io.realm.RealmChangeListener; + +// Helper class for supporting add change listeners on OsResults & OsList. +@Keep +interface ObservableCollection { + + class CollectionObserverPair extends ObserverPairList.ObserverPair { + public CollectionObserverPair(T observer, Object listener) { + super(observer, listener); + } + + public void onChange(T observer, @Nullable OrderedCollectionChangeSet changes) { + if (listener instanceof OrderedRealmCollectionChangeListener) { + //noinspection unchecked + ((OrderedRealmCollectionChangeListener) listener).onChange(observer, changes); + } else if (listener instanceof RealmChangeListener) { + //noinspection unchecked + ((RealmChangeListener) listener).onChange(observer); + } else { + throw new RuntimeException("Unsupported listener type: " + listener); + } + } + } + + class RealmChangeListenerWrapper implements OrderedRealmCollectionChangeListener { + private final RealmChangeListener listener; + + RealmChangeListenerWrapper(RealmChangeListener listener) { + this.listener = listener; + } + + @Override + public void onChange(T collection, @Nullable OrderedCollectionChangeSet changes) { + listener.onChange(collection); + } + + @Override + public boolean equals(Object obj) { + return obj instanceof RealmChangeListenerWrapper && + listener == ((RealmChangeListenerWrapper) obj).listener; + } + + @Override + public int hashCode() { + return listener.hashCode(); + } + } + + class Callback implements ObserverPairList.Callback { + private final OrderedCollectionChangeSet changeSet; + + Callback(@Nullable OrderedCollectionChangeSet changeSet) { + this.changeSet = changeSet; + } + + @Override + public void onCalled(CollectionObserverPair pair, Object observer) { + //noinspection unchecked + pair.onChange(observer, changeSet); + } + } + + // Called by JNI + @SuppressWarnings("SameParameterValue") + void notifyChangeListeners(long nativeChangeSetPtr); +} diff --git a/realm/realm-library/src/main/java/io/realm/internal/OsList.java b/realm/realm-library/src/main/java/io/realm/internal/OsList.java index aeb60ecb9f..bedf3e928a 100644 --- a/realm/realm-library/src/main/java/io/realm/internal/OsList.java +++ b/realm/realm-library/src/main/java/io/realm/internal/OsList.java @@ -4,16 +4,20 @@ import javax.annotation.Nullable; +import io.realm.OrderedRealmCollectionChangeListener; +import io.realm.RealmChangeListener; /** * Java wrapper of Object Store List class. This backs managed versions of RealmList. */ -public class OsList implements NativeObject { +public class OsList implements NativeObject, ObservableCollection { private final long nativePtr; private final NativeContext context; private final Table targetTable; private static final long nativeFinalizerPtr = nativeGetFinalizerPtr(); + private final ObserverPairList observerPairs = + new ObserverPairList(); public OsList(UncheckedRow row, long columnIndex) { SharedRealm sharedRealm = row.getTable().getSharedRealm(); @@ -212,6 +216,44 @@ public Table getTargetTable() { return targetTable; } + public void addListener(T observer, OrderedRealmCollectionChangeListener listener) { + if (observerPairs.isEmpty()) { + nativeStartListening(nativePtr); + } + CollectionObserverPair collectionObserverPair = new CollectionObserverPair(observer, listener); + observerPairs.add(collectionObserverPair); + } + + public void addListener(T observer, RealmChangeListener listener) { + addListener(observer, new RealmChangeListenerWrapper(listener)); + } + + public void removeListener(T observer, OrderedRealmCollectionChangeListener listener) { + observerPairs.remove(observer, listener); + if (observerPairs.isEmpty()) { + nativeStopListening(nativePtr); + } + } + + public void removeListener(T observer, RealmChangeListener listener) { + removeListener(observer, new RealmChangeListenerWrapper(listener)); + } + + public void removeAllListeners() { + observerPairs.clear(); + nativeStopListening(nativePtr); + } + + // Called by JNI + @Override + public void notifyChangeListeners(long nativeChangeSetPtr) { + if (nativeChangeSetPtr == 0) { + // First time "query" returns. Do nothing. + return; + } + observerPairs.foreach(new Callback(new OsCollectionChangeSet(nativeChangeSetPtr))); + } + private static native long nativeGetFinalizerPtr(); // TODO: nativeTablePtr is not necessary. It is used to create FieldDescriptor which should be generated from @@ -292,4 +334,8 @@ public Table getTargetTable() { private static native void nativeSetString(long nativePtr, long pos, @Nullable String value); private static native Object nativeGetValue(long nativePtr, long pos); + + private native void nativeStartListening(long nativePtr); + + private native void nativeStopListening(long nativePtr); }