Skip to content

Commit

Permalink
Add new functionality to Usage chart requests (#716)
Browse files Browse the repository at this point in the history
* Add new functionality to Usage chart requests

Added the ability for Usage chart requests to provide non-numeric values for
'_filter' parameters. The new feature will attempt to translate these
non-numeric values into appropriate id values that can be used during chart
generation. Specifically this new feature is to support integration w/
Coldfront.

* Updates to support multiple string values

* Style fix

* Updating documentation

* Simplification of the `translateValue` function per @jpwhite4

Per code review comments I've simplified this function.
  - Moved the `is_numeric` check to the top of the function and used it as a
    short circuit.
  - Removed the anonymous function and moved the code it contained to the bottom
    of the function, executed contingent on which `case`, if any, is hit.

* New integration tests for the newly added Usage functionality

This new test checks that the `_filter` variables accept non-id
values (optionally, multiple comma delimited values). The test data includes the
following:
  - Tests run as all users ( pub, cd, cs, pi, usr )
  - Multiple Realms ( Jobs, Cloud )
  - For the Jobs realm, all combinations of: ( including single, multiple,
    numeric & non-numeric values )
    - 'resource', 'resource_filter'
    - 'pi', 'pi_filter'
  - For the Cloud realm: ( including single, multiple, numeric, and non-numeric
    values )
    - 'project', 'project_filter'

* Style Fix

* Updates per @jpwhite4 comment

As this function should only ever return one value the code has been updated so
that this is the case.

* Updates per convo w/ @smgallo

- Updated `translateFilterValue` to return an array of values and the calling code
to work with an array as opposed to a single value. This is to support the use
case of a system account being associated with more than one person.

- Updated `translateFilterValue` to only interpret `$usageFilterValue` as a
  string if it contains quotes ( single or double ). This allows us to identify
  when the calling code is passing us a value that is intended to be
  interpreted as a string not a number.

- Updated Test Cases so that string values are quoted with both single / double
  quotes.

- Added additional test cases that test what happens when a string value is
  provided that is not found in the db.

* Refactor / Simplification of Usage Filter generation

Refactored a portion of the `convertChartRequest` function and combined the two
`for-each` loops that were populating the `$meFilters` variable. Now we have one
loop that has the same functionality.

Also a tiny comment update in the `translateFilterValue` function for the
'pi' filter type. Now that we always return an array the previous comment didn't
really make sense anymore.

* Simplified `translateFilterValue`

Now that we have a reliable test for what constitutes a "string" ( i.e. must be
quoted ) we can simplify the internals of `translateFilterValue` by removing the
vast majority of the if/elseif block. Also added a few comments to help provide
context.

* Providing more informative exception info

Added `$usageFilterType` to the message for the exception thrown in
`translateFilterValue`.

* Updating the translateFilterValue exception message

Added the invalid `$usageFilterValue` to the exception message and updated the
section in the tests that handled detecting when exceptions are thrown to handle
this update.

* Update classes/DataWarehouse/Access/Usage.php

Co-Authored-By: ryanrath <ryanrath@buffalo.edu>
  • Loading branch information
ryanrath authored Dec 10, 2018
1 parent 133a3ae commit 6df963f
Show file tree
Hide file tree
Showing 2 changed files with 445 additions and 29 deletions.
128 changes: 100 additions & 28 deletions classes/DataWarehouse/Access/Usage.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

namespace DataWarehouse\Access;

use CCR\DB;
use CCR\Log;
use Exception;

use DataWarehouse;
use DataWarehouse\Access\MetricExplorer;
use Models\Services\Acls;
use PDO;
use XDChartPool;
use XDUser;

Expand Down Expand Up @@ -963,25 +966,6 @@ private function convertChartRequest(array $usageRequest, $useGivenFormat) {
$usageFilterSuffix = '_filter';
$usageFilterSuffixLength = strlen($usageFilterSuffix);
$usageRealm = \xd_utilities\array_get($usageRequest, 'realm');
$meFilters = array();
foreach ($usageRequest as $usageKey => $usageValue) {
if (!\xd_utilities\string_ends_with($usageKey, $usageFilterSuffix)) {
continue;
}

$usageFilterType = substr($usageKey, 0, -$usageFilterSuffixLength);
$usageFilterValues = explode(',', $usageValue);

foreach ($usageFilterValues as $usageFilterValue) {
$meFilters[] = array(
'id' => "$usageFilterType=$usageFilterValue",
'value_id' => $usageFilterValue,
'dimension_id' => $usageFilterType,
'realms' => array($usageRealm),
'checked' => true,
);
}
}

// Create global filters from any Usage drilldowns.
$realmAggregateClass = "\\DataWarehouse\\Query\\$usageRealm\\Aggregate";
Expand All @@ -991,18 +975,42 @@ private function convertChartRequest(array $usageRequest, $useGivenFormat) {
$realmGroupBy = new $realmGroupByClass();
return $realmGroupBy->getName();
}, $realmGroupByClasses);

$meFilters = array();

// Extract the supported filter values from $usageRequest
foreach ($usageRequest as $usageKey => $usageValue) {
if (!in_array($usageKey, $realmGroupByNames)) {
continue;

// handles '<dimension>_filter' properties
if (\xd_utilities\string_ends_with($usageKey, $usageFilterSuffix)) {
$usageFilterType = substr($usageKey, 0, -$usageFilterSuffixLength);
$usageFilterValues = explode(',', $usageValue);

foreach ($usageFilterValues as $usageFilterValue) {
$translatedValues = $this->translateFilterValue($usageFilterType, $usageFilterValue);

foreach($translatedValues as $translatedValue) {
$meFilters[] = array(
'id' => "$usageFilterType=$translatedValue",
'value_id' => $translatedValue,
'dimension_id' => $usageFilterType,
'realms' => array($usageRealm),
'checked' => true,
);
}
}
}

$meFilters[] = array(
'id' => "$usageKey=$usageValue",
'value_id' => $usageValue,
'dimension_id' => $usageKey,
'realms' => array($usageRealm),
'checked' => true,
);
// handles '<dimension>' properties
if (in_array($usageKey, $realmGroupByNames)) {
$meFilters[] = array(
'id' => "$usageKey=$usageValue",
'value_id' => $usageValue,
'dimension_id' => $usageKey,
'realms' => array($usageRealm),
'checked' => true,
);
}
}

// Store the global filters in the Metric Explorer request.
Expand Down Expand Up @@ -1107,4 +1115,68 @@ private function generateFilename($title, $start_date, $end_date, $isTimeseries)
. ($isTimeseries ? 'timeseries' : 'aggregate')
;
}

/**
* Attempts to translate the $usageFilterValue which is either a quoted string ( single or double ),
* numeric string, or number via a sql query, based on $usageFilterType. If a non-quoted string
* is supplied it will be treated as a number. This affects the logic of what values are returned.
* Currently supported $usageFilterType values and the tables ( columns ) they lookup values
* in are:
*
* - 'pi': modw.systemaccount ( username )
* - 'resource': modw.resourcefact ( code )
* - 'project': modw_cloud.account ( display )
*
* If the $usageFilterValue is numeric or the $usageFilterType is not one of those currently
* supported then array($usageFilterValue) is returned.
*
* @param string $usageFilterType the 'type' of filter to translate. Currently supported
* values: pi, resource, project
* @param string|int $usageFilterValue the value to be translated
* @return array of the translated values.
* @throws Exception if there is a problem connecting to the db or executing a query.
*/
private function translateFilterValue($usageFilterType, $usageFilterValue)
{

$query = null;

switch ($usageFilterType) {
case 'pi':
// This query may return multiple values
$query = "SELECT DISTINCT sa.person_id AS value FROM modw.systemaccount sa WHERE sa.username = :value;";
break;
case 'resource':
$query = "SELECT id AS value FROM modw.resourcefact WHERE code = :value";
break;
case 'project':
$query = "SELECT account_id AS value FROM modw_cloud.account WHERE display = :value";
break;
}

$filterValueIsString = preg_match('/^[\'"].*[\'"]$/', $usageFilterValue) === 1;

// We only attempt translation if we support the `$usageFilterType` provided and
// the `$usageFilterValue` is a quoted string.
if ($query !== null && $filterValueIsString) {
$value = trim($usageFilterValue, '\'"');

$db = DB::factory('database');

$stmt = $db->prepare($query);
$stmt->execute(array(':value' => $value));

$rows = $stmt->fetchAll(PDO::FETCH_COLUMN, 0);

// We need to test if there was an error ( bool returned ) because count(bool) === 1.
if ($rows !== false && count($rows) > 0) {
return $rows;
}

// If a string was provided but no id(s) were found then exception out.
throw new Exception(sprintf("Invalid value detected for filter '%s': %s", $usageFilterType, $usageFilterValue));
}

return array($usageFilterValue);
}
}
Loading

0 comments on commit 6df963f

Please sign in to comment.