Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
Housekeeping fixes (#7)
Browse files Browse the repository at this point in the history
Apply fixes reported by static analysis. 
Ensure global functions are not overloaded.
Decouple controller from ContainerAware.
  • Loading branch information
martin-georgiev authored Oct 24, 2018
1 parent 0757d4e commit c6a60da
Show file tree
Hide file tree
Showing 13 changed files with 128 additions and 155 deletions.
24 changes: 13 additions & 11 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

"require": {
"php": ">=7.1",
"ext-json": "*",
"doctrine/doctrine-bundle": "~1.6",
"doctrine/orm": "~2.1",
"stof/doctrine-extensions-bundle": "*",
Expand All @@ -41,38 +42,39 @@
"behat/symfony2-extension": "^2.1",
"friendsofphp/php-cs-fixer": "*",
"jakub-onderka/php-parallel-lint": "^1.0",
"phpstan/phpstan": "^0.10.2",
"phpunit/phpunit": "^5.7",
"satooshi/php-coveralls": "^2.0",
"php-coveralls/php-coveralls": "^2.1",
"phpstan/phpstan": "^0.10.5",
"phpstan/phpstan-phpunit": "^0.10.0",
"phpunit/phpunit": "^7.4",
"sensiolabs/security-checker": "^4.1",
"symfony/phpunit-bridge": "^3.0|^4.0"
},

"scripts": {
"check-code-style": [
"bin/php-cs-fixer fix --config='./.php_cs' --show-progress=none --dry-run --no-interaction --diff -v"
"php-cs-fixer fix --config='./.php_cs' --show-progress=none --dry-run --no-interaction --diff -v"
],
"check-security": [
"bin/security-checker security:check"
"security-checker security:check"
],
"fix-code-style": [
"bin/php-cs-fixer fix --config='./.php_cs' --show-progress=none --no-interaction --diff -v"
"php-cs-fixer fix --config='./.php_cs' --show-progress=none --no-interaction --diff -v"
],
"run-static-analysis": [
"bin/phpstan analyse --level=1 src/"
"phpstan analyse --level=7 src/"
],
"run-static-analysis-including-tests": [
"@run-static-analysis",
"bin/phpstan analyse --level=1 tests/"
"phpstan analyse --level=3 tests/"
],
"run-tests": [
"bin/phpunit"
"phpunit"
],
"run-tests-with-clover": [
"bin/phpunit --coverage-clover build/logs/clover.xml"
"phpunit --coverage-clover build/logs/clover.xml"
],
"validate-files": [
"bin/parallel-lint --exclude vendor --exclude bin ."
"parallel-lint --exclude vendor --exclude bin ."
]
},

Expand Down
2 changes: 2 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
includes:
- vendor/phpstan/phpstan-phpunit/extension.neon
5 changes: 2 additions & 3 deletions phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
convertWarningsToExceptions="true"
stopOnFailure="false"
bootstrap="vendor/autoload.php"
verbose="true"
syntaxCheck="true">
verbose="true">

<testsuites>
<testsuite>
<testsuite name="VisitorBundle Full Test Suite">
<directory>./tests</directory>
</testsuite>
</testsuites>
Expand Down
39 changes: 30 additions & 9 deletions src/Controller/DeviceController.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,38 @@

use Alpha\VisitorTrackingBundle\Entity\Device;
use Alpha\VisitorTrackingBundle\Entity\Session;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Alpha\VisitorTrackingBundle\Manager\DeviceFingerprintManager;
use Alpha\VisitorTrackingBundle\Storage\SessionStore;
use Doctrine\ORM\EntityManager;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;

class DeviceController extends Controller
class DeviceController
{
private $entityManager;

private $logger;

private $sessionStore;

private $deviceFingerprintManager;

public function __construct(
EntityManager $entityManager,
LoggerInterface $logger,
SessionStore $sessionStore,
DeviceFingerprintManager $deviceFingerprintManager
) {
$this->entityManager = $entityManager;
$this->logger = $logger;
$this->sessionStore = $sessionStore;
$this->deviceFingerprintManager = $deviceFingerprintManager;
}

public function fingerprintAction(Request $request): Response
{
$em = $this->getDoctrine()->getManager();

$session = $this->get('alpha.visitor_tracking.storage.session')->getSession();
$session = $this->sessionStore->getSession();
$device = null;

if ($session instanceof Session) {
Expand All @@ -30,12 +51,12 @@ public function fingerprintAction(Request $request): Response
$device->setFingerprint($request->getContent());
$device->setSession($session);

$this->get('alpha.visitor_tracking.manager.device_fingerprint')->generateHashes($device);
$this->deviceFingerprintManager->generateHashes($device);

$em->persist($device);
$em->flush($device);
$this->entityManager->persist($device);
$this->entityManager->flush($device);

$this->get('logger')->debug(sprintf('A new device fingerprint was added with the id %d.', $device->getId()));
$this->logger->debug(\sprintf('A new device fingerprint was added with the id %d.', $device->getId()));

return new Response('', 201);
}
Expand Down
1 change: 0 additions & 1 deletion src/DependencyInjection/AlphaVisitorTrackingExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ public function load(array $configs, ContainerBuilder $container): void
$configuration = new Configuration();
$config = $this->processConfiguration($configuration, $configs);


$loader = new Loader\YamlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
$loader->load('services.yml');

Expand Down
2 changes: 2 additions & 0 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public function getConfigTreeBuilder(): TreeBuilder
$treeBuilder = new TreeBuilder();
$rootNode = $treeBuilder->root('alpha_visitor_tracking');

\assert($rootNode instanceof ArrayNodeDefinition);
$rootNode->append($this->createSubscriberNode());

return $treeBuilder;
Expand All @@ -23,6 +24,7 @@ public function getConfigTreeBuilder(): TreeBuilder
private function createSubscriberNode(): ArrayNodeDefinition
{
$root = (new TreeBuilder())->root('session_subscriber');
\assert($root instanceof ArrayNodeDefinition);
$root->addDefaultsIfNotSet();

$root->children()->arrayNode('firewall_blacklist')
Expand Down
10 changes: 3 additions & 7 deletions src/Entity/Seed.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,11 @@ class Seed

/**
* @var string
*
* @ORM\Column(type="string")
*/
protected $value;

/**
* @param string $name
* @param int $numberOfValues
* @param array $weights
*/
public function __construct(string $name, int $numberOfValues, ?array $weights = null)
{
$this->name = $name;
Expand Down Expand Up @@ -102,7 +98,7 @@ public function getValue()
private function setValue(int $numberOfValues, ?array $weights = null): void
{
if ($weights === null) {
$weights = array_fill(1, $numberOfValues, 1);
$weights = \array_fill(1, $numberOfValues, 1);
}

if (\count($weights) !== $numberOfValues) {
Expand All @@ -111,7 +107,7 @@ private function setValue(int $numberOfValues, ?array $weights = null): void

// This does not need to be cryptographically secure, mt_rand() is roughly 2x faster than random_int().
/** @noinspection RandomApiMigrationInspection */
$random = mt_rand(1, array_sum($weights));
$random = \mt_rand(1, (int) \array_sum($weights));
$total = 0;

foreach ($weights as $seed => $weight) {
Expand Down
Loading

0 comments on commit c6a60da

Please sign in to comment.