updates for comments, new validation method array_ints and tests for …

…it , fix statistics
Helioviewer-Project · Jun 10, 2024 · fdcc4dc · fdcc4dc
1 parent b4856f0
commit fdcc4dc
Show file tree

Hide file tree

Showing 9 changed files with 132 additions and 38 deletions.
diff --git a/docroot/index.php b/docroot/index.php
@@ -173,7 +173,7 @@ function loadModule($params) {
                 $actions_to_keep_stats_for = [
                     'getClosestImage', 
                     'takeScreenshot', 
-                    'postsScreenshot', 
+                    'postScreenshot', 
                     'getJPX', 
                     'getJPXClosestToMidPoint', 
                     'uploadMovieToYouTube', 
@@ -182,8 +182,7 @@ function loadModule($params) {
 
                 // Note that in addition to the above, buildMovie requests and
                 // addition to getTile when the tile was already in the cache.
-                if ( HV_ENABLE_STATISTICS_COLLECTION &&
-                    in_array($params['action'], $actions_to_keep_stats_for) ) {
+                if ( HV_ENABLE_STATISTICS_COLLECTION && in_array($params['action'], $actions_to_keep_stats_for) ) {
 
                     include_once HV_ROOT_DIR.'/../src/Database/Statistics.php';
                     $statistics = new Database_Statistics();

diff --git a/docroot/statistics/statistics.js b/docroot/statistics/statistics.js
@@ -18,7 +18,7 @@ var heirarchy = {
     "Images":["takeScreenshot","postScreenshot","getTile","getClosestImage","getJP2Image-web","getJP2Image-jpip","getJP2Image","downloadScreenshot","getJPX","getJPXClosestToMidPoint", "downloadImage"],
     "Movies":["buildMovie","getMovieStatus","queueMovie","postMovie","reQueueMovie","playMovie","downloadMovie","getUserVideos","getObservationDateVideos","uploadMovieToYouTube","checkYouTubeAuth","getYouTubeAuth"],
     "Events":["getEventGlossary", "events", "getEvents","getFRMs","getEvent","getEventFRMs","getDefaultEventTypes","getEventsByEventLayers","importEvents"],
-    "Data":["getRandomSeed","getDataSources","getJP2Header","getDataCoverage","getStatus","getNewsFeed","getDataCoverageTimeline","getClosestData","getSolarBodiesGlossary","getSolarBodies","getTrajectoryTime","sciScript-SSWIDL","sciScript-SunPy","getSciDataScript","updateDataCoverage","getEclipseImage"],
+    "Data":["getRandomSeed","getDataSources","getJP2Header","getDataCoverage","getStatus","getNewsFeed","getDataCoverageTimeline","getClosestData","getSolarBodiesGlossary","getSolarBodies","getTrajectoryTime","sciScript-SSWIDL","sciScript-SunPy","getSciDataScript","updateDataCoverage","getEclipseImage","getClosestImageDatesForSources"],
     "Other":["shortenURL", "goto", "getUsageStatistics","movie-notifications-granted","movie-notifications-denied","logNotificationStatistics","launchJHelioviewer", "saveWebClientState", "getWebClientState"],
     "WebGL":["getTexture","getGeometryServiceData"]
 };

diff --git a/scripts/hv_stats/hv_stats.py b/scripts/hv_stats/hv_stats.py
@@ -2271,9 +2271,9 @@ def add_harea_if_timestamp_in_range(start_time, end_time, color, alpha, label):
     "Total":["total","rate_limit_exceeded"],
     "Client Sites":["standard","embed","minimal"],
     "Images":["takeScreenshot","postScreenshot","getTile","getClosestImage","getJP2Image-web","getJP2Image-jpip","getJP2Image","downloadScreenshot","getJPX","getJPXClosestToMidPoint"],
-    "Movies":["buildMovie","getMovieStatus","queueMovie","postMovie","reQueueMovie","playMovie","downloadMovie","getUserVideos","getObservationDateVideos","uploadMovieToYouTube","checkYouTubeAuth","getYouTubeAuth"],
+    "Movies":["buildMovie","postMovie","getMovieStatus","queueMovie","postMovie","reQueueMovie","playMovie","downloadMovie","getUserVideos","getObservationDateVideos","uploadMovieToYouTube","checkYouTubeAuth","getYouTubeAuth"],
     "Events":["getEventGlossary","getEvents","getFRMs","getEvent","getEventFRMs","getDefaultEventTypes","getEventsByEventLayers","importEvents"],
-    "Data":["getRandomSeed","getDataSources","getJP2Header","getDataCoverage","getStatus","getNewsFeed","getDataCoverageTimeline","getClosestData","getSolarBodiesGlossary","getSolarBodies","getTrajectoryTime","sciScript-SSWIDL","sciScript-SunPy","getSciDataScript","updateDataCoverage","getEclipseImage"],
+    "Data":["getRandomSeed","getDataSources","getJP2Header","getDataCoverage","getStatus","getNewsFeed","getDataCoverageTimeline","getClosestData","getSolarBodiesGlossary","getSolarBodies","getTrajectoryTime","sciScript-SSWIDL","sciScript-SunPy","getSciDataScript","updateDataCoverage","getEclipseImage", "getClosestImageDatesForSources"],
     "Other":["shortenURL","getUsageStatistics","movie-notifications-granted","movie-notifications-denied","logNotificationStatistics","launchJHelioviewer", "saveWebClientState", "getWebClientState"],
     "WebGL":["getTexture","getGeometryServiceData"]
 };

diff --git a/src/Database/ImgIndex.php b/src/Database/ImgIndex.php
@@ -49,14 +49,14 @@ public function insertScreenshot($date, $imageScale, $roi, $watermark, $layers,
 
         $this->_dbConnect();
 
-		// old implementation removed for events strings
-		// used to be $this->events->serialize();
-		$old_events_layer_string = ""; 
+        // old implementation removed for events strings
+        // used to be $this->events->serialize();
+        $old_events_layer_string = ""; 
 
-		// old if events labels are shown switch , removed for new implementation
-		// used to be $this->eventsLabels;
-		$old_events_labels_bool = false; 
-		
+        // old if events labels are shown switch , removed for new implementation
+        // used to be $this->eventsLabels;
+        $old_events_labels_bool = false; 
+        
 
         $sql = sprintf(
                   "INSERT INTO screenshots "
@@ -549,7 +549,7 @@ public function getClosestDataBeforeAndAfter($date, $sourceId)
                  . "WHERE "
                  .     "sourceId " . " = %d AND "
                  .     "date "     . "> '%s' "
-                 . "ORDER BY date DESC "
+                 . "ORDER BY date ASC "
                  . "LIMIT 1;",
                  (int)$sourceId,
                  $this->_dbConnection->link->real_escape_string($datestr)

diff --git a/src/Database/MovieDatabase.php b/src/Database/MovieDatabase.php
@@ -56,19 +56,21 @@ public function insertMovie($startTime, $endTime, $reqObservationDate, $imageSca
 
         $startTime = isoDateToMySQL($startTime);
         $endTime   = isoDateToMySQL($endTime);
+
         if($reqObservationDate != false){
-	        $reqObservationDate   = '"'.$this->_dbConnection->link->real_escape_string(isoDateToMySQL($reqObservationDate)).'"';
+            $reqObservationDate   = '"'.$this->_dbConnection->link->real_escape_string(isoDateToMySQL($reqObservationDate)).'"';
         }else{
-	        $reqObservationDate   = "NULL";
+            $reqObservationDate   = "NULL";
         }
 
-		// old implementation removed for events strings
-		// used to be $this->events->serialize();
-		$old_events_layer_string = ""; 
+        // !Attention we are keeping those fields for current rows in movies and screenshots tables
+        // old implementation removed for events strings
+        // used to be $this->events->serialize();
+        $old_events_layer_string = ""; 
 
-		// old if events labels are shown switch , removed for new implementation
-		// used to be $this->eventsLabels;
-		$old_events_labels_bool = false; 
+        // old if events labels are shown switch , removed for new implementation
+        // used to be $this->eventsLabels;
+        $old_events_labels_bool = false; 
 
         $sql = sprintf(
                    'INSERT INTO movies '

diff --git a/src/Database/Statistics.php b/src/Database/Statistics.php
@@ -424,8 +424,10 @@ public function getUsageStatistics($resolution, $dateStart = null, $dateEnd = nu
         // Array to keep track of counts for each action
         $counts = array(
             "buildMovie"                       => array(),
+            "postMovie"                        => array(),
             "getClosestData"                   => array(),
             "getClosestImage"                  => array(),
+            "getClosestImageDatesForSources"   => array(),
             "getJPX"                           => array(),
             "getJPXClosestToMidPoint"          => array(),
             "takeScreenshot"                   => array(),
@@ -447,10 +449,12 @@ public function getUsageStatistics($resolution, $dateStart = null, $dateEnd = nu
         // Summary array
         $summary = array(
             "buildMovie"                       => 0,
+            "postMovie"                        => 0,
             "getClosestData"                   => 0,
             "getClosestImage"                  => 0,
             "getJPX"                           => 0,
             "getJPXClosestToMidPoint"          => 0,
+            "getClosestImageDatesForSources"   => 0,
             "takeScreenshot"                   => 0,
             "postScreenshot"                   => 0,
             "uploadMovieToYouTube"             => 0,
@@ -832,6 +836,7 @@ private function _createCountsArray(){
             'getWebClientState'           => array(),
             'goto'                        => array(),
             'takeScreenshot'              => array(),
+            'postScreenshot'              => array(),
             'getRandomSeed'               => array(),
             'getJP2Image'                 => array(),
             'getJPX'                      => array(),
@@ -865,6 +870,7 @@ private function _createCountsArray(){
             'getGeometryServiceData'      => array(),
             'buildMovie'                  => array(),//this one happens in HelioviewerMovie.php
             "getClosestData"              => array(),
+            "getClosestImageDatesForSources" => array(),
             "embed"                       => array(),
             "minimal"                     => array(),
             "standard"                    => array(),
@@ -883,6 +889,7 @@ private function _createSummaryArray(){
             "total"                       => 0,
             'downloadScreenshot'          => 0,
             'getClosestImage'             => 0,
+            "getClosestImageDatesForSources" => 0,
             'getDataSources'              => 0,
             'getJP2Header'                => 0,
             'getNewsFeed'                 => 0,
@@ -899,6 +906,7 @@ private function _createSummaryArray(){
             'getWebClientState'           => 0,
             'goto'                        => 0,
             'takeScreenshot'              => 0,
+            'postScreenshot'              => 0,
             'getRandomSeed'               => 0,
             'getJP2Image'                 => 0,
             'getJPX'                      => 0,

diff --git a/src/Module/WebClient.php b/src/Module/WebClient.php
@@ -129,12 +129,9 @@ public function getClosestImageDatesForSources() {
 
         $imgIndex = new Database_ImgIndex();
 
-        $sources = trim($this->_params['sources']);
-        $source_ids = explode(',', $sources);
-
         $results = [];
 
-        foreach($source_ids as $sid) {
+        foreach($this->_params['sources'] as $sid) {
             $closestImages = $imgIndex->getClosestDataBeforeAndAfter($this->_params['date'], $sid);
             $results[$sid]['prev_date'] = $closestImages['prev_date'];
             $results[$sid]['next_date'] = $closestImages['next_date'];
@@ -1696,6 +1693,7 @@ public function validate() {
             $expected = array(
                'required' => array('date', 'sources'),
                'dates'    => array('date'),
+               'array_ints'  => array('sources'),
             );
             break;
         case 'getDataSources':

diff --git a/src/Validation/InputValidator.php b/src/Validation/InputValidator.php
@@ -34,18 +34,19 @@ public static function checkInput(&$expected, &$input, &$optional)
     {
         // Validation checks
         $checks = array(
-            "required" => "checkForMissingParams",
-            "alphanum" => "checkAlphaNumericStrings",
-            "ints"     => "checkInts",
-            "floats"   => "checkFloats",
-            "bools"    => "checkBools",
-            "dates"    => "checkDates",
-            "encoded"  => "checkURLEncodedStrings",
-            "urls"     => "checkURLs",
-            "files"    => "checkFilePaths",
-            "uuids"    => "checkUUIDs",
-            "layer"    => "checkLayerValidity",
-            "choices"  => "checkChoices"
+            "required"   => "checkForMissingParams",
+            "alphanum"   => "checkAlphaNumericStrings",
+            "ints"       => "checkInts",
+            "array_ints" => "checkOfArrayInts",
+            "floats"     => "checkFloats",
+            "bools"      => "checkBools",
+            "dates"      => "checkDates",
+            "encoded"    => "checkURLEncodedStrings",
+            "urls"       => "checkURLs",
+            "files"      => "checkFilePaths",
+            "uuids"      => "checkUUIDs",
+            "layer"      => "checkLayerValidity",
+            "choices"    => "checkChoices"
         );
 
         // Run validation checks
@@ -211,6 +212,36 @@ public static function checkInts($ints, &$params)
         }
     }
 
+
+    /**
+     * Typecasts validates and fixes types for array integer parameters
+     *
+     * @param array $ints    A list of integer array parameters which are used by an action.
+     * @param array &$params The parameters that were passed in
+     *
+     * @return void
+     */
+    public static function checkOfArrayInts($ints, &$params)
+    {
+        foreach ($ints as $int) {
+            if (isset($params[$int])) {
+
+
+                $integers_to_check = explode(',',$params[$int]);
+                $validated_ints = [];
+
+                foreach($integers_to_check as $itc) {
+                    if (filter_var(trim($itc), FILTER_VALIDATE_INT) === false) {
+                        throw new InvalidArgumentException("Invalid value for $int. Please specify an integer array value, as ex:1,2,3", 25);
+                    }
+                    $validated_ints[] = (int) trim($itc);
+                }
+
+                $params[$int] = $validated_ints;
+            }
+        }
+    }
+
     /**
      * Typecasts validates and fixes types for float parameters
      *

diff --git a/tests/unit_tests/validation/ValidatorTest.php b/tests/unit_tests/validation/ValidatorTest.php
@@ -61,4 +61,60 @@ public function test_ValidateLayerArray_MultipleLayers(): void
         // that no exception was raised.
         $this->assertTrue(true);
     }
+
+    public function test_ValidateArrayIntegersProblem1(): void
+    {
+        // The expected layer string to be given
+        $input = array(
+            'sources' => ''
+        );
+
+        $rules = array(
+            'array_ints' => array('sources')
+        );
+
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Invalid value for sources. Please specify an integer array value, as ex:1,2,3');
+        Validation_InputValidator::checkInput($rules, $input, $input) ;
+        // checkInput will raise an exception if it fails, so assertTrue means
+        // that no exception was raised.
+    }
+
+    public function test_ValidateArrayIntegersProblem2(): void
+    {
+        // The expected layer string to be given
+        $input = array(
+            'sources' => 'a,1,2'
+        );
+
+        $rules = array(
+            'array_ints' => array('sources')
+        );
+
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('Invalid value for sources. Please specify an integer array value, as ex:1,2,3');
+        Validation_InputValidator::checkInput($rules, $input, $input) ;
+        // checkInput will raise an exception if it fails, so assertTrue means
+        // that no exception was raised.
+    }
+
+    public function test_ValidateArrayIntegersCorrectly(): void
+    {
+        // The expected layer string to be given
+        $input = array(
+            'sources' => '4,1,2'
+        );
+
+        $rules = array(
+            'array_ints' => array('sources')
+        );
+
+        Validation_InputValidator::checkInput($rules, $input, $input) ;
+
+        $this->assertEquals($input, [
+            'sources' => [4,1,2]
+        ]);
+        // checkInput will raise an exception if it fails, so assertTrue means
+        // that no exception was raised.
+    }
 }