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.

Sign up for GitHub

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

90/edit preview mode support v3 review #4521

Merged
merged 4 commits into from
Sep 16, 2023
Merged

90/edit preview mode support v3 review #4521

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
70d2179
TASK: Introduce constant `RenderingMode::FRONTEND` for system integra…
mhsdesign Sep 15, 2023
e2c6f5e
TASK: RenderingModeService instances runtime cache
mhsdesign Sep 15, 2023
0463da9
TASK: Remove half-baked solution of session independent preview's
mhsdesign Sep 15, 2023
5639b6b
TASK: Optimize runtime cache for RenderingModes for readability
mficzel Sep 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions Neos.Neos/Classes/Controller/Frontend/NodeController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use Neos\Flow\Security\Context as SecurityContext;
use Neos\Flow\Session\SessionInterface;
use Neos\Flow\Utility\Now;
use Neos\Neos\Domain\Model\RenderingMode;
use Neos\Neos\Domain\Service\NodeSiteResolvingService;
use Neos\Neos\Domain\Service\RenderingModeService;
use Neos\Neos\FrontendRouting\Exception\InvalidShortcutException;
Expand Down Expand Up @@ -114,7 +115,6 @@ class NodeController extends ActionController

/**
* @param string $node Legacy name for backwards compatibility of route components
* @param string $renderingModeName Name of the user interface mode to use
* @throws NodeNotFoundException
* @throws \Neos\Flow\Mvc\Exception\StopActionException
* @throws \Neos\Flow\Mvc\Exception\UnsupportedRequestTypeException
Expand All @@ -125,13 +125,10 @@ class NodeController extends ActionController
* with unsafe requests from widgets or plugins that are rendered on the node
* - For those the CSRF token is validated on the sub-request, so it is safe to be skipped here
*/
public function previewAction(string $node, string $renderingModeName = null): void
public function previewAction(string $node): void
{
if (is_null($renderingModeName)) {
$renderingMode = $this->renderingModeService->findByCurrentUser();
} else {
$renderingMode = $this->renderingModeService->findByName($renderingModeName);
}
// @todo add $renderingModeName as parameter and append it for successive links again as get parameter to node uris
$renderingMode = $this->renderingModeService->findByCurrentUser();

$visibilityConstraints = VisibilityConstraints::frontend();
if ($this->privilegeManager->isPrivilegeTargetGranted('Neos.Neos:Backend.GeneralAccess')) {
Expand Down Expand Up @@ -247,7 +244,7 @@ public function showAction(string $node, bool $showInvisible = false): void
$this->handleShortcutNode($nodeAddress, $contentRepository);
}

$this->view->setOption('renderingModeName', 'frontend');
$this->view->setOption('renderingModeName', RenderingMode::FRONTEND);

$this->view->assignMultiple([
'value' => $nodeInstance,
Expand Down
17 changes: 13 additions & 4 deletions Neos.Neos/Classes/Domain/Model/RenderingMode.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@

namespace Neos\Neos\Domain\Model;

use Neos\Utility\ObjectAccess;
use Neos\Neos\Domain\Exception;

/**
* Describes the mode in which the Neos interface is rendering currently,
* mainly distinguishing between edit and preview modes currently.
*/
class RenderingMode
{
public const FRONTEND = 'frontend';

/**
* @param array<string,mixed> $options
*/
Expand All @@ -36,13 +38,20 @@ public function __construct(
}

/**
* Creates an UserInterfaceMode object by configuration
* Creates a rendering mode from its configuration
*
* @param string $modeName
* @param array<string,mixed> $configuration
*/
public static function createFromConfiguration(string $modeName, array $configuration): RenderingMode
{
if ($modeName === RenderingMode::FRONTEND) {
throw new Exception(
'Cannot instantiate system rendering mode "frontend" from configuration.'
. ' Please use RenderingMode::createFrontend().',
1694802951840
);
}
$mode = new RenderingMode(
$modeName,
$configuration['isEditingMode'] ?? false,
Expand All @@ -55,12 +64,12 @@ public static function createFromConfiguration(string $modeName, array $configur
}

/**
* Creates the live User interface mode
* Creates the system integrated rendering mode 'frontend'
*/
public static function createFrontend(): RenderingMode
{
return new RenderingMode(
'frontend',
RenderingMode::FRONTEND,
false,
false,
'Frontend',
Expand Down
23 changes: 13 additions & 10 deletions Neos.Neos/Classes/Domain/Service/RenderingModeService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ class RenderingModeService
*/
protected $defaultEditPreviewMode;

/**
* @var array<string, RenderingMode>
*/
private array $instances = [];

/**
* Get the current rendering mode.
* Will return a live mode when not in backend.
Expand Down Expand Up @@ -85,16 +90,14 @@ public function findDefault(): RenderingMode
*/
public function findByName(string $modeName): RenderingMode
{
if ($modeName === 'frontend') {
return RenderingMode::createFrontend();
}
if (isset($this->editPreviewModes[$modeName])) {
return RenderingMode::createFromConfiguration($modeName, $this->editPreviewModes[$modeName]);
}
throw new Exception(
'The requested interface render mode "' . $modeName . '" is not configured.'
return $this->instances[$modeName] ??= match (true) {
Copy link
Member

Choose a reason for hiding this comment

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

I kinda dislike that you put the runtime cache in instances into one live with code that populates this.

Needlessly harder to read and unterstand. Offcourse the runtime cache makes sense.

Copy link
Member

@mficzel mficzel Sep 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

suggestion:

	public function findByName(string $modeName): RenderingMode
    {
        if ($instance = $this->instances[$modeName]) {
            return $instance;
        }
        if ($modeName === RenderingMode::FRONTEND) {
            $this->instances[$modeName] = RenderingMode::createFrontend();
        } else {
            $this->instances[$modeName] = RenderingMode::createFromConfiguration($modeName, $this->editPreviewModes[$modeName]);
        }
        return $this->instances[$modeName];        
    }

Copy link
Member

Choose a reason for hiding this comment

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

??= and match($modeName) would be fine for me. What is imho too much is the match true you have to use to thwrow the exception. Will adjust and the merge this into the other pr.

Funny is that even the php documentation does not explain that operator other than mentioning its existence.

Copy link
Member

@mficzel mficzel Sep 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

BTW: This would be fine for me ... but it will not throw on unconfigured modes

return $this->instances[$modeName] ??= match($modeName) {
   RenderingMode::FRONTEND => RenderingMode::createFrontend(),
   default => RenderingMode::createFromConfiguration($modeName, $this->editPreviewModes[$modeName])
};

$modeName === RenderingMode::FRONTEND => RenderingMode::createFrontend(),
isset($this->editPreviewModes[$modeName]) => RenderingMode::createFromConfiguration($modeName, $this->editPreviewModes[$modeName]),
default => throw new Exception(
'The requested rendering mode "' . $modeName . '" is not configured.'
. ' Please make sure it exists as key in the Settings path "Neos.Neos.Interface.editPreviewModes".',
1427715962
);
1427715962
)
};
}
}
3 changes: 2 additions & 1 deletion Neos.Neos/Classes/View/FusionExceptionView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use Neos\Fusion\Core\Runtime as FusionRuntime;
use Neos\Fusion\Core\RuntimeFactory;
use Neos\Fusion\Exception\RuntimeException;
use Neos\Neos\Domain\Model\RenderingMode;
use Neos\Neos\Domain\Repository\SiteRepository;
use Neos\Neos\Domain\Service\FusionService;
use Neos\Neos\Domain\Service\SiteNodeUtility;
Expand Down Expand Up @@ -204,7 +205,7 @@ protected function getFusionRuntime(

$fusionGlobals = FusionGlobals::fromArray([
'request' => $this->controllerContext->getRequest(),
'renderingModeName' => 'frontend'
'renderingModeName' => RenderingMode::FRONTEND
]);
$this->fusionRuntime = $this->runtimeFactory->createFromConfiguration(
$fusionConfiguration,
Expand Down
3 changes: 2 additions & 1 deletion Neos.Neos/Classes/View/FusionView.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Neos\Fusion\Core\Runtime;
use Neos\Fusion\Core\RuntimeFactory;
use Neos\Fusion\Exception\RuntimeException;
use Neos\Neos\Domain\Model\RenderingMode;
use Neos\Neos\Domain\Repository\SiteRepository;
use Neos\Neos\Domain\Service\FusionService;
use Neos\Neos\Domain\Service\SiteNodeUtility;
Expand Down Expand Up @@ -102,7 +103,7 @@ public function render(): string|ResponseInterface
'boolean'
],
'renderingModeName' => [
'frontend',
RenderingMode::FRONTEND,
'Name of the user interface mode to use',
'string'
]
Expand Down
3 changes: 2 additions & 1 deletion Neos.Neos/Configuration/Settings.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,8 @@ Neos:

defaultEditPreviewMode: inPlace
editPreviewModes:
# the "frontend" renderingMode is hardcoded internally and cannot be modified
# the system integrated rendering mode "frontend" cannot be configured
# frontend: {}
inPlace:
isEditingMode: true
isPreviewMode: false
Expand Down