Skip to content

Commit

Permalink
Fix circular dependency issue with configs using environment variables
Browse files Browse the repository at this point in the history
The issue was introduced with nette/di update. The generated code changed
in a way that the circular dependency started appearing. The easiest solution
was to get rid of "setup" method of EnvironmentConfig service and extract
the configs to specific classes.

remp/remp#1299
  • Loading branch information
rootpd committed Oct 3, 2023
1 parent af6ad25 commit 4487a8c
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 70 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,13 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p

## [Unreleased]

### [Mailer]

- **BREAKING**: Removed `EnvironmentConfig::setParam()` and `EnvironmentConfig::getParam()` methods. remp/remp#1299
- Use of these could lead to circular dependency issues if values were read by environment config itself.
- We recommend the extraction of these values to their separate config classes.
- Fixed circular dependency issue with configs using environment variables. remp/remp#1299

## Archive

- [v3.3](./changelogs/CHANGELOG-v3.3.md)
Expand Down
14 changes: 7 additions & 7 deletions Mailer/composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions Mailer/extensions/mailer-module/src/Models/Config/EditorConfig.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php
declare(strict_types=1);

namespace Remp\MailerModule\Models\Config;

class EditorConfig
{
public const EDITOR_CODEMIRROR = 'codemirror';
public const EDITOR_WYSIWYG = 'wysiwyg';

private string $templateEditor = self::EDITOR_CODEMIRROR;

public function setTemplateEditor(string $editor): void
{
$this->templateEditor = match ($editor) {
self::EDITOR_CODEMIRROR => self::EDITOR_CODEMIRROR,
self::EDITOR_WYSIWYG => self::EDITOR_WYSIWYG,
default => throw new \Exception('Unsupported editor configured: ' . $editor),
};
}

public function getTemplateEditor(): string
{
return $this->templateEditor;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php

namespace Remp\MailerModule\Models\Config;

class LinkedServices
{
private array $linkedServices = [];

public function linkService(string $code, ?string $url, ?string $icon): void
{
if (empty($url)) {
return;
}
$this->linkedServices[$code] = [
'url' => $url,
'icon' => $icon,
];
}

public function getServices(): array
{
return $this->linkedServices;
}
}
19 changes: 19 additions & 0 deletions Mailer/extensions/mailer-module/src/Models/Config/SearchConfig.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php
declare(strict_types=1);

namespace Remp\MailerModule\Models\Config;

class SearchConfig
{
private int $maxResultCount = 5;

public function setMaxResultCount(int $maxResultCount): void
{
$this->maxResultCount = $maxResultCount;
}

public function getMaxResultCount(): int
{
return $this->maxResultCount;
}
}
30 changes: 0 additions & 30 deletions Mailer/extensions/mailer-module/src/Models/EnvironmentConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,26 +5,6 @@

class EnvironmentConfig
{
private $linkedServices = [];

private $params = [];

public function linkService(string $code, ?string $url, ?string $icon): void
{
if (empty($url)) {
return;
}
$this->linkedServices[$code] = [
'url' => $url,
'icon' => $icon,
];
}

public function getLinkedServices(): array
{
return $this->linkedServices;
}

public function get(string $key): ?string
{
if (!isset($_ENV[$key])) {
Expand Down Expand Up @@ -67,14 +47,4 @@ public function getBool(string $key): ?bool
}
return filter_var($value, FILTER_VALIDATE_BOOLEAN);
}

public function setParam(string $key, ?string $value): void
{
$this->params[$key] = $value;
}

public function getParam(string $key, $default = null): ?string
{
return $this->params[$key] ?? $default;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Remp\MailerModule\Components\ApplicationStatus\IApplicationStatusFactory;
use Remp\MailerModule\Forms\IFormFactory;
use Remp\MailerModule\Models\Auth\PermissionManager;
use Remp\MailerModule\Models\Config\LinkedServices;
use Remp\MailerModule\Models\Config\LocalizationConfig;
use Remp\MailerModule\Models\EnvironmentConfig;

Expand All @@ -24,6 +25,9 @@ abstract class BasePresenter extends Presenter

/** @var LocalizationConfig @inject */
public $localizationConfig;

/** @var LinkedServices @inject */
public $linkedServices;

public function startup(): void
{
Expand All @@ -34,8 +38,8 @@ public function startup(): void
}

$this->template->currentUser = $this->getUser();
$this->template->linkedServices = $this->environmentConfig->getLinkedServices();
$this->template->locale = $this->environmentConfig->getParam('locale');
$this->template->linkedServices = $this->linkedServices->getServices();
$this->template->locale = $this->localizationConfig->getDefaultLocale();
$this->template->langs = $this->localizationConfig->getSecondaryLocales();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@
use Nette;
use Remp\MailerModule\Components\MissingConfiguration\IMissingConfigurationFactory;
use Remp\MailerModule\Components\MissingConfiguration\MissingConfiguration;
use Remp\MailerModule\Models\EnvironmentConfig;
use Remp\MailerModule\Models\Config\LinkedServices;
use Remp\MailerModule\Models\Config\LocalizationConfig;

class Error4xxPresenter extends BasePresenter
{
/** @var EnvironmentConfig @inject */
public $environmentConfig;
/** @var LocalizationConfig @inject */
public $localizationConfig;

/** @var LinkedServices @inject */
public $linkedServices;

public function startup(): void
{
Expand All @@ -25,8 +29,8 @@ public function startup(): void
public function renderDefault(Nette\Application\BadRequestException $exception): void
{
$this->template->currentUser = $this->getUser();
$this->template->linkedServices = $this->environmentConfig->getLinkedServices();
$this->template->locale = $this->environmentConfig->getParam('locale');
$this->template->linkedServices = $this->linkedServices->getServices();
$this->template->locale = $this->localizationConfig->getDefaultLocale();

// load template 403.latte or 404.latte or ... 4xx.latte
$file = __DIR__ . "/templates/Error/{$exception->getCode()}.latte";
Expand Down
26 changes: 7 additions & 19 deletions Mailer/extensions/mailer-module/src/Presenters/SearchPresenter.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,27 @@

namespace Remp\MailerModule\Presenters;

use Remp\MailerModule\Repositories\BatchTemplatesRepository;
use Remp\MailerModule\Models\Config\SearchConfig;
use Remp\MailerModule\Repositories\JobsRepository;
use Remp\MailerModule\Repositories\LayoutsRepository;
use Remp\MailerModule\Repositories\ListsRepository;
use Remp\MailerModule\Repositories\TemplatesRepository;

final class SearchPresenter extends BasePresenter
{
private $templatesRepository;

private $layoutsRepository;

private $listsRepository;

private $jobsRepository;

public function __construct(
TemplatesRepository $templatesRepository,
LayoutsRepository $layoutsRepository,
ListsRepository $listsRepository,
JobsRepository $jobsRepository
private TemplatesRepository $templatesRepository,
private LayoutsRepository $layoutsRepository,
private ListsRepository $listsRepository,
private JobsRepository $jobsRepository,
private SearchConfig $searchConfig,
) {
parent::__construct();

$this->templatesRepository = $templatesRepository;
$this->layoutsRepository = $layoutsRepository;
$this->listsRepository = $listsRepository;
$this->jobsRepository = $jobsRepository;
}

public function actionDefault($term): void
{
$limit = (int) $this->environmentConfig->getParam('max_result_count', '5');
$limit = $this->searchConfig->getMaxResultCount();
$layouts = array_values($this->layoutsRepository->search($term, $limit));
$lists = array_values($this->listsRepository->search($term, $limit));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
use Remp\MailerModule\Components\MailLinkStats\MailLinkStats;
use Remp\MailerModule\Forms\IFormFactory;
use Remp\MailerModule\Models\Config\Config;
use Remp\MailerModule\Models\Config\EditorConfig;
use Remp\MailerModule\Models\ContentGenerator\GeneratorInputFactory;
use Remp\MailerModule\Models\ContentGenerator\Replace\RtmClickReplace;
use Remp\MailerModule\Repositories\ActiveRow;
Expand Down Expand Up @@ -46,6 +47,7 @@ public function __construct(
private TemplateTranslationsRepository $templateTranslationsRepository,
private MailLinkStats $mailLinkStats,
private Config $config,
private EditorConfig $editorConfig,
) {
parent::__construct();
}
Expand Down Expand Up @@ -263,7 +265,7 @@ public function renderNew(): void
$this->template->layouts = $layouts;
$this->template->snippets = $snippets;
$this->template->lists = $lists;
$this->template->templateEditor = $this->environmentConfig->getParam('template_editor', 'codemirror');
$this->template->templateEditor = $this->editorConfig->getTemplateEditor();
$this->template->editedLocale = null;
}

Expand Down Expand Up @@ -291,7 +293,7 @@ public function renderEdit($id, string $editedLocale = null): void
$this->template->layouts = $layouts;
$this->template->snippets = $snippets;
$this->template->lists = $lists;
$this->template->templateEditor = $this->environmentConfig->getParam('template_editor', 'codemirror');
$this->template->templateEditor = $this->editorConfig->getTemplateEditor();
$this->template->editedLocale = $editedLocale;
}

Expand Down
19 changes: 14 additions & 5 deletions Mailer/extensions/mailer-module/src/config/config.neon
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
services:
router: Remp\MailerModule\Models\RouterFactory::createRouter
environmentConfig:
factory: Remp\MailerModule\Models\EnvironmentConfig
environmentConfig: Remp\MailerModule\Models\EnvironmentConfig

linkedServices:
factory: Remp\MailerModule\Models\Config\LinkedServices
setup:
- linkService(beam, %remp.beam.web_addr%, album)
- linkService(campaign, %remp.campaign.web_addr%, trending-up)
- linkService(mailer, /, email)
- setParam(locale, %locale%)
- setParam(max_result_count, %max_result_count%)
- setParam(template_editor, %template_editor%)

searchConfig:
factory: Remp\MailerModule\Models\Config\SearchConfig
setup:
- setMaxResultCount(beam, %max_result_count%)

editorConfig:
factory: Remp\MailerModule\Models\Config\EditorConfig
setup:
- setTemplateEditor(%template_editor%)

localizationConfig:
factory: Remp\MailerModule\Models\Config\LocalizationConfig(@environmentConfig::get('LOCALE'))
Expand Down

0 comments on commit 4487a8c

Please sign in to comment.