Skip to content

Commit

Permalink
Merge pull request #3330 from robertlemke/task/session-code-cleanup
Browse files Browse the repository at this point in the history
!!! TASK: Modernize and clean up session-related code
  • Loading branch information
robertlemke authored Mar 14, 2024
2 parents 52140c7 + 7cd4be6 commit bddcf98
Show file tree
Hide file tree
Showing 28 changed files with 449 additions and 716 deletions.
80 changes: 25 additions & 55 deletions Neos.Flow/Classes/Session/Aspect/LazyLoadingAspect.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
namespace Neos\Flow\Session\Aspect;

/*
Expand All @@ -19,55 +20,29 @@

/**
* Adds the aspect of lazy loading to objects with scope session.
*
* @Flow\Aspect
* @Flow\Introduce("filter(Neos\Flow\Session\Aspect\SessionObjectMethodsPointcutFilter)", interfaceName = "Neos\Flow\Session\Aspect\LazyLoadingProxyInterface")
* @Flow\Scope("singleton")
*/
#[Flow\Aspect]
#[Flow\Introduce(pointcutExpression: "filter(Neos\Flow\Session\Aspect\SessionObjectMethodsPointcutFilter)", interfaceName: LazyLoadingProxyInterface::class)]
#[Flow\Scope("singleton")]
class LazyLoadingAspect
{
/**
* @Flow\Inject
* @var ObjectManagerInterface
*/
protected $objectManager;

/**
* @Flow\Inject
* @var SessionManagerInterface
*/
protected $sessionManager;
#[Flow\Inject]
protected ?LoggerInterface $logger = null;

/**
* @var LoggerInterface
*/
protected $logger;

/**
* @var array
*/
protected $sessionOriginalInstances = [];
protected array $sessionOriginalInstances = [];

/**
* Injects the (system) logger based on PSR-3.
*
* @param LoggerInterface $logger
* @return void
*/
public function injectLogger(LoggerInterface $logger)
{
$this->logger = $logger;
public function __construct(
readonly protected ObjectManagerInterface $objectManager,
readonly protected SessionManagerInterface $sessionManager,
) {
}

/**
* Registers an object of scope session.
*
* @param string $objectName
* @param object $object
* @return void
* @see \Neos\Flow\ObjectManagement\ObjectManager
*/
public function registerSessionInstance($objectName, $object)
public function registerSessionInstance(string $objectName, object $object): void
{
$this->sessionOriginalInstances[$objectName] = $object;
}
Expand All @@ -77,36 +52,31 @@ public function registerSessionInstance($objectName, $object)
* Those methods will trigger a session initialization if a session does not exist
* yet.
*
* @param JoinPointInterface $joinPoint The current join point
* @return void
* @fixme The pointcut expression below does not consider the options of the session annotation – needs adjustments in the AOP framework
* @Flow\Before("methodAnnotatedWith(Neos\Flow\Annotations\Session)")
* @fixme The pointcut expression below does not consider the options of the session annotation and needs adjustments in the AOP framework
*/
public function initializeSession(JoinPointInterface $joinPoint)
#[Flow\Before("methodAnnotatedWith(Neos\Flow\Annotations\Session)")]
public function initializeSession(JoinPointInterface $joinPoint): void
{
$session = $this->sessionManager->getCurrentSession();

if ($session->isStarted() === true) {
return;
}

/** @var string $objectName */
$objectName = $this->objectManager->getObjectNameByClassName(get_class($joinPoint->getProxy()));
$methodName = $joinPoint->getMethodName();

$this->logger->debug(sprintf('Session initialization triggered by %s->%s.', $objectName, $methodName));
if ($this->logger) {
$objectName = $this->objectManager->getObjectNameByClassName(get_class($joinPoint->getProxy()));
$methodName = $joinPoint->getMethodName();
$this->logger->debug(sprintf('Session initialization triggered by %s->%s.', $objectName, $methodName));
}
$session->start();
}

/**
* Around advice, wrapping every method of a scope session object. It redirects
* all method calls to the session object once there is one.
*
* @param JoinPointInterface $joinPoint The current join point
* @return mixed
* @Flow\Around("filter(Neos\Flow\Session\Aspect\SessionObjectMethodsPointcutFilter)")
*/
public function callMethodOnOriginalSessionObject(JoinPointInterface $joinPoint)
#[Flow\Around("filter(Neos\Flow\Session\Aspect\SessionObjectMethodsPointcutFilter)")]
public function callMethodOnOriginalSessionObject(JoinPointInterface $joinPoint): mixed
{
$objectName = $this->objectManager->getObjectNameByClassName(get_class($joinPoint->getProxy()));
$methodName = $joinPoint->getMethodName();
Expand All @@ -118,9 +88,9 @@ public function callMethodOnOriginalSessionObject(JoinPointInterface $joinPoint)

if ($this->sessionOriginalInstances[$objectName] === $proxy) {
return $joinPoint->getAdviceChain()->proceed($joinPoint);
} else {
$arguments = array_values($joinPoint->getMethodArguments());
return $this->sessionOriginalInstances[$objectName]->$methodName(...$arguments);
}

$arguments = array_values($joinPoint->getMethodArguments());
return $this->sessionOriginalInstances[$objectName]->$methodName(...$arguments);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
namespace Neos\Flow\Session\Aspect;

/*
Expand Down
106 changes: 31 additions & 75 deletions Neos.Flow/Classes/Session/Aspect/LoggingAspect.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php
declare(strict_types=1);
namespace Neos\Flow\Session\Aspect;

/*
Expand All @@ -13,62 +14,38 @@

use Neos\Flow\Annotations as Flow;
use Neos\Flow\Aop\JoinPointInterface;
use Neos\Flow\Session\Exception\SessionNotStartedException;
use Neos\Flow\Session\SessionInterface;
use Psr\Log\LoggerInterface;

/**
* An aspect which centralizes the logging of important session actions.
*
* @Flow\Aspect
* @Flow\Scope("singleton")
*/
#[Flow\Aspect]
#[Flow\Scope("singleton")]
class LoggingAspect
{
/**
* @Flow\Inject(name="Neos.Flow:SecurityLogger")
* @var LoggerInterface
*/
protected $logger;

/**
* Injects the (security) logger based on PSR-3.
*
* @param LoggerInterface $logger
* @return void
*/
public function injectLogger(LoggerInterface $logger)
{
$this->logger = $logger;
}
#[Flow\Inject]
protected ?LoggerInterface $logger = null;

/**
* Logs calls of start()
*
* @Flow\After("within(Neos\Flow\Session\SessionInterface) && method(.*->start())")
* @param JoinPointInterface $joinPoint The current joinpoint
* @return void
* @throws SessionNotStartedException
*/
public function logStart(JoinPointInterface $joinPoint)
#[Flow\After("within(Neos\Flow\Session\SessionInterface) && method(.*->start())")]
public function logStart(JoinPointInterface $joinPoint): void
{
/** @var SessionInterface $session */
$session = $joinPoint->getProxy();
if ($session->isStarted()) {
$this->logger->info(sprintf('%s: Started session with id %s.', $this->getClassName($joinPoint), $session->getId()));
$this->logger?->info(sprintf('%s: Started session with id %s.', $this->getSessionImplementationClassName($joinPoint), $session->getId()));
}
}

/**
* Logs calls of resume()
*
* @Flow\After("within(Neos\Flow\Session\SessionInterface) && method(.*->resume())")
* @param JoinPointInterface $joinPoint The current joinpoint
* @return void
*/
public function logResume(JoinPointInterface $joinPoint)
#[Flow\After("within(Neos\Flow\Session\SessionInterface) && method(.*->resume())")]
public function logResume(JoinPointInterface $joinPoint): void
{
/** @var SessionInterface $session */
$session = $joinPoint->getProxy();
if ($session->isStarted()) {
if ($session instanceof SessionInterface && $session->isStarted()) {
$inactivityInSeconds = $joinPoint->getResult();
if ($inactivityInSeconds === 1) {
$inactivityMessage = '1 second';
Expand All @@ -81,76 +58,55 @@ public function logResume(JoinPointInterface $joinPoint)
} else {
$inactivityMessage = sprintf('more than %s hours', (int)($inactivityInSeconds / 3600));
}
$this->logger->debug(sprintf('%s: Resumed session with id %s which was inactive for %s. (%ss)', $this->getClassName($joinPoint), $session->getId(), $inactivityMessage, $inactivityInSeconds));
$this->logger?->debug(sprintf('%s: Resumed session with id %s which was inactive for %s. (%ss)', $this->getSessionImplementationClassName($joinPoint), $session->getId(), $inactivityMessage, $inactivityInSeconds));
}
}

/**
* Logs calls of destroy()
*
* @Flow\Before("within(Neos\Flow\Session\SessionInterface) && method(.*->destroy())")
* @param JoinPointInterface $joinPoint The current joinpoint
* @return void
* @throws SessionNotStartedException
*/
public function logDestroy(JoinPointInterface $joinPoint)
#[Flow\Before("within(Neos\Flow\Session\SessionInterface) && method(.*->destroy())")]
public function logDestroy(JoinPointInterface $joinPoint): void
{
/** @var SessionInterface $session */
$session = $joinPoint->getProxy();
if ($session->isStarted()) {
if ($session instanceof SessionInterface && $session->isStarted()) {
$reason = $joinPoint->isMethodArgument('reason') ? $joinPoint->getMethodArgument('reason') : 'no reason given';
$this->logger->debug(sprintf('%s: Destroyed session with id %s: %s', $this->getClassName($joinPoint), $session->getId(), $reason));
$this->logger?->debug(sprintf('%s: Destroyed session with id %s: %s', $this->getSessionImplementationClassName($joinPoint), $session->getId(), $reason));
}
}

/**
* Logs calls of renewId()
*
* @Flow\Around("within(Neos\Flow\Session\SessionInterface) && method(.*->renewId())")
* @param JoinPointInterface $joinPoint The current joinpoint
* @return mixed The result of the target method if it has not been intercepted
* @throws SessionNotStartedException
*/
public function logRenewId(JoinPointInterface $joinPoint)
#[Flow\Around("within(Neos\Flow\Session\SessionInterface) && method(.*->renewId())")]
public function logRenewId(JoinPointInterface $joinPoint): mixed
{
/** @var SessionInterface $session */
$session = $joinPoint->getProxy();
$oldId = $session->getId();
$newId = $joinPoint->getAdviceChain()->proceed($joinPoint);
if ($session->isStarted()) {
$this->logger->info(sprintf('%s: Changed session id from %s to %s', $this->getClassName($joinPoint), $oldId, $newId));
if (($session instanceof SessionInterface) && $session->isStarted()) {
$this->logger?->info(sprintf('%s: Changed session id from %s to %s', $this->getSessionImplementationClassName($joinPoint), $session->getId(), $newId));
}
return $newId;
}

/**
* Logs calls of collectGarbage()
*
* @Flow\AfterReturning("within(Neos\Flow\Session\SessionInterface) && method(.*->collectGarbage())")
* @param JoinPointInterface $joinPoint The current joinpoint
* @return void
*/
public function logCollectGarbage(JoinPointInterface $joinPoint)
#[Flow\AfterReturning("within(Neos\Flow\Session\SessionInterface) && method(.*->collectGarbage())")]
public function logCollectGarbage(JoinPointInterface $joinPoint): void
{
$sessionRemovalCount = $joinPoint->getResult();
if ($sessionRemovalCount > 0) {
$this->logger->info(sprintf('%s: Triggered garbage collection and removed %s expired sessions.', $this->getClassName($joinPoint), $sessionRemovalCount));
$this->logger?->info(sprintf('%s: Triggered garbage collection and removed %s expired sessions.', $this->getSessionImplementationClassName($joinPoint), $sessionRemovalCount));
} elseif ($sessionRemovalCount === 0) {
$this->logger->info(sprintf('%s: Triggered garbage collection but no sessions needed to be removed.', $this->getClassName($joinPoint)));
$this->logger?->info(sprintf('%s: Triggered garbage collection but no sessions needed to be removed.', $this->getSessionImplementationClassName($joinPoint)));
} elseif ($sessionRemovalCount === false) {
$this->logger->warning(sprintf('%s: Ommitting garbage collection because another process is already running. Consider lowering the GC propability if these messages appear a lot.', $this->getClassName($joinPoint)));
$this->logger?->warning(sprintf('%s: Ommitting garbage collection because another process is already running. Consider lowering the GC probability if these messages appear a lot.', $this->getSessionImplementationClassName($joinPoint)));
}
}

/**
* Determines the short or full class name of the session implementation
*
* @param JoinPointInterface $joinPoint
* @return string
*/
protected function getClassName(JoinPointInterface $joinPoint)
protected function getSessionImplementationClassName(JoinPointInterface $joinPoint): string
{
$className = $joinPoint->getClassName();
$sessionNamespace = substr(SessionInterface::class, 0, -strrpos(SessionInterface::class, '\\') + 1);
if (strpos($className, $sessionNamespace) === 0) {
if (str_starts_with($className, $sessionNamespace)) {
$className = substr($className, strlen($sessionNamespace));
}
return $className;
Expand Down
Loading

0 comments on commit bddcf98

Please sign in to comment.