Repository: goldibex/targaryen
Pull request #96 (2016-12-09 02:47:18)

Fix data node snapshot priority handling
- Validate priority input.
- Make sure snapshot priority is null when undefined.


Patch:
diff --git a/lib/database.js b/lib/database.js
index 8db74a7..23ed713 100644
--- a/lib/database.js
+++ b/lib/database.js
@@ -214,7 +214,9 @@ class RuleDataSnapshot {
    * @return {string|number|null}
    */
   getPriority() {
-    return this[nodeKey].$priority();
+    const priority = this[nodeKey].$priority();
+
+    return priority === undefined ? null : priority;
   }
 
   /**
diff --git a/lib/store.js b/lib/store.js
index 8e3891c..c3c1baa 100644
--- a/lib/store.js
+++ b/lib/store.js
@@ -5,7 +5,8 @@
 'use strict';
 
 const paths = require('./paths');
-const primitives = new Set(['string', 'number', 'boolean']);
+const primitiveTypes = new Set(['string', 'number', 'boolean']);
+const priorityTypes = new Set(['string', 'number']);
 const invalidChar = ['.', '$', '[', ']', '#', '/'];
 
 /**
@@ -25,7 +26,7 @@ function isObject(value) {
  * @return {boolean}
  */
 function isPrimitive(value) {
-  return primitives.has(typeof value);
+  return primitiveTypes.has(typeof value);
 }
 
 /**
@@ -38,6 +39,18 @@ function isValidKey(key) {
   return !invalidChar.some(c => key.includes(c));
 }
 
+/**
+ * Test the priority is valid.
+ *
+ * It tests if it's either be a string or a number.
+ *
+ * @param  {any}  priority Value to test
+ * @return {Boolean}
+ */
+function isValidPriority(priority) {
+  return priorityTypes.has(typeof priority);
+}
+
 /**
  * Test the value is a server value.
  *
@@ -83,9 +96,18 @@ class DataNode {
    * @param  {number} now   The current update timestamp
    */
   constructor(value, now) {
+
     this[priorityKey] = value['.priority'];
     this[valueKey] = value['.value'];
 
+    if (this[priorityKey] != null && !isValidPriority(this[priorityKey])) {
+      throw new Error(`Invalid node priority ${this[priorityKey]}`);
+    }
+
+    if (this[valueKey] != null && !isPrimitive(this[valueKey])) {
+      throw new Error(`Invalid node value ${this[valueKey]}`);
+    }
+
     const keys = value['.value'] === undefined ? Object.keys(value) : [];
 
     keys.filter(isValidKey).forEach(key => {
@@ -287,7 +309,7 @@ class DataNode {
 
     });
 
-    return DataNode.from(copy, this.$priority, now);
+    return DataNode.from(copy, this.$priority(), now);
 
   }
 
@@ -342,7 +364,7 @@ class DataNode {
 
     });
 
-    return DataNode.from(newRoot, this.$priority, now || Date.now());
+    return DataNode.from(newRoot, this.$priority(), now || Date.now());
 
   }
 


Test patch:
diff --git a/test/spec/lib/database.js b/test/spec/lib/database.js
index 2263c7d..ee11e2e 100644
--- a/test/spec/lib/database.js
+++ b/test/spec/lib/database.js
@@ -84,7 +84,7 @@ describe('database', function() {
 
     describe('#getPriority', function() {
 
-      it.skip('gets the priority at the specified path', function() {
+      it('gets the priority at the specified path', function() {
         const data = {
           '.priority': 1,
           foo: {
diff --git a/test/spec/lib/store.js b/test/spec/lib/store.js
index e25fed1..8250e08 100644
--- a/test/spec/lib/store.js
+++ b/test/spec/lib/store.js
@@ -125,7 +125,7 @@ describe('store', function() {
       expect(data.$set('foo', value, 'something').foo.$priority()).to.equal('something');
     });
 
-    it.skip('should throw when setting invalid priority', function() {
+    it('should throw when setting invalid priority', function() {
       expect(() => store.create({'.value': 1, '.priority': {foo: 'bar'}})).to.throw();
       expect(() => store.create({'.value': 1, '.priority': true})).to.throw();