Skip to content
This repository

Выравнивание логики уничтожения моделей #174

Merged
merged 2 commits into from 5 months ago

3 participants

edoroshenko chestozo Aleksei Androsov
edoroshenko
Collaborator

Сделал следующее:

  1. Убрал пересоздание модели видом при её удалении. Вообще, это очень странное было место: удаляешь модель, а она раз и обратно создаётся.

  2. Сделал, чтобы при уничтожении модели её экземпляр физически не удалялся. Она теперь только инвалидируется, а экземпляр остаётся. Это необходимо, потомучто вид, зависящй от этой модели, может даже после её удаления обращаться к ней через this.models.

  3. Метод ns.Model.find теперь возвращает только валидные модели. Чтобы после удаления модели она не искалась через find. Метод ns.Model.get работает как раньше - возвращает существующий экземпляр (вне зависимости от статуса) или создаёт новый.

В результате, как мне кажется, стало ровнее и понятнее работать удаление моделей.

  1. Убрал копипаст метода getRequestViews из viewCollection. Я это сделал в предыдущем заходе, и хоть оно в результате и не пригодилось решил оставить.
chestozo chestozo commented on the diff November 02, 2013
src/ns.model.js
@@ -508,8 +520,8 @@ ns.Model.destroy = function(model) {
508 520
 };
509 521
 
510 522
 //  Проверяем, есть ли модель в кэше и валидна ли она.
511  
-ns.Model.isValid = function(id, key) {
512  
-    var model = ns.Model.get(id, key);
  523
+ns.Model.isValid = function(id, params) {
4
chestozo Collaborator

Вот тут как-то жёстко меняется api. А зачем?

edoroshenko Collaborator

просто меняется имя переменной =)

edoroshenko Collaborator

сюда всегда по факту передаются параметры

chestozo Collaborator

Ааааа )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/ns.model.js
((10 lines not shown))
475 476
  */
