From 740e226eac6215a63a6298e4acb49c6239b090e2 Mon Sep 17 00:00:00 2001 From: Brett McBride Date: Tue, 23 Apr 2024 12:36:15 +1000 Subject: [PATCH] remove value type and variable type checking per review feedback, this can be checked with tests rather than at runtime --- src/Config/Configuration/Configuration.php | 55 +------ src/Config/Configuration/ValueTypes.php | 136 ----------------- src/Config/Configuration/VariableTypes.php | 62 -------- .../Configuration/ConfigurationTest.php | 137 ++++++------------ 4 files changed, 50 insertions(+), 340 deletions(-) delete mode 100644 src/Config/Configuration/ValueTypes.php delete mode 100644 src/Config/Configuration/VariableTypes.php diff --git a/src/Config/Configuration/Configuration.php b/src/Config/Configuration/Configuration.php index 459968c2d..9328532d6 100644 --- a/src/Config/Configuration/Configuration.php +++ b/src/Config/Configuration/Configuration.php @@ -28,10 +28,7 @@ public static function has(string $name): bool public static function getInt(string $key, int $default = null): int { return (int) self::validateVariableValue( - CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::INTEGER), - $default - ), + CompositeResolver::instance()->resolve($key, $default), FILTER_VALIDATE_INT ); } @@ -39,10 +36,7 @@ public static function getInt(string $key, int $default = null): int public static function getString(string $key, string $default = null): string { return (string) self::validateVariableValue( - CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::STRING), - $default - ) + CompositeResolver::instance()->resolve($key, $default), ); } @@ -50,7 +44,7 @@ public static function getBoolean(string $key, bool $default = null): bool { $resolved = self::validateVariableValue( CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::BOOL), + $key, null === $default ? $default : ($default ? 'true' : 'false') ) ); @@ -71,40 +65,28 @@ public static function getMixed(string $key, $default = null): mixed public static function getMap(string $key, array $default = null): array { return MapParser::parse( - CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::MAP), - $default - ) + CompositeResolver::instance()->resolve($key, $default), ); } public static function getList(string $key, array $default = null): array { return ListParser::parse( - CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::LIST), - $default - ) + CompositeResolver::instance()->resolve($key, $default), ); } public static function getEnum(string $key, string $default = null): string { return (string) self::validateVariableValue( - CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::ENUM), - $default - ) + CompositeResolver::instance()->resolve($key, $default), ); } public static function getFloat(string $key, float $default = null): float { return (float) self::validateVariableValue( - CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::FLOAT), - $default - ), + CompositeResolver::instance()->resolve($key, $default), FILTER_VALIDATE_FLOAT ); } @@ -113,10 +95,7 @@ public static function getRatio(string $key, float $default = null): float { return RatioParser::parse( self::validateVariableValue( - CompositeResolver::instance()->resolve( - self::validateVariableType($key, VariableTypes::RATIO), - $default - ) + CompositeResolver::instance()->resolve($key, $default), ) ); } @@ -131,30 +110,12 @@ public static function getDefault(string $variableName) return ClassConstantAccessor::getValue(Defaults::class, $variableName); } - public static function getType(string $variableName): ?string - { - return ClassConstantAccessor::getValue(ValueTypes::class, $variableName); - } - public static function isEmpty($value): bool { // don't use 'empty()', since '0' is not considered to be empty return $value === null || $value === ''; } - private static function validateVariableType(string $variableName, string $type): string - { - $variableType = self::getType($variableName); - - if ($variableType !== null && $variableType !== $type && $variableType !== VariableTypes::MIXED) { - throw new UnexpectedValueException( - sprintf('Variable "%s" is not supposed to be of type "%s" but type "%s"', $variableName, $type, $variableType) - ); - } - - return $variableName; - } - private static function validateVariableValue(mixed $value, ?int $filterType = null): mixed { if ($filterType !== null && filter_var($value, $filterType) === false) { diff --git a/src/Config/Configuration/ValueTypes.php b/src/Config/Configuration/ValueTypes.php deleted file mode 100644 index 339c48d7e..000000000 --- a/src/Config/Configuration/ValueTypes.php +++ /dev/null @@ -1,136 +0,0 @@ - ['getString'], - VariableTypes::BOOL => ['getBoolean'], - VariableTypes::INTEGER => ['getInt'], - VariableTypes::FLOAT => ['getFloat'], - VariableTypes::RATIO => ['getRatio'], - VariableTypes::ENUM => ['getEnum'], - VariableTypes::LIST => ['getList'], - VariableTypes::MAP => ['getMap'], - VariableTypes::MIXED => ['getMixed'], + 'string' => ['getString'], + 'bool' => ['getBoolean'], + 'integer' => ['getInt'], + 'float' => ['getFloat'], + 'ratio' => ['getRatio'], + 'enum' => ['getEnum'], + 'list' => ['getList'], + 'map' => ['getMap'], + 'mixed' => ['getMixed'], ]; private const TYPES = [ - VariableTypes::STRING => [Variables::OTEL_SERVICE_NAME], - VariableTypes::BOOL => [Variables::OTEL_EXPORTER_OTLP_INSECURE], - VariableTypes::INTEGER => [Variables::OTEL_BSP_MAX_QUEUE_SIZE], - VariableTypes::ENUM => [Variables::OTEL_LOG_LEVEL], - VariableTypes::LIST => [Variables::OTEL_PROPAGATORS], - VariableTypes::MAP => [Variables::OTEL_RESOURCE_ATTRIBUTES], - VariableTypes::MIXED => [Variables::OTEL_TRACES_SAMPLER_ARG], + 'string' => [Variables::OTEL_SERVICE_NAME], + 'bool' => [Variables::OTEL_EXPORTER_OTLP_INSECURE], + 'integer' => [Variables::OTEL_BSP_MAX_QUEUE_SIZE], + 'enum' => [Variables::OTEL_LOG_LEVEL], + 'list' => [Variables::OTEL_PROPAGATORS], + 'map' => [Variables::OTEL_RESOURCE_ATTRIBUTES], + 'mixed' => [Variables::OTEL_TRACES_SAMPLER_ARG], ]; private const USER_VALUES = [ - VariableTypes::STRING => ['foo', 'foo'], - VariableTypes::BOOL => ['true', true], - VariableTypes::INTEGER => ['42', 42], - VariableTypes::ENUM => ['val1', 'val1'], - VariableTypes::LIST => [['val1', 'val2'], ['val1','val2']], - VariableTypes::MAP => [['var1' => 'val1', 'var2' => 'val2'], ['var1'=>'val1','var2'=>'val2']], - VariableTypes::MIXED => ['foo', 'foo'], + 'string' => ['foo', 'foo'], + 'bool' => ['true', true], + 'integer' => ['42', 42], + 'enum' => ['val1', 'val1'], + 'list' => [['val1', 'val2'], ['val1','val2']], + 'map' => [['var1' => 'val1', 'var2' => 'val2'], ['var1'=>'val1','var2'=>'val2']], + 'mixed' => ['foo', 'foo'], ]; private const USER_ENV_VALUES = [ - VariableTypes::STRING => ['foo', 'foo'], - VariableTypes::BOOL => ['true', true], - VariableTypes::INTEGER => ['42', 42], - VariableTypes::ENUM => ['val1', 'val1'], - VariableTypes::LIST => ['val1,val2', ['val1','val2']], - VariableTypes::MAP => ['var1=val1,var2=val2', ['var1'=>'val1','var2'=>'val2']], - VariableTypes::MIXED => ['foo', 'foo'], + 'string' => ['foo', 'foo'], + 'bool' => ['true', true], + 'integer' => ['42', 42], + 'enum' => ['val1', 'val1'], + 'list' => ['val1,val2', ['val1','val2']], + 'map' => ['var1=val1,var2=val2', ['var1'=>'val1','var2'=>'val2']], + 'mixed' => ['foo', 'foo'], ]; private const LIBRARY_DEFAULTS = [ - VariableTypes::STRING => [Variables::OTEL_EXPORTER_OTLP_ENDPOINT, 'http://localhost:4318'], - VariableTypes::BOOL => [Variables::OTEL_EXPORTER_OTLP_INSECURE, false], - VariableTypes::INTEGER => [Variables::OTEL_BSP_MAX_QUEUE_SIZE, 2048], - VariableTypes::ENUM => [Variables::OTEL_LOG_LEVEL, 'info'], - VariableTypes::LIST => [Variables::OTEL_PROPAGATORS, ['tracecontext', 'baggage']], + 'string' => [Variables::OTEL_EXPORTER_OTLP_ENDPOINT, 'http://localhost:4318'], + 'bool' => [Variables::OTEL_EXPORTER_OTLP_INSECURE, false], + 'integer' => [Variables::OTEL_BSP_MAX_QUEUE_SIZE, 2048], + 'enum' => [Variables::OTEL_LOG_LEVEL, 'info'], + 'list' => [Variables::OTEL_PROPAGATORS, ['tracecontext', 'baggage']], ]; private const DEFAULT_VALUES = [ @@ -85,10 +84,10 @@ class ConfigurationTest extends TestCase ]; private const NO_DEFAULTS = [ - VariableTypes::STRING => [Variables::OTEL_SERVICE_NAME], - VariableTypes::ENUM => [Variables::OTEL_EXPORTER_OTLP_COMPRESSION], - VariableTypes::MAP => [Variables::OTEL_EXPORTER_OTLP_HEADERS], - VariableTypes::MIXED => [Variables::OTEL_TRACES_SAMPLER_ARG], + 'string' => [Variables::OTEL_SERVICE_NAME], + 'enum' => [Variables::OTEL_EXPORTER_OTLP_COMPRESSION], + 'map' => [Variables::OTEL_EXPORTER_OTLP_HEADERS], + 'mixed' => [Variables::OTEL_TRACES_SAMPLER_ARG], ]; private const KNOWN_VALUES = [ @@ -262,16 +261,6 @@ public function test_no_value_throws_exception(string $methodName): void call_user_func([Configuration::class, $methodName], 'FOO_BAR_' . $methodName); } - /** - * @dataProvider invalidTypeProvider - */ - public function test_invalid_type_throws_exception(string $methodName, string $variable): void - { - $this->expectException(UnexpectedValueException::class); - - call_user_func([Configuration::class, $methodName], $variable); - } - /** * @dataProvider noDefaultProvider */ @@ -334,24 +323,6 @@ public static function nonEmptyMethodNameProvider(): Generator } } - public static function invalidTypeProvider(): Generator - { - foreach (self::METHOD_NAMES as $varType => $names) { - if ($varType === VariableTypes::MIXED) { - continue; - } - $methodName = $names[0]; - foreach (self::TYPES as $methodType => $types) { - if ($varType === $methodType || $methodType === VariableTypes::MIXED) { - continue; - } - $variableName = $types[0]; - - yield sprintf('%s - %s', $varType, $methodType) => [$methodName, $variableName]; - } - } - } - public static function noDefaultProvider(): Generator { foreach (self::NO_DEFAULTS as $varType => $values) { @@ -400,30 +371,6 @@ public function test_retrieve_value_library_default(): void ); } - /** - * @dataProvider typeProvider - */ - public function test_get_type(string $varName, string $type): void - { - $this->assertSame( - $type, - Configuration::getType($varName) - ); - } - - public static function typeProvider(): array - { - return [ - 'bool' => ['OTEL_EXPORTER_OTLP_INSECURE', VariableTypes::BOOL], - 'string' => ['OTEL_SERVICE_NAME', VariableTypes::STRING], - 'integer' => ['OTEL_BSP_MAX_QUEUE_SIZE', VariableTypes::INTEGER], - 'enum' => ['OTEL_LOG_LEVEL', VariableTypes::ENUM], - 'list' => ['OTEL_PROPAGATORS', VariableTypes::LIST], - 'map' => ['OTEL_RESOURCE_ATTRIBUTES', VariableTypes::MAP], - 'mixed' => ['OTEL_TRACES_SAMPLER_ARG', VariableTypes::MIXED], - ]; - } - /** * @dataProvider defaultValueProvider */