Merge pull request #141 from tompng/delete_before_fetch_bugfix

Delete before fetch bugfix

.github/workflows/test.yml

@@ -5,7 +5,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        ruby: [ '2.7', '3.0', '3.1' ]
+        ruby: [ '3.0', '3.1', '3.2', '3.3' ]
         gemfiles:
           - gemfiles/Gemfile-rails-6
           - gemfiles/Gemfile-rails-7

Gemfile.lock

@@ -23,15 +23,10 @@ GEM
     ar_serializer (1.2.0)
       activerecord
       top_n_loader
-    coderay (1.1.3)
     concurrent-ruby (1.1.10)
     i18n (1.10.0)
       concurrent-ruby (~> 1.0)
-    method_source (1.0.0)
     minitest (5.15.0)
-    pry (0.14.1)
-      coderay (~> 1.1)
-      method_source (~> 1.0)
     rake (13.0.6)
     sqlite3 (1.4.2)
     top_n_loader (1.0.2)
@@ -46,7 +41,6 @@ DEPENDENCIES
   activerecord-import
   ar_serializer
   ar_sync!
-  pry
   rake
   sqlite3
 

ar_sync.gemspec

@@ -22,7 +22,7 @@ Gem::Specification.new do |spec|
 
   spec.add_dependency 'activerecord'
   spec.add_dependency 'ar_serializer'
-  %w[rake pry sqlite3 activerecord-import].each do |gem_name|
+  %w[rake sqlite3 activerecord-import].each do |gem_name|
     spec.add_development_dependency gem_name
   end
 end

bin/console

@@ -2,10 +2,10 @@
 
 require 'bundler/setup'
 require 'ar_sync'
-require 'pry'
+require 'irb'
 require_relative '../test/model'
 ArSync.on_notification do |events|
   puts "\e[1m#{events.inspect}\e[m"
 end
 
-Pry.start
+IRB.start

core/ArSyncStore.js

@@ -299,6 +299,7 @@ var ArSyncRecord = /** @class */ (function (_super) {
     __extends(ArSyncRecord, _super);
     function ArSyncRecord(query, data, request, root) {
         var _this = _super.call(this) || this;
+        _this.fetching = new Set();
         _this.root = root;
         if (request)
             _this.initForReload(request);
@@ -385,6 +386,7 @@ var ArSyncRecord = /** @class */ (function (_super) {
         var aliasName = (query && query.as) || path;
         if (action === 'remove') {
             var child = this.children[aliasName];
+            // this.fetching.delete(`${aliasName}:${id}`) // To cancel consumeAdd
             if (child)
                 child.release();
             this.children[aliasName] = null;
@@ -395,7 +397,13 @@ var ArSyncRecord = /** @class */ (function (_super) {
         else if (action === 'add') {
             if (this.data[aliasName] && this.data[aliasName].id === id)
                 return;
+            var fetchKey_1 = aliasName + ":" + id;
+            this.fetching.add(fetchKey_1);
             modelBatchRequest.fetch(className, ArSyncRecord.compactQueryAttributes(query), id).then(function (data) {
+                // Record already removed
+                if (!_this.fetching.has(fetchKey_1))
+                    return;
+                _this.fetching.delete(fetchKey_1);
                 if (!data || !_this.data)
                     return;
                 var model = new ArSyncRecord(query, data, null, _this.root);
@@ -503,6 +511,7 @@ var ArSyncCollection = /** @class */ (function (_super) {
         var _this = _super.call(this) || this;
         _this.ordering = { orderBy: 'id', direction: 'asc' };
         _this.aliasOrderKey = 'id';
+        _this.fetching = new Set();
         _this.root = root;
         _this.path = path;
         _this.query = query;
@@ -630,7 +639,12 @@ var ArSyncCollection = /** @class */ (function (_super) {
                 }
             }
         }
+        this.fetching.add(id);
         modelBatchRequest.fetch(className, this.compactQueryAttributes, id).then(function (data) {
+            // Record already removed
+            if (!_this.fetching.has(id))
+                return;
+            _this.fetching.delete(id);
             if (!data || !_this.data)
                 return;
             var model = new ArSyncRecord(_this.query, data, null, _this.root);
@@ -700,6 +714,7 @@ var ArSyncCollection = /** @class */ (function (_super) {
     };
     ArSyncCollection.prototype.consumeRemove = function (id) {
         var idx = this.data.findIndex(function (a) { return a.id === id; });
+        this.fetching.delete(id); // To cancel consumeAdd
         if (idx < 0)
             return;
         this.mark();

src/core/ArSyncStore.ts

@@ -253,6 +253,7 @@ class ArSyncRecord extends ArSyncContainerBase {
   children: { [key: string]: ArSyncContainerBase | null }
   paths: string[]
   reloadQueryCache
+  fetching = new Set<string>()
   constructor(query, data, request, root) {
     super()
     this.root = root
@@ -332,14 +333,21 @@ class ArSyncRecord extends ArSyncContainerBase {
     const aliasName = (query && query.as) || path;
     if (action === 'remove') {
       const child = this.children[aliasName]
+      this.fetching.delete(`${aliasName}:${id}`) // To cancel consumeAdd
       if (child) child.release()
       this.children[aliasName] = null
       this.mark()
       this.data[aliasName] = null
       this.onChange([aliasName], null)
     } else if (action === 'add') {
       if (this.data[aliasName] && this.data[aliasName].id === id) return
+      const fetchKey = `${aliasName}:${id}`
+      this.fetching.add(fetchKey)
       modelBatchRequest.fetch(className, ArSyncRecord.compactQueryAttributes(query), id).then(data => {
+        // Record already removed
+        if (!this.fetching.has(fetchKey)) return
+
+        this.fetching.delete(fetchKey)
         if (!data || !this.data) return
         const model = new ArSyncRecord(query, data, null, this.root)
         const child = this.children[aliasName]
@@ -428,6 +436,7 @@ class ArSyncCollection extends ArSyncContainerBase {
   data: any[]
   children: ArSyncRecord[]
   aliasOrderKey = 'id'
+  fetching = new Set<number>()
   constructor(sync_keys: string[], path: string, query, data: any[], request, root){
     super()
     this.root = root
@@ -524,7 +533,12 @@ class ArSyncCollection extends ArSyncContainerBase {
         }
       }
     }
+    this.fetching.add(id)
     modelBatchRequest.fetch(className, this.compactQueryAttributes, id).then((data: any) => {
+      // Record already removed
+      if (!this.fetching.has(id)) return
+
+      this.fetching.delete(id)
       if (!data || !this.data) return
       const model = new ArSyncRecord(this.query, data, null, this.root)
       model.parentModel = this
@@ -585,6 +599,7 @@ class ArSyncCollection extends ArSyncContainerBase {
   }
   consumeRemove(id: number) {
     const idx = this.data.findIndex(a => a.id === id)
+    this.fetching.delete(id) // To cancel consumeAdd
     if (idx < 0) return
     this.mark()
     this.children[idx].release()

test/helper/test_runner.rb

@@ -76,23 +76,16 @@ def eval_script(code)
   def assert_script(code, **options, &block)
     timeout = options.has_key?(:timeout) ? options.delete(:timeout) : 1
     start = Time.now
-    raise if block ? options.size != 0 : options.size >= 2
 
     if options.empty?
       block ||= :itself.to_proc
     else
-      key, value = options.entries.first
-      /^(?<reversed>not_)?to_(?<verb>.+)/ =~ key
-      test = lambda do |v|
-        if verb == 'be'
-          v == value
-        else
-          v.send verb + '?', value
-        end
-      end
       block = lambda do |v|
-        f = test.call(v)
-        reversed ? !f : f
+        options.all? do |key, value|
+          /^(?<reversed>not_)?to_(?<verb>.+)/ =~ key
+          result = verb == 'be' ? v == value : v.send(verb + '?', value)
+          reversed ? !result : result
+        end
       end
     end
     return true if block.call eval_script(code)

test/sync_test.rb

@@ -90,6 +90,34 @@ class Schema
   runner.assert_script 'userModel.data.articles[0].comments[0].user.id', to_be: users.third.id
 end
 
+tap do # has_many destroy fast
+  title1 = "NewPost#{rand}"
+  title2 = "NewPost#{rand}"
+  temp_post = user.posts.create user: users.sample, title: title1
+  # Emulate record destroyed just after fetch started
+  temp_post._initialize_sync_info_before_mutation
+  temp_post._sync_notify :destroy
+  new_post = user.posts.create user: users.sample, title: title2
+  runner.assert_script 'userModel.data.articles.map(a => a.title)', to_include: title2, not_to_include: title1
+  temp_post.destroy
+  new_post.destroy
+end
+
+tap do # has_one destroy fast
+  comment = user.posts.first.comments.first
+  comment_code = 'userModel.data.articles[0].comments[0]'
+  comment.stars.where(user: user).destroy_all
+  runner.assert_script "#{comment_code}.myReaction", to_be: nil
+  # Emulate record destroyed just after fetch started
+  star = comment.stars.where(user: user).create
+  star._initialize_sync_info_before_mutation
+  star._sync_notify :destroy
+  comment.user.update name: rand.to_s
+  runner.assert_script "#{comment_code}.user.name", to_be: comment.user.name
+  runner.assert_script "#{comment_code}.myReaction", to_be: nil
+  star.destroy
+end
+
 tap do # parent replace
   comment = user.posts.first.comments.first
   runner.assert_script 'userModel.data.articles[0].comments[0].id', to_be: comment.id