476 477
 ns.Model.find = function(id, params) {
  478
+    var model = this._find(id, params);
  479
+    if (model && model.isValid()) {
2
Aleksei Androsov Owner
doochik added a note November 06, 2013

А как мне теперь получить любую закешированную модель?

edoroshenko Collaborator

Поговорили голосом.
А как мне теперь получить любую закешированную модель?
С помощью ns.Model.get. Теперь нет существенной разницы между отсутствием экземпляра модели и наличием невалидного экземпляра.

Договорились переименовать метод find в getValid, чтобы название было правдой.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Aleksei Androsov doochik merged commit 33819ae into from November 08, 2013
Aleksei Androsov doochik closed this November 08, 2013
Aleksei Androsov doochik deleted the branch November 08, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 2 unique commits by 1 author.

Nov 01, 2013
storing invalid model instances after destruction e5fa0a6
Nov 06, 2013
find -> getValid ce00d85
This page is out of date. Refresh to see the latest.
30  src/ns.model.js
@@ -451,7 +451,8 @@ ns.Model.prototype.getRequestParams = function() {
451 451
  * @returns {ns.Model}
452 452
  */
453 453
 ns.Model.get = function(id, params) {
454  
-    var model = ns.Model.find(id, params);
  454
+
  455
+    var model = this._find(id, params);
455 456
 
456 457
     if (!model) {
457 458
         var Ctor = _ctors[id];
@@ -468,18 +469,32 @@ ns.Model.get = function(id, params) {
468 469
 };
469 470
 
470 471
 /**
  472
+ * Returns valid cached model instance.
  473
+ * @param {String} id Model's ID.
  474
+ * @param {Object} [params] Model's params
  475
+ * @returns {ns.Model|null}
  476
+ */
  477
+ns.Model.getValid = function(id, params) {
  478
+    var model = this._find(id, params);
  479
+    if (model && model.isValid()) {
  480
+        return model;
  481
+    }
  482
+    return null;
  483
+};
  484
+
  485
+/**
471 486
  * Returns cached model instance.
472 487
  * @param {String} id Model's ID.
473 488
  * @param {Object} [params] Model's params
474  
- * @returns {ns.Model|undefined}
  489
+ * @returns {ns.Model|null}
475 490
  */
476  
-ns.Model.find = function(id, params) {
  491
+ns.Model._find = function(id, params) {
477 492
     if (!(id in _infos)) {
478 493
         throw new Error('[ns.Model] "' + id + '" is not defined');
479 494
     }
480 495
 
481 496
     var key = ns.Model.key(id, params);
482  
-    return _cache[id][key];
  497
+    return _cache[id][key] || null;
483 498
 };
484 499
 
485 500
 /**
@@ -496,9 +511,6 @@ ns.Model.destroy = function(model) {
496 511
 
497 512
     var cached = _cache[id][key];
498 513
     if (cached) {
499  
-        // remove from cache
500  
-        delete _cache[id][key];
501  
-
502 514
         // notify subscribers about disappearance
503 515
         model.trigger('ns-model-destroyed');
504 516
 
@@ -508,8 +520,8 @@ ns.Model.destroy = function(model) {
508 520
 };
509 521
 
510 522
 //  Проверяем, есть ли модель в кэше и валидна ли она.
511  
-ns.Model.isValid = function(id, key) {
512  
-    var model = ns.Model.get(id, key);
  523
+ns.Model.isValid = function(id, params) {
  524
+    var model = ns.Model.get(id, params);
513 525
     if (!model) { return; } // undefined означает, что кэша нет вообще, а false -- что он инвалидный.
514 526
 
515 527
     return model.isValid();
62  src/ns.view.js
@@ -97,17 +97,16 @@ ns.View.prototype._init = function(id, params, async) {
97 97
  * Инициализирует модели
98 98
  */
99 99
 ns.View.prototype._initModels = function() {
100  
-    // Создаёи модели или берем их из кэша, если они уже есть
101  
-    for (var model_id in this.info.models) {
102  
-        this._initModel(model_id);
103  
-    }
104  
-};
105  
-
106  
-ns.View.prototype._initModel = function(id) {
107 100
     if (!this.models) {
108 101
         this.models = {};
109 102
     }
110  
-    this.models[id] = ns.Model.get(id, this.params);
  103
+
  104
+    // Создаём модели или берем их из кэша, если они уже есть
  105
+    for (var id in this.info.models) {
  106
+        if (!this.models[id]) {
  107
+            this.models[id] = ns.Model.get(id, this.params);
  108
+        }
  109
+    }
111 110
 };
112 111
 
113 112
 //  ---------------------------------------------------------------------------------------------------------------  //
@@ -653,8 +652,7 @@ ns.View.prototype._bindModels = function() {
653 652
         var model = models[model_id];
654 653
 
655 654
         model.on('ns-model-destroyed', function() {
656  
-            delete that.models[this.id];
657  
-            that._initModel(this.id);
  655
+            that.invalidate();
658 656
         });
659 657
 
660 658
         var jpaths = subviews[model_id];
@@ -909,7 +907,7 @@ ns.View.prototype.isSubviewsOk = function() {
909 907
  * Возвращает true, если блок валиден.
910 908
  * @return {Boolean}
911 909
  */
912  
-ns.View.prototype.isValid = function() {
  910
+ns.View.prototype.isValid = ns.View.prototype.isValidSelf = function() {
913 911
     return this.isOk() && this.isSubviewsOk() && this.isModelsValidWithVersions();
914 912
 };
915 913
 
@@ -974,6 +972,32 @@ ns.View.prototype._apply = function(callback) {
974 972
  * @return {*}
975 973
  */
976 974
 ns.View.prototype._getRequestViews = function(updated, pageLayout, params) {
  975
+
  976
+    // При необходимости добавим текущий вид в список "запрашиваемых"
  977
+    this._tryPushToRequest(updated);
  978
+
  979
+    // Если views еще не определены (первая отрисовка)
  980
+    if (!this.views) {
  981
+        //  FIXME: Почему бы это в конструкторе не делать?
  982
+        this.views = {};
  983
+        // Создаем подблоки
  984
+        for (var view_id in pageLayout) {
  985
+            this._addView(view_id, params, pageLayout[view_id].type);
  986
+        }
  987
+    }
  988
+
  989
+    this._apply(function(view, id) {
  990
+        view._getRequestViews(updated, pageLayout[id].views, params);
  991
+    });
  992
+
  993
+    return updated;
  994
+};
  995
+
  996
+/**
  997
+ * Добавляет вид в соответствующий список "запрашиваемых" видов в случае,
  998
+ * если запрос необходим
  999
+ */
  1000
+ns.View.prototype._tryPushToRequest = function(updated) {
977 1001
     /**
978 1002
      * Флаг, означающий, что view грузится асинхронно.
979 1003
      * @type {Boolean}
@@ -994,25 +1018,11 @@ ns.View.prototype._getRequestViews = function(updated, pageLayout, params) {
994 1018
             // прекращаем обработку
995 1019
             return updated;
996 1020
         }
997  
-    } else if (!this.isValid()) {
  1021
+    } else if (!this.isValidSelf()) {
998 1022
         // если обычный блок не валиден
999 1023
         updated.sync.push(this);
1000 1024
     }
1001 1025
 
1002  
-    // Если views еще не определены (первая отрисовка)
1003  
-    if (!this.views) {
1004  
-        //  FIXME: Почему бы это в конструкторе не делать?
1005  
-        this.views = {};
1006  
-        // Создаем подблоки
1007  
-        for (var view_id in pageLayout) {
1008  
-            this._addView(view_id, params, pageLayout[view_id].type);
1009  
-        }
1010  
-    }
1011  
-
1012  
-    this._apply(function(view, id) {
1013  
-        view._getRequestViews(updated, pageLayout[id].views, params);
1014  
-    });
1015  
-
1016 1026
     return updated;
1017 1027
 };
1018 1028
 
38  src/ns.viewCollection.js
@@ -58,8 +58,7 @@ ns.ViewCollection.prototype._bindModels = function() {
58 58
         var model = models[model_id];
59 59
 
60 60
         model.on('ns-model-destroyed', function() {
61  
-            delete that.models[this.id];
62  
-            that._initModel(this.id);
  61
+            that.invalidate();
63 62
         });
64 63
 
65 64
         model.on('ns-model-changed', function(e, o) {
@@ -75,12 +74,6 @@ ns.ViewCollection.prototype.isValid = function() {
75 74
     return this.isValidSelf() && this.isValidDesc();
76 75
 };
77 76
 
78  
-/**
79  
- * Check self validity (except items).
80  
- * @returns {Boolean}
81  
- */
82  
-ns.ViewCollection.prototype.isValidSelf = ns.View.prototype.isValid;
83  
-
84 77
 ns.ViewCollection.prototype.isValidDesc = function() {
85 78
     for (var key in this.views) {
86 79
         if (!this.views[key].isValid()) {
@@ -159,34 +152,7 @@ ns.ViewCollection.prototype._apply = function(callback) {
159 152
     }
160 153
 };
161 154
 
162  
-ns.ViewCollection.prototype._getRequestViews = function(updated) {
163  
-    /**
164  
-     * Флаг, означающий, что view грузится асинхронно.
165  
-     * @type {Boolean}
166  
-     */
167  
-    this.asyncState = false;
168  
-
169  
-    if (this.async) {
170  
-        var hasValidModels = this.isModelsValid();
171  
-        var hasValidStatus = this.isOk();
172  
-        if (hasValidModels && !hasValidStatus) {
173  
-            // если асинхронный блок имеет валидные модели, но невалидный статус - рисуем его синхронно
174  
-            updated.sync.push(this);
175  
-
176  
-        } else if (!hasValidModels) {
177  
-            this.asyncState = true;
178  
-            // если асинхронный блок имеет невалидные модели, то его не надо рисовать
179  
-            updated.async.push(this);
180  
-            // прекращаем обработку
181  
-            return updated;
182  
-        }
183  
-    } else if (!this.isValidSelf()) {
184  
-        // если обычный блок не валиден
185  
-        updated.sync.push(this);
186  
-    }
187  
-
188  
-    return updated;
189  
-};
  155
+ns.ViewCollection.prototype._getRequestViews = ns.View.prototype._tryPushToRequest;
190 156
 
191 157
 ns.ViewCollection.prototype._getUpdateTree = function(tree, layout, params) {
192 158
     var decl;
2  test/spec/ns.box.js
@@ -68,7 +68,7 @@ describe('ns.Box', function() {
68 68
             },
69 69
             rewriteParamsOnInit: function(params) {
70 70
                 return {
71  
-                    pOwn: ns.Model.find('model4').get('.value')
  71
+                    pOwn: ns.Model.getValid('model4').get('.value')
72 72
                 };
73 73
             }
74 74
         });
19  test/spec/ns.model.js
@@ -136,19 +136,20 @@ describe('ns.Model', function() {
136 136
             });
137 137
         });
138 138
 
139  
-        describe('ns.Model.find():', function() {
  139
+        describe('ns.Model.getValid():', function() {
140 140
 
141  
-            it('should return undefined if model doesn\'t exists', function() {
142  
-                expect(ns.Model.find('m1')).to.be(undefined);
  141
+            it('should return null if model doesn\'t exists', function() {
  142
+                expect(ns.Model.getValid('m1')).to.be(null);
143 143
             });
144 144
 
145  
-            it('should return model if exists', function() {
  145
+            it('should return valid model if exists', function() {
146 146
                 var m = ns.Model.get('m1');
147  
-                expect(ns.Model.find('m1')).to.be.ok(m);
  147
+                m.setData({foo: 'bar'});
  148
+                expect(ns.Model.getValid('m1')).to.be.ok(m);
148 149
             });
149 150
 
150 151
             it('should throw exception if model is not defined', function() {
151  
-                var exists = function() { ns.Model.find('non-exists-model'); };
  152
+                var exists = function() { ns.Model.getValid('non-exists-model'); };
152 153
                 expect(exists).to.throwException();
153 154
             });
154 155
 
@@ -166,7 +167,7 @@ describe('ns.Model', function() {
166 167
             });
167 168
 
168 169
             it('should throw exception if model is not defined', function() {
169  
-                var exists = function() { ns.Model.find('non-exists-model'); };
  170
+                var exists = function() { ns.Model.getValid('non-exists-model'); };
170 171
                 expect(exists).to.throwException();
171 172
             });
172 173
 
@@ -551,7 +552,7 @@ describe('ns.Model', function() {
551 552
                 this.model2.destroyWith(this.model1);
552 553
                 ns.Model.destroy(this.model1);
553 554
 
554  
-                expect(ns.Model.find('model2', { id: 1 })).not.to.be.ok();
  555
+                expect(ns.Model.getValid('model2', { id: 1 })).not.to.be.ok();
555 556
             });
556 557
 
557 558
             it('should throw error if tried to destroy ns.Model with string', function() {
@@ -559,7 +560,7 @@ describe('ns.Model', function() {
559 560
             });
560 561
 
561 562
             it('should throw error if tried to destroy ns.Model with undefined', function() {
562  
-                expect(function() { this.model1.destroyWith(ns.Model.find('model2', {id: 2})); }).to.throwException();
  563
+                expect(function() { this.model1.destroyWith(ns.Model.getValid('model2', {id: 2})); }).to.throwException();
563 564
 
564 565
             });
565 566
         });
9  test/spec/ns.viewCollection.js
@@ -437,8 +437,10 @@ describe('ns.ViewCollection', function() {
437 437
                 .start()
438 438
                 .done(function() {
439 439
                     ns.Model.destroy(ns.Model.get('mCollection'));
  440
+                    ns.Model.destroy(ns.Model.get('mItem', {id: '0'}));
  441
+                    ns.Model.destroy(ns.Model.get('mItem', {id: '1'}));
440 442
 
441  
-                    ns.Model.get('mCollection').setData({data: [{id: 3}]});
  443
+                    ns.Model.get('mCollection').setData({data: [{id: '2'}]});
442 444
 
443 445
                     new ns.Update(this.APP, layout, {})
444 446
                         .start()
@@ -452,6 +454,11 @@ describe('ns.ViewCollection', function() {
452 454
             delete this.APP;
453 455
         });
454 456
 
  457
+        it('shouldn`t find destroyed models', function() {
  458
+            expect(ns.Model.getValid('mItem', {id: '0'})).not.to.be.ok();
  459
+            expect(ns.Model.getValid('mItem', {id: '1'})).not.to.be.ok();
  460
+        });
  461
+
455 462
         it('should have 1 node for view vItem', function() {
456 463
             expect(this.APP.node.querySelectorAll('.ns-view-vItem').length).to.be(1);
457 464
         });
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.