1. Kenneth Jørgensen
  2. discrete

Commits

Kenneth Jørgensen  committed 0c545fb

Changed the API for the Loader to prevent adding keys which may be mistaken for IDs

  • Participants
  • Parent commits 5e56255
  • Branches default

Comments (0)

Files changed (5)

File coffeelint.coffee

View file
 	"no_throwing_strings":
 		"level": "error"
 	"cyclomatic_complexity":
-		"value": 18
+		"value": 19
 		"level": "error"
 	"no_backticks":
 		"level": "error"

File discrete.js

View file
     RepoPersistor.reset();
 
     RepoPersistor.prototype.load = function(id, callback) {
-      var model,
-        _this = this;
+      var err, model;
       model = repo.get(id);
       if (model == null) {
-        _.defer(function() {
-          return callback(new Error("not-found"), null);
-        });
+        err = new Error("not-found");
+        (function(err) {
+          return _.defer(function() {
+            return callback(err, null);
+          });
+        })(err);
         return;
       }
       if (_.isFunction(callback)) {
-        return _.defer(function() {
-          return callback(null, model);
-        });
+        return (function(model) {
+          return _.defer(function() {
+            return callback(null, model);
+          });
+        })(model);
       }
     };
 
       id = model.id();
       model = repo.put(model);
       if (_.isFunction(callback)) {
-        return _.defer(function() {
-          return callback(null, model);
-        });
+        return (function(model) {
+          return _.defer(function() {
+            return callback(null, model);
+          });
+        })(model);
       }
     };
 
       return this.persistor = new persistor();
     };
 
-    Loader.prototype.add = function(name, model) {
-      var i, key, val, _i, _len,
+    Loader.prototype.add = function(models) {
+      var id, key, map, model, name, val, _i, _len,
         _this = this;
-      if (name == null) {
-        return this;
+      if (!models) {
+        throw new Error("No models supplied");
       }
-      if (_.isArray(name)) {
-        for (_i = 0, _len = name.length; _i < _len; _i++) {
-          i = name[_i];
-          this.add(i);
+      if (arguments.length !== 1) {
+        throw new Error("Expected one argument, " + arguments.length + " supplied");
+      }
+      map = {};
+      if (models instanceof Collection) {
+        models = models.toJSON();
+      }
+      if (_.isArray(models)) {
+        for (_i = 0, _len = models.length; _i < _len; _i++) {
+          model = models[_i];
+          if (model == null) {
+            continue;
+          }
+          if (model instanceof Model) {
+            id = model.id();
+            if (id == null) {
+              throw new Error("Model '" + model.type + "' does not have an ID");
+            }
+            map[id] = model;
+          } else {
+            map[model] = model;
+          }
         }
-        return this;
+      } else if (models instanceof Map) {
+        models.each(function(key, model) {
+          return map[key] = model;
+        });
+      } else if (models instanceof Model) {
+        map[models.id()] = models;
+      } else if (_.isObject(models)) {
+        for (key in models) {
+          if (!__hasProp.call(models, key)) continue;
+          val = models[key];
+          map[key] = val;
+        }
+      } else if (_.isString(models) || _.isNumber(models)) {
+        map[models] = models;
+      } else {
+        throw new Error("Models must be either Collection, array, Map, Model, Object, or string or number, '" + (typeof models) + "' supplied");
       }
-      if (name instanceof Collection) {
-        name.each(function(i) {
-          return _this.add(i);
-        });
-        return this;
-      }
-      if (name instanceof Map) {
-        name.each(function(key, val) {
-          return _this.add(key, val);
-        });
-        return this;
-      }
-      if (_.isObject(name) && !(name instanceof Model)) {
-        for (key in name) {
-          if (!__hasProp.call(name, key)) continue;
-          val = name[key];
-          this.add(key, val);
+      for (name in map) {
+        if (!__hasProp.call(map, name)) continue;
+        model = map[name];
+        if (_.isObject(model) && !(model instanceof Model)) {
+          throw new Error("Non-model object supplied for model");
         }
-        return this;
-      }
-      if (name instanceof Model) {
-        model = name;
-        name = model.id();
-      } else if (!(name instanceof Model) && (model == null)) {
-        model = name;
-      }
-      if (_.isObject(model) && !(model instanceof Model)) {
-        throw new Error("Non-model object supplied for model");
-      }
-      if (name == null) {
-        throw new Error("Name is not defined");
-      }
-      if (model == null) {
-        throw new Error("Name is not defined");
-      }
-      this._models[name] = model;
-      if (this._queue) {
-        this._queue.push({
-          name: name,
-          model: model
-        });
+        this._models[name] = model;
+        if (this._queue) {
+          this._queue.push({
+            name: name,
+            model: model
+          });
+        }
       }
       return this;
     };
 
+    Loader.prototype._addSingle = function(key, model) {};
+
     Loader.prototype.get = function(name) {
       return this._models[name];
     };
         });
         return Async.waterfall(handlers, function(err) {
           if (err) {
-            throw err;
+            return done(err);
+          } else {
+            return done(null);
           }
-          return done();
         });
       };
       this._queue = queue = Async.queue(worker, this.concurrency);

File spec/LoaderSpec.coffee

View file
 		expect(loader.add "id:0").toBe loader
 		expect(loader.get "id:0").toBe "id:0"
 		# With custom index and id.
-		expect(loader.add "foo", "id:1").toBe loader
+		expect(loader.add foo: "id:1").toBe loader
 		expect(loader.get "foo").toBe "id:1"
 		# No custom index and model.
 		expect(loader.add m1).toBe loader
 		expect(loader.get "id:111").toBe m1
 		expect(loader.get "id:222").toBeUndefined()
 		# With custom index and model.
-		expect(loader.add "test", m2).toBe loader
+		expect(loader.add test: m2).toBe loader
 		expect(loader.get "test").toBe m2
 		expect(loader.get "id:222").toBeUndefined()
 		# Return all.
 		expect(loader.get "id:333").toBeUndefined()
 
 	it "should load IDs, load relations, and poll on completion", ->
-		loader.add "m1", "id:111"
+		loader.add m1: "id:111"
 		loader.poll poll = sinon.spy (loader, name, model) ->
 			if name is "m1"
-				loader.add "m2", model.get "forward"
+				loader.add m2: model.get "forward"
 		loader.load done
 		waitsFor (-> done.called), "Done never called", 100
 		runs ->
 			expect(models["id:333"]).toBeUndefined()
 
 	it "should complain if adding non-models", ->
-		test = -> loader.add "index", {}
+		test = -> loader.add index: {}
 		expect(test).toThrow "Non-model object supplied for model"
+
+	it "should complain when adding models wrong", ->
+		expect(-> loader.add "key", "val").toThrow "Expected one argument, 2 supplied"

File src/Loader.coffee

View file
 		return @persistor = new persistor()
 
 	# Adds a model to the loader.
-	add: (name, model) ->
-		#throw new Error "Models cannot be added to a completed Loader" if @completed
-		return @ if !name?
-		# Arrays.
-		if _.isArray name
-			for i in name
-				@add i
-			return @
-		# Collections.
-		if name instanceof Collection
-			name.each (i) =>
-				@add i
-			return @
-		# Maps.
-		if name instanceof Map
-			name.each (key, val) =>
-				@add key, val
-			return @
-		# Non-Model objects.
-		if _.isObject(name) and not (name instanceof Model)
-			for own key, val of name
-				@add key, val
-			return @
+	add: (models) ->
+		throw new Error "No models supplied" unless models
+		throw new Error "Expected one argument, #{arguments.length} supplied" unless arguments.length is 1
+		map = {}
+		# Convert collection to array.
+		if models instanceof Collection
+			models = models.toJSON()
+		# Convert array to object.
+		if _.isArray models
+			for model in models
+				continue unless model?
+				# Models.
+				if model instanceof Model
+					id = model.id()
+					throw new Error "Model '#{model.type}' does not have an ID" unless id?
+					map[id] = model
+				# IDs.
+				else
+					map[model] = model
+		# Convert Map objects.
+		else if models instanceof Map
+			models.each (key, model) =>
+				map[key] = model
+		# Single models.
+		else if models instanceof Model
+			map[models.id()] = models
+		# Actual map objects.
+		else if _.isObject models
+			for own key, val of models
+				map[key] = val
+		# Single stings.
+		else if _.isString(models) or _.isNumber(models)
+			map[models] = models
+		else
+			throw new Error "Models must be either Collection, array, Map, Model, Object, or string or number, '#{typeof models}' supplied"
+		# Everything valid has now been converted to a simple map object, add it all to be loaded.
+		for own name, model of map
+			if _.isObject(model) and not (model instanceof Model)
+				throw new Error "Non-model object supplied for model"
+			#console.log name, model
+			# Add to model container.
+			@_models[name] = model
+			# Push to queue.
+			if @_queue
+				@_queue.push
+					name: name
+					model: model
+		return @
 
-		# No custom name and model
-		if name instanceof Model
-			model = name
-			name = model.id()
-		# Single ID.
-		else if not (name instanceof Model) and not model?
-			model = name
-		# Non-model objects.
-		if _.isObject(model) and not (model instanceof Model)
-			throw new Error "Non-model object supplied for model"
+	# Adds a single model to be loaded.
+	# This an internal function.
+	# `add()` should be used instead.
+	_addSingle: (key, model) ->
 
-		throw new Error "Name is not defined" unless name?
-		throw new Error "Name is not defined" unless model?
-
-		# Add to model container.
-		@_models[name] = model
-		# Push to queue.
-		if @_queue
-			@_queue.push
-				name: name
-				model: model
-		return @
 
 	# Returns an added model.
 	get: (name) ->
 				done()
 			# Execute task.
 			Async.waterfall handlers, (err) =>
-				throw err if err
-				done()
+				if err
+					done err
+				else
+					done null
 
 		# Start queue.
 		@_queue = queue = Async.queue worker, @concurrency

File src/RepoPersistor.coffee

View file
 	load: (id, callback) ->
 		model = repo.get id
 		unless model?
-			_.defer -> callback new Error("not-found"), null
+			err = new Error("not-found")
+			do (err) -> _.defer -> callback err, null
 #			callback new Error("not-found"), null
 			return
 		if _.isFunction callback
-			_.defer => callback null, model
+			do (model) -> _.defer -> callback null, model
 #			callback null, model
 
 	# Saves a model.
 		id = model.id()
 		model = repo.put model
 		if _.isFunction callback
-			_.defer -> callback null, model
+			do (model) -> _.defer -> callback null, model
 
 	# Returns the repo.
 	getRepo: ->