fix: [WIP] fix and improve .env configs population and operations for setup step #3856
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3856 +/- ##
=============================================
- Coverage 27.58% 27.55% -0.03%
- Complexity 10972 10980 +8
=============================================
Files 922 922
Lines 33900 33933 +33
=============================================
Hits 9351 9351
- Misses 24549 24582 +33
|
Version
There are 0 BREAKING CHANGE, 0 feature, 8 fixes |
| @@ -203,6 +209,12 @@ public function __invoke($params) | |||
| 'install_path' => $options['root_path'] . 'tao/install/', | |||
| ]; | |||
|
|
|||
| $markers = new ConfigurationMarkers( | |||
There was a problem hiding this comment.
You probably don't need this code block here.
There was a problem hiding this comment.
also new class instance here https://github.com/oat-sa/tao-core/pull/3856/files#diff-38e78ad9e920c0ee35b839de9d3fe884488dddda66302bf965b85a01402f40ddR243 is not necessary now
| @@ -38,12 +38,12 @@ | |||
|
|
|||
| require_once($root . 'vendor/autoload.php'); | |||
|
|
|||
| new DotEnvReader(); | |||
There was a problem hiding this comment.
Do we need this change? Its intentional?
| @@ -124,7 +124,16 @@ public function writeConstants(array $constants) | |||
| } elseif (is_numeric($val)) { | |||
| $content = preg_replace('/(\'' . $name . '\')(.*?)$/ms', '$1, ' . $val . ');', $content); | |||
| } | |||
| elseif (method_exists($val, '__toPhpSprintfCode')) { | |||
There was a problem hiding this comment.
Did you check how the __toPhpCode() method is being used? Do we need to have you method called manually?
| if(strlen(preg_replace(self::MARKER_PATTERN, '', $item)) > 0) { | ||
| $item = $this->serializableFactory->create($indexes, $item, true); | ||
| } else { | ||
| $item = $this->serializableFactory->create($indexes, $item); | ||
| } |
There was a problem hiding this comment.
if(strlen(preg_replace(self::MARKER_PATTERN, '', $item)) > 0) {
$item = $this->serializableFactory->create($indexes, $item, true);
} else {
$item = $this->serializableFactory->create($indexes, $item);
}to
$flag = strlen(preg_replace(self::MARKER_PATTERN, '', $item)) > 0;
$item = $this->serializableFactory->create($indexes, $item, $flag);| */ | ||
| public function __toPhpCode(): string | ||
| { | ||
| return vsprintf( |
There was a problem hiding this comment.
maybe we can call __toPhpSprintfCode() here ? No worries about calling it in install/utils/class.ConfigWriter.php wdyt?
| /** | ||
| * Replace configuration variable with environment variable | ||
| */ | ||
| class SerializableSecretDtoExtended |
There was a problem hiding this comment.
To remove code duplication we could create new abstract class with common code from SerializableSecretDtoExtended and SerializableSecretDto and do SerializableSecretDtoExtended extends SerializableSecretDtoAbstract
| /** | ||
| * Replace configuration variable with environment variable | ||
| */ | ||
| class SerializableSecretDtoExtended |
There was a problem hiding this comment.
Maybe name SerializableSprintfSecretDto will be better? More explanatory for sure :)
| */ | ||
| public function __toString(): string | ||
| { | ||
| return vsprintf( |
There was a problem hiding this comment.
also just call __toPhpSprintfCode() here.
| @@ -142,6 +142,7 @@ public function __invoke(ContainerConfigurator $configurator): void | |||
|
|
|||
| $services | |||
| ->set(ResourceTransferProxy::class, ResourceTransferProxy::class) | |||
| ->share(false) | |||
There was a problem hiding this comment.
What it does? Its related to your code? Looks like something which need to be double checked
fix: [WIP] fix and improve .env configs population and operations for setup step