More complete tests for getData and associated simplification of the getData code.

2 files changed, 68 insertions, 40 deletions

diff --git a/jarmon/jarmon.js b/jarmon/jarmon.js
index 3f63b3e..6ac410c 100644
--- a/jarmon/jarmon.js
+++ b/jarmon/jarmon.js
@@ -165,16 +165,11 @@ jarmon.RrdQuery.prototype.getData = function(startTimeJs, endTimeJs, dsId, cfNam
              'endtime: ', endTimeJs].join(''));
     }
 
-    // The startTime, endTime and lastupdate time are not necessarily on a step
-    // boundaries. Here we divide, round and then multiply by the step size to
-    // find the nearest "Primary Data Point" (PDP) time.
-    console.log('RRD: ', this.rrd);
-    var minStep = this.rrd.getMinStep();
-    var startTime = Math.round(startTimeJs/1000/minStep) * minStep;
+    var startTime = startTimeJs/1000;
     var lastUpdated = this.rrd.getLastUpdate();
-    var lastPdpTime = Math.round(lastUpdated / minStep) * minStep;
 
-    var endTime = lastPdpTime;
+    // default endTime to the last updated time (quantized to rrd step boundry)
+    var endTime = lastUpdated - lastUpdated%this.rrd.getMinStep();
     if(endTimeJs) {
         endTime = endTimeJs/1000;
     }
@@ -202,8 +197,7 @@ jarmon.RrdQuery.prototype.getData = function(startTimeJs, endTimeJs, dsId, cfNam
 
         step = rra.getStep();
         rraRowCount = rra.getNrRows();
-
-        lastRowTime = Math.round(lastUpdated/step)*step;
+        lastRowTime = lastUpdated-lastUpdated%step;
         firstRowTime = lastRowTime - rraRowCount * step;
 
         // We assume that the RRAs are listed in ascending order of time range,
@@ -219,36 +213,30 @@ jarmon.RrdQuery.prototype.getData = function(startTimeJs, endTimeJs, dsId, cfNam
         throw TypeError('Unrecognised consolidation function: ' + cfName);
     }
 
-    var startRowTime = Math.floor(startTime/step)*step + step;
-    var endRowTime = Math.floor(endTime/step)*step + step;
-
     var flotData = [];
-    var timestamp = startRowTime;
+    var dsIndex = ds.getIdx();
 
-    // Fill in any blank values at the start of the query, before we reach the
-    // first data in the chosen RRA
-    while(timestamp<=endRowTime && timestamp<firstRowTime) {
-        flotData.push([timestamp*1000.0, null]);
-        timestamp+=step
-    }
+    var startRowTime = startTime - startTime%step;
+    var endRowTime = endTime - endTime%step;
 
-    var dsIndex = ds.getIdx();
-    var val
-    var i = rraRowCount - 1 - (lastRowTime - timestamp)  / step;
-    while(timestamp<=endRowTime && timestamp < lastRowTime) {
-        val = rra.getEl(i, dsIndex)
-        flotData.push([timestamp*1000.0, val]);
-        i += 1;
-        timestamp += step
-    }
+    //console.log('FRT: ', new Date(startRowTime*1000));
+    //console.log('ERT: ', new Date(endRowTime*1000));
+    //console.log('LRT: ', new Date(lastRowTime*1000));
+    //console.log('DIFF: ', (lastRowTime - startRowTime) / step);
+    //console.log('ROWS: ', rraRowCount);
 
+    var startRowIndex = rraRowCount - (lastRowTime - startRowTime)  / step;
+    var endRowIndex = rraRowCount - (lastRowTime - endRowTime)  / step;
 
+    //console.log('SRI: ', startRowIndex);
+    //console.log('ERI: ', endRowIndex);
 
-    // Fill in any blank values at the end of the query, after we've used all
-    // the data in the chosen RRA
-    while(timestamp<=endRowTime) {
-        flotData.push([timestamp*1000.0, null]);
-        timestamp+=step
+    var val;
+    var timestamp = startRowTime;
+    for(var i=startRowIndex; i<endRowIndex; i++) {
+        val = rra.getEl(i, dsIndex)
+        flotData.push([timestamp*1000.0, val]);
+        timestamp += step
     }
 
     // Now get the date of the earliest record in entire rrd file, ie that of
diff --git a/jarmon/jarmon.test.js b/jarmon/jarmon.test.js
index 9e8eaca..0ff1840 100644
--- a/jarmon/jarmon.test.js
+++ b/jarmon/jarmon.test.js
@@ -218,19 +218,61 @@ YUI({ logInclude: { TestRunner: true } }).use('console', 'test', function(Y) {
                 function(self, rrd) {
                     self.resume(function() {
                         var rq = new jarmon.RrdQuery(rrd, '');
-                        var data = rq.getData(RRD_STARTTIME, RRD_ENDTIME);
-                        Y.Assert.areEqual(RRD_RRAROWS+1, data.data.length);
-                        Y.Assert.areEqual(2, data.data[2][1]);
+
+                        // We request data starting 1 STEP +1s after the RRD file
+                        // first val and ending 1 STEP -1s before the RRD last val
+                        // ie one step within the RRD file, but 1s away from the
+                        // step boundary to test the quantisation of the
+                        // requested time range.
+                        var data = rq.getData(
+                            RRD_STARTTIME + (RRD_STEP+1) * 1000,
+                            RRD_ENDTIME - (RRD_STEP-1) * 1000);
+
+                        // so we expect two less rows than the total rows in the
+                        // file.
+                        Y.Assert.areEqual(RRD_RRAROWS-2, data.data.length);
+
+                        // The value of the first returned row should be the
+                        // second value in the RRD file (starts at value 0)
+                        Y.Assert.areEqual(1, data.data[0][1]);
+
+                        // The value of the last returned row should be the
+                        // 10 value in the RRD file (starts at value 0)
+                        Y.Assert.areEqual(10, data.data[data.data.length-1][1]);
+
+                        // The timestamp of the first returned row should be
+                        // exactly one step after the start of the RRD file
                         Y.Assert.areEqual(
                             RRD_STARTTIME+RRD_STEP*1000, data.data[0][0]);
+
+                        // RRD_ENDTIME is on a step boundary and is therfore
+                        // actually the start time of a new row
+                        // So when we ask for endTime = RRD_ENDTIME-STEP-1 we
+                        // actually get data up to the 2nd to last RRD row.
                         Y.Assert.areEqual(
-                            RRD_ENDTIME + RRD_STEP*1000,
+                            RRD_ENDTIME-RRD_STEP*1000*2,
                             data.data[data.data.length-1][0]);
                     });
                 }, this);
             this.wait();
         },
 
+        test_getDataUnknownValues: function () {
+            /**
+             * If the requested time range is outside the range of the RRD file
+             * we should not get any values back
+             **/
+            this.d.addCallback(
+                function(self, rrd) {
+                    self.resume(function() {
+                        var rq = new jarmon.RrdQuery(rrd, '');
+                        var data = rq.getData(RRD_ENDTIME, RRD_ENDTIME+1000);
+                        Y.Assert.areEqual(0, data.data.length);
+                    });
+                }, this);
+            this.wait();
+        }
+
     }));
 
 
@@ -242,7 +284,5 @@ YUI({ logInclude: { TestRunner: true } }).use('console', 'test', function(Y) {
         height: '400px'
     });
     yconsole.render('#log');
-
-    //run all tests
     Y.Test.Runner.run();
 });