Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Configuration class to support merging objects in local config files #782

Merged
merged 10 commits into from
Jan 25, 2019
71 changes: 58 additions & 13 deletions classes/Configuration/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ protected function preTransformTasks()
{
$this->addKeyTransformer(new CommentTransformer($this->logger));
$this->addKeyTransformer(new JsonReferenceTransformer($this->logger));
$this->addKeyTransformer(new StripMergePrefixTransformer($this->logger));
return $this;
} //preTransformTasks()

Expand Down Expand Up @@ -485,19 +486,12 @@ protected function processLocalConfig($localConfigFile)
protected function merge(Configuration $localConfigObj, $overwrite = false)
{

// If overwriting or the key doesn't exist, set it. Otherwise if the value is an
// array append it. If not overwriting and the value is not an array silently skip
// it.

foreach ( $localConfigObj->getTransformedConfig() as $k => $v ) {
if ( $overwrite || ! isset($this->transformedConfig->$k) ) {
$this->transformedConfig->$k = $v;
} elseif ( is_array($this->transformedConfig->$k) ) {
array_push($this->transformedConfig->$k, $v);
} else {
$this->logger->debug("Skip duplicate key in local config (overwrite == false)");
}
}
$this->transformedConfig = $this->mergeLocal(
$this->transformedConfig,
$localConfigObj->getTransformedConfig(),
$localConfigObj->getFilename(),
$overwrite
);

foreach ( $localConfigObj->getSectionNames() as $sectionName ) {
$localConfigData = $localConfigObj->getSectionData($sectionName);
Expand All @@ -515,6 +509,47 @@ protected function merge(Configuration $localConfigObj, $overwrite = false)

} // merge()

/**
* Merge $incoming object into the $existing object recursively.
*
* @param \stdClass $existing the object to be merged into.
* @param \stdClass $incoming the object to be merged from.
* @param string $incomingFileName the file that $incoming originates from.
* @param boolean $overwrite whether or not to force overwriting of $existing w/ $incoming.
* @return \stdClass the updated $existing object.
*/
protected function mergeLocal(\stdClass &$existing, \stdClass $incoming, $incomingFileName, $overwrite)
{
foreach($incoming as $property => $incomingValue) {

if ( $overwrite || ! isset($existing->$property) ) {
$existing->$property = $incoming->$property;
} else {
$existingValue = $existing->$property;

if (is_object($existingValue) && is_object($incomingValue)) {
$existing->$property = $this->mergeLocal($existingValue, $incomingValue, $incomingFileName, $overwrite);
} elseif (is_array($existingValue) && is_array($incomingValue)) {
$existing->$property = array_merge($existingValue, $incomingValue);
} elseif (is_scalar($existingValue) && is_scalar($incomingValue)) {
$existing->$property = $incomingValue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need the check that both of these values are scalars? Otherwise if the existing value is an object and the incoming value is a scalar we will hit this clause. We should provide a warning here as well if we have a type mismatch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, if we fix up this method can't we just call it from the merge() method and get rid of the if-then-else there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still need to be able to handle types other than Object at the top level but it's entirely possible I'm missing something.

} else {
$this->logger->warning(
sprintf(
"Type mismatch. Unable to merge local value for key '%s' (type: %s) with global value (type: %s) from local file %s",
$property,
gettype($incomingValue),
gettype($existingValue),
$incomingFileName
)
);
}
}
}

return $existing;
}

/**
* Perform any tasks that need to occur after merging the local configuration objects into the
* global configuration object. By default no actions are performed, allowing child classes to
Expand Down Expand Up @@ -983,6 +1018,16 @@ public function __isset($property)
return ( array_key_exists($property, $this->sectionData) && null !== $this->sectionData[$property] );
} // __isset()

/**
* Return this Configuration's $filename property.
*
* @return string
*/
public function getFilename()
{
return $this->filename;
}

/** -----------------------------------------------------------------------------------------
* Return the JSON representation of the parsed and translated Configuration.
*
Expand Down
48 changes: 48 additions & 0 deletions classes/Configuration/StripMergePrefixTransformer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

namespace Configuration;

use CCR\Loggable;
use Log;
use stdClass;

/**
* Class StripMergePrefixTransformer
*
* This class allows config files designed to work with `Config` to work with the new
* `Configuration` class. It accomplishes this by stripping the merge prefix "+" from keys. This
* works because the default merge behavior of `Configuration` is the same as `Config` w/ '+'
* prefixed keys.
*
* @package Configuration
*/
class StripMergePrefixTransformer extends Loggable implements iConfigFileKeyTransformer
{
const MERGE_PREFIX = '+';

/**
* @see iConfigFileKeyTransformer::__construct()
*/
public function __construct(Log $logger = null)
{
parent::__construct($logger);
}

/**
* @see iConfigFileKeyTransformer::keyMatches()
*/
public function keyMatches($key)
{
return substr($key, 0, 1) === self::MERGE_PREFIX;
}

/**
* @see iConfigFileKeyTransformer::transform()
*/
public function transform(&$key, &$value, stdClass $obj, Configuration $config)
{
$key = substr($key, 1, strlen($key) - 1);

return true;
}
}
22 changes: 17 additions & 5 deletions open_xdmod/modules/xdmod/tests/bootstrap.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,23 @@
// Autoloader for test classes.
spl_autoload_register(
function ($className) use ($dir) {
$classPath
= $dir
. '/lib/'
. str_replace('\\', '/', $className)
. '.php';
if (strpos($className, 'TestHarness') !== false) {
$classPath = implode(
DIRECTORY_SEPARATOR,
array(
$dir,
'../integration_tests',
'lib',
str_replace('\\', '/', $className) . '.php'
)
);
} else {
$classPath
= $dir
. '/lib/'
. str_replace('\\', '/', $className)
. '.php';
}

if (is_readable($classPath)) {
return require_once $classPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@

use ETL\Configuration\EtlConfiguration;
use ETL\EtlOverseerOptions;
use CCR\Json;
use Configuration\Configuration;
use TestHarness\TestFiles;
use Xdmod\Config;

class EtlConfigurationTest extends \PHPUnit_Framework_TestCase
{
Expand All @@ -19,6 +23,15 @@ class EtlConfigurationTest extends \PHPUnit_Framework_TestCase
const TMPDIR = '/tmp/xdmod-etl-configuration-test';
private static $defaultModuleName = null;

private $testFiles;

public function __construct($name = null, array $data = array(), $dataName = '')
{
parent::__construct($name, $data, $dataName);
$this->testFiles = new TestFiles(__DIR__ . '/../../../');
}


public static function setUpBeforeClass()
{
// Query the configuration file for the default module name
Expand Down Expand Up @@ -120,4 +133,63 @@ public function testConfigurationVariables()

$this->assertEquals($expected, $generated, $file);
}

/**
* Tests if `Configuration\Configuration` can produce the same basic output as `Config`. This is
* part of the Configuration Code Consolidation.
*
* NOTE ========================================================================================
* This Test should be moved to ConfigurationTest.php and the `@depend` removed once
* https://app.asana.com/0/807629084565719/1101232922862525/f has been addressed.
* =============================================================================================
* @depends testConfigurationVariables
*
* @dataProvider provideTestConfigEquivalence
*
* @param array $options options that control how the test is to be conducted. Required
* key / values are:
* - section : The section that will be requested from `Config` to generate the expected
* output
* - expected: The filename to use when generating or retrieving the expected output.
* @throws \Exception
*/
public function testConfigEquivalence(array $options)
{
$section = $options['section'];
$expectedFileName = $options['expected'];

$expectedFilePath = $this->testFiles->getFile('configuration', $expectedFileName);

if (!is_file($expectedFilePath)) {
$config = Config::factory();
$actual = $config[$section];
@file_put_contents($expectedFilePath, json_encode($actual) . "\n");
echo "\nGenerated Expected Output for: $expectedFilePath\n";
} else {
$expected = @file_get_contents($expectedFilePath);

$configFile = implode(DIRECTORY_SEPARATOR, array(CONFIG_DIR, "$section.json"));
$configDir = implode(DIRECTORY_SEPARATOR, array(CONFIG_DIR, "$section.d"));

$config = new Configuration(
$configFile,
CONFIG_DIR,
null,
array(
'local_config_dir' => $configDir
)
);
$config->initialize();
$actual = $config->toJson() . "\n";

$this->assertEquals($expected, $actual);
}
}

public function provideTestConfigEquivalence()
{
return JSON::loadFile(
$this->testFiles->getFile('configuration', 'xdmod_config', 'input')
);
}
} // class EtlConfigurationTest
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
[
{
"section": "roles",
"expected": "xdmod_config_roles.json"
}
],
[
{
"section": "datawarehouse",
"expected": "xdmod_config_datawarehouse.json"
}
]
]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"realms":{"Jobs":{"schema":"modw_aggregates","table":"jobfact","datasource":"HPcDB","group_bys":[{"name":"none","class":"GroupByNone"},{"name":"nodecount","class":"GroupByNodeCount"},{"name":"person","class":"GroupByPerson"},{"name":"pi","class":"GroupByPI"},{"name":"resource","class":"GroupByResource"},{"name":"resource_type","class":"GroupByResourceType"},{"name":"nsfdirectorate","class":"GroupByNSFDirectorate"},{"name":"parentscience","class":"GroupByParentScience"},{"name":"fieldofscience","class":"GroupByScience"},{"name":"jobsize","class":"GroupByJobSize"},{"name":"jobwalltime","class":"GroupByJobTime"},{"name":"jobwaittime","class":"GroupByJobWaitTime"},{"name":"queue","class":"GroupByQueue"},{"name":"username","class":"GroupByUsername"},{"name":"day","class":"GroupByDay"},{"name":"month","class":"GroupByMonth"},{"name":"quarter","class":"GroupByQuarter"},{"name":"year","class":"GroupByYear"},{"name":"provider","class":"GroupByProvider","visible":false}],"statistics":[{"name":"job_count","class":"JobCountStatistic"},{"name":"job_count","class":"JobCountStatistic"},{"name":"running_job_count","class":"RunningJobCountStatistic","control":true},{"name":"started_job_count","class":"StartedJobCountStatistic","control":true},{"name":"submitted_job_count","class":"SubmittedJobCountStatistic"},{"name":"active_person_count","class":"ActiveUserCountStatistic"},{"name":"active_pi_count","class":"ActivePICountStatistic"},{"name":"total_cpu_hours","class":"TotalCPUHoursStatistic"},{"name":"total_waitduration_hours","class":"TotalWaitHoursStatistic"},{"name":"total_node_hours","class":"TotalNodeHoursStatistic"},{"name":"total_wallduration_hours","class":"TotalWallHoursStatistic"},{"name":"avg_cpu_hours","class":"AverageCPUHoursStatistic"},{"name":"sem_avg_cpu_hours","class":"SEMAverageCPUHoursStatistic","visible":false},{"name":"avg_node_hours","class":"AverageNodeHoursStatistic"},{"name":"sem_avg_node_hours","class":"SEMAverageNodeHoursStatistic","visible":false},{"name":"avg_waitduration_hours","class":"AverageWaitHoursStatistic"},{"name":"sem_avg_waitduration_hours","class":"SEMAverageWaitHoursStatistic","visible":false},{"name":"avg_wallduration_hours","class":"AverageWallHoursStatistic"},{"name":"sem_avg_wallduration_hours","class":"SEMAverageWallHoursStatistic","visible":false},{"name":"avg_processors","class":"AverageProcessorCountStatistic"},{"name":"sem_avg_processors","class":"SEMAverageProcessorCountStatistic","visible":false},{"name":"min_processors","class":"MinProcessorCountStatistic"},{"name":"max_processors","class":"MaxProcessorCountStatistic"},{"name":"utilization","class":"UtilizationStatistic"},{"name":"expansion_factor","class":"ExpansionFactorStatistic"},{"name":"normalized_avg_processors","class":"NormalizedAverageProcessorCountStatistic"},{"name":"avg_job_size_weighted_by_cpu_hours","class":"JobSizeWeightedByCPUHours"},{"name":"active_resource_count","class":"ActiveResourceCountStatistic"}]},"Storage":{"schema":"modw","table":"storagefact","datasource":"Storage usage logs","group_bys":[{"name":"none","class":"GroupByNone"},{"name":"day","class":"GroupByDay"},{"name":"month","class":"GroupByMonth"},{"name":"quarter","class":"GroupByQuarter"},{"name":"year","class":"GroupByYear"},{"name":"resource","class":"GroupByResource"},{"name":"resource_type","class":"GroupByResourceType"},{"name":"mountpoint","class":"GroupByMountpoint"},{"name":"person","class":"GroupByPerson"},{"name":"pi","class":"GroupByPI"},{"name":"username","class":"GroupByUsername"},{"name":"nsfdirectorate","class":"GroupByNSFDirectorate"},{"name":"parentscience","class":"GroupByParentScience"},{"name":"fieldofscience","class":"GroupByScience"}],"statistics":[{"name":"user_count","class":"UserCountStatistic"},{"name":"avg_file_count","class":"FileCountStatistic"},{"name":"sem_file_count","class":"SemFileCountStatistic","visible":false},{"name":"avg_logical_usage","class":"LogicalUsageStatistic"},{"name":"sem_logical_usage","class":"SemLogicalUsageStatistic","visible":false},{"name":"avg_physical_usage","class":"PhysicalUsageStatistic"},{"name":"sem_physical_usage","class":"SemPhysicalUsageStatistic","visible":false},{"name":"avg_hard_threshold","class":"HardThresholdStatistic"},{"name":"avg_soft_threshold","class":"SoftThresholdStatistic"},{"name":"avg_logical_utilization","class":"LogicalUtilizationStatistic"}]},"Cloud":{"schema":"modw_cloud","table":"cloudfact","datasource":"Cloud","group_bys":[{"name":"none","class":"GroupByNone"},{"name":"project","class":"GroupByProject"},{"name":"configuration","class":"GroupByConfiguration"},{"name":"resource","class":"GroupByResource"},{"name":"day","class":"GroupByDay"},{"name":"month","class":"GroupByMonth"},{"name":"quarter","class":"GroupByQuarter"},{"name":"year","class":"GroupByYear"},{"name":"vm_size_memory","class":"GroupByVMMemory"},{"name":"vm_size","class":"GroupByVMCores"},{"name":"submission_venue","class":"GroupBySubmissionVenue"}],"statistics":[{"name":"cloud_num_sessions_ended","class":"SessionsCountStatistic"},{"name":"cloud_num_sessions_running","class":"RunningSessionsCountStatistic","control":true},{"name":"cloud_num_sessions_started","class":"StartedSessionsCountStatistic","control":true},{"name":"cloud_avg_wallduration_hours","class":"AverageWallHoursStatistic"},{"name":"cloud_core_time","class":"CoreHoursStatistic"},{"name":"cloud_wall_time","class":"WallHoursStatistic"},{"name":"cloud_avg_cores_reserved","class":"WeightedCoresReservedStatistic"},{"name":"cloud_avg_memory_reserved","class":"AverageMemoryReservedStatistic"},{"name":"cloud_avg_rv_storage_reserved","class":"AverageRootVolumeStorageReservedStatistic"}]}}}
Loading