Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
/build/
/vendor/
/bin/
/.idea/
36 changes: 33 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,65 +42,92 @@ Access the project with your favourite browser. You should see similar welcome s


## Test tasks:
/!\ Précision: je suis sous Windows ("/" "\")

1. Change the text on symfony homepage from "Welcome to Symfony 2.8.8" to "This is a test"
OK

1. Run the PhpUnit test. Check if there are any errors, if so fix them.
OK (en mettant en commentaires "swiftmailer" dans config_test.yml et modifier le test)
(commande à la racine : "phpunit -c app/ src/AppBundle/Tests/Controller/DefaultControllerTest.php")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Je ne comprend pas pourquoi tu as du désactiver le swiftmailer ? Qu'est-ce qui se passait ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK j'ai vu en voulant faire le test, essaies de résoudre le problème sans virer ça du config_test :)

@ThomasPetithory ThomasPetithory Jan 2, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ok j'ai rajouter un require pour le swiftmailer bundle et fais un composer install, et là c'est ok, je l'ai pas configuré par contre


1. Create a new Bundle "InterviewBundle" within the namespace "Test"
OK (changement de l'index de l'appli avec la commande generate:bundle)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

(commande à la racine: "php app/console generate:bundle --namespace=Test/InterviewBundle --bundle-name=TestInterviewBundle")

1. Create a method helloAction under AppBundle\Controller\DefaultController
* for route `/hello`
* with a proper json return `{"hello":"world!"}`
Ok (return JsonResponse)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍


1. Create a "Bios" collection and load the example data into your MongoDB server
* copy the json string from mongodb website ([link](https://docs.mongodb.com/manual/reference/bios-example-collection/))
* or download and load the archive dump ([link](https://raw.githubusercontent.com/OskHa/php_interview_test/master/symfony_mongodb_example.archive))
OK mais il faut que vous fassiez la manip:
* mettre la collection en copier/coller dans une database "test" ou changer dans app/config/parameters.yml
* utiliser le dump symfony_mongodb_example.archive à la racine

1. Define ODM "Bios" document under namespace Test/InterviewBundle/Documents
Ok sauf qu'on ne doit pas utiliser "Documents" mais "Document" pour que le mapping fonctionne (pas trouvé d'autres solutions)
(commande à la racine pour les setters/getters: "php app/console doctrine:mongodb:generate:documents TestInterviewBundle")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍


1. Define ODM "Bios" repository under namespace Test/InterviewBundle/Repositories
Ok sauf qu'on doit utiliser "Repository" et non "Repositories"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Effectivement la bonne pratique est Repository mais ça aurait pu fonctionner aussi avec Repositories ;)

(commande à la racine; "php app/console doctrine:mongodb:generate:repositories TestInterviewBundle")

/!\A partir de ce point, j'ai créé une methode (avec la route \testdb) pour tester les methodes demandées
De plus, les nouvelles routes sont définies dans le routing du Bundle pour la portabilité
1. Implement following repository methods
* findByFirstName($firstName)
* findByContribution($contributionName)
* findByDeadBefore($year)
OK (on peut voir les résultats sur /testdb, on peut changer les paramètres des recherches dans le controller
src/Test/InterviewBundle/Controller/DefaultController.php)
/!\ les ids de la db sont parfois 'bizarre' (expl: 51df07b094c6acd67e492f41 (oui oui c'est bien un id))

1. Define and create a service "BiosService" under namespace Test/InterviewBundle/Services and implement following methods
* getAllAwards()
* Use the logger to log operations (error, warning, debug)
Ok, inscrit dans les services du Bundle (pour la portabilité)
Cependant, j'ai utilisé ma classe test pour tester le service, mais je ne me suis pas servi du logger

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ca serait bien d'implémenter le logger stp :).

Sur les retours, un service ne doit pas renvoyer de Response (une Response c'est essentiellement un objet qui HTTP pour renvoeyr des données au navigateur et donc, envoyé que par le controller). Est-ce que tu peux modifier un peu le code pour que ça soit plus propre là dessus ?

De même, ton renvoi "false" depuis tes Repository me semble un peu brouillon car je pense qu'il y'a moyen d'éviter ça derrière, mais je termine la revue pour te le confirmer !

Sinon le reste me semble bien !

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

J'ai injecter le logger dans le BiosService

1. Create ContributionsController under namespace Test/InterviewBundle/Controller
OK

1. Add a contributionsAction method to your ContributionsController
* for route `/contributions`
* make a use of your BiosService
* avoid logic under controller
* method should list all contributions
* with a proper json return `["contrib", ...]`
OK

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bon, ça va venir contredire mon post précédent de "un service ne doit pas renvoyer de Response" car dans ce cas précis, je comprend pourquoi tu l'as fait.

Je pense que je n'aurai pas géré ça comme ça (genre je pense que j'aurai assigné le résultat du service dans une variable et renvoyé la response depuis le controller pour des questions de réutilisation du code). Mais ce que tu as fait fonctionne et on est sur de l'affinage donc pas de souci !

1. Add a biosByContributionAction method to your ContributionsController
* for route `/contributions/{contributionName}`
* make a use of your BiosService
* avoid logic under controller
* method should list all bios documents with provided contribution
* with a proper json return `[{...}]`
Ok

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

idem

1. make a unit test for the controller
* check if route `/hello` has response code 200
* check if route `/hello` response is a json
* check if route `/contributions` has response code 200
* check if route `/contributions/fake` has response code 404
* check if route `/contributions/OOP` has response code 200

Ok j'ai mis les tests dans Test/InterviewBundle/Tests/Controller/DefaultControllerTest.php
(commande à la racine: "phpunit -c app/ src/Test/InterviewBundle//Tests/Controller/DefaultControllerTest.php")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

1. make a unit test for the BiosService
* at least 1 method of your choice
Ok j'ai mis un test de 'getAllContributions' avec retour Json à la suite des tests précédents

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Alors, si tu test le service, il faut créer une nouvelle class (et pas mettre ton test dans la classe de test du controller).

Le test est pas mal sinon !

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

J'ai séparé les deux tests et corriger celui de service :
en fait, il vérifiait la réponse de $client->request('GET', '/contributions/OOP'); (qui rend un json) situé au dessus et non le retour du service!

1. write a command called `test:command` that should accept 1 argument called id under namespace Test/InterviewBundle/Command
* The command should check if a Bios document with an id of the argument exists
* if document exists, return info "document exists"
* if document doesnt exist, return error "document doesnt exist"

Ok (par contre Mongo a un petit soucis pour find un objet avec un id, pas moyen de le faire proprement en 'requête' alors j'ai du le faire en php)

## Bonus tasks

Expand All @@ -109,14 +136,17 @@ Access the project with your favourite browser. You should see similar welcome s
test:
ping: pong
```
Ok

1. Check the symfony application for errors and fix them if any.
Ok, j'ai du modifier "test:" en "test_interview:" qui est le namespace du bundle
et ajouter du code dans le DependencyInjection/Configuration pour ajouter le paramètre ping de valeur pong (valeur par défaut obligatoire)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

1. write a prompt for the command `test:command`
* Prompt text is "This is a test. Do you want to continue (y/N) ?"
* If you decline, return error "Nothing done. Exiting..."
* If you accept, run the command

OK, j'ai du modifier le code demandé en 15 forcément

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ca marche, avec un check sur le paramètre qui devrait être obligatoire serait encore mieux ;)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Well done, mais c'est juste une vérification d'existence, un message et un exit

# That's it!
## Thank you for your participation! Good luck submitting your results!
2 changes: 2 additions & 0 deletions app/AppKernel.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public function registerBundles()
new Symfony\Bundle\MonologBundle\MonologBundle(),
new Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle(),
new AppBundle\AppBundle(),
new Test\InterviewBundle\TestInterviewBundle(),
new Symfony\Bundle\SwiftmailerBundle\SwiftmailerBundle(),
);

if (in_array($this->getEnvironment(), array('dev', 'test'), true)) {
Expand Down
12 changes: 11 additions & 1 deletion app/Resources/views/default/index.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
<div id="wrapper">
<div id="container">
<div id="welcome">
<h1><span>Welcome to</span> Symfony {{ constant('Symfony\\Component\\HttpKernel\\Kernel::VERSION') }}</h1>
<!-- Demande 1 -->
<h1>This is a test</h1>
<!--<h1><span>Welcome to</span> Symfony {{ constant('Symfony\\Component\\HttpKernel\\Kernel::VERSION') }}</h1>-->
</div>

<div id="status">
Expand Down Expand Up @@ -39,6 +41,14 @@
<a href="http://symfony.com/doc/{{ constant('Symfony\\Component\\HttpKernel\\Kernel::VERSION')[:3] }}/book/page_creation.html">
How to create your first page in Symfony
</a>
<br/>
<!-- petit rajout pour avoir les liens directement sur l'acceuil -->
<ul>
<li><a href="{{ path('test_interview_homepage') }}">Index du bundle Test</a></li>
<li><a href="{{ path('hello') }}">Action Hello du controleur default du bundle App</a></li>
<li><a href="{{ path('testdb') }}">Page perso de Test pour les différents méthodes et service demandés</a></li>
<li><a href="{{ path('contributions') }}">Page des contributions</a></li>
</ul>
</p>
</div>

Expand Down
91 changes: 70 additions & 21 deletions app/SymfonyRequirements.php
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,9 @@ public function __construct($cfgName, $evaluation, $approveCfgAbsence = false, $
*/
class RequirementCollection implements IteratorAggregate
{
/**
* @var Requirement[]
*/
private $requirements = array();

/**
Expand Down Expand Up @@ -265,7 +268,7 @@ public function addCollection(RequirementCollection $collection)
/**
* Returns both requirements and recommendations.
*
* @return array Array of Requirement instances
* @return Requirement[]
*/
public function all()
{
Expand All @@ -275,7 +278,7 @@ public function all()
/**
* Returns all mandatory requirements.
*
* @return array Array of Requirement instances
* @return Requirement[]
*/
public function getRequirements()
{
Expand All @@ -292,7 +295,7 @@ public function getRequirements()
/**
* Returns the mandatory requirements that were not met.
*
* @return array Array of Requirement instances
* @return Requirement[]
*/
public function getFailedRequirements()
{
Expand All @@ -309,7 +312,7 @@ public function getFailedRequirements()
/**
* Returns all optional recommendations.
*
* @return array Array of Requirement instances
* @return Requirement[]
*/
public function getRecommendations()
{
Expand All @@ -326,7 +329,7 @@ public function getRecommendations()
/**
* Returns the recommendations that were not met.
*
* @return array Array of Requirement instances
* @return Requirement[]
*/
public function getFailedRecommendations()
{
Expand Down Expand Up @@ -376,7 +379,8 @@ public function getPhpIniConfigPath()
*/
class SymfonyRequirements extends RequirementCollection
{
const REQUIRED_PHP_VERSION = '5.3.3';
const LEGACY_REQUIRED_PHP_VERSION = '5.3.3';
const REQUIRED_PHP_VERSION = '5.5.9';

/**
* Constructor that initializes the requirements.
Expand All @@ -386,16 +390,26 @@ public function __construct()
/* mandatory requirements follow */

$installedPhpVersion = phpversion();
$requiredPhpVersion = $this->getPhpRequiredVersion();

$this->addRequirement(
version_compare($installedPhpVersion, self::REQUIRED_PHP_VERSION, '>='),
sprintf('PHP version must be at least %s (%s installed)', self::REQUIRED_PHP_VERSION, $installedPhpVersion),
sprintf('You are running PHP version "<strong>%s</strong>", but Symfony needs at least PHP "<strong>%s</strong>" to run.
Before using Symfony, upgrade your PHP installation, preferably to the latest version.',
$installedPhpVersion, self::REQUIRED_PHP_VERSION),
sprintf('Install PHP %s or newer (installed version is %s)', self::REQUIRED_PHP_VERSION, $installedPhpVersion)
$this->addRecommendation(
$requiredPhpVersion,
'Vendors should be installed in order to check all requirements.',
'Run the <code>composer install</code> command.',
'Run the "composer install" command.'
);

if (false !== $requiredPhpVersion) {
$this->addRequirement(
version_compare($installedPhpVersion, $requiredPhpVersion, '>='),
sprintf('PHP version must be at least %s (%s installed)', $requiredPhpVersion, $installedPhpVersion),
sprintf('You are running PHP version "<strong>%s</strong>", but Symfony needs at least PHP "<strong>%s</strong>" to run.
Before using Symfony, upgrade your PHP installation, preferably to the latest version.',
$installedPhpVersion, $requiredPhpVersion),
sprintf('Install PHP %s or newer (installed version is %s)', $requiredPhpVersion, $installedPhpVersion)
);
}

$this->addRequirement(
version_compare($installedPhpVersion, '5.3.16', '!='),
'PHP version must not be 5.3.16 as Symfony won\'t work properly with it',
Expand Down Expand Up @@ -433,7 +447,7 @@ public function __construct()
);
}

if (version_compare($installedPhpVersion, self::REQUIRED_PHP_VERSION, '>=')) {
if (false !== $requiredPhpVersion && version_compare($installedPhpVersion, $requiredPhpVersion, '>=')) {
$timezones = array();
foreach (DateTimeZone::listAbbreviations() as $abbreviations) {
foreach ($abbreviations as $abbreviation) {
Expand Down Expand Up @@ -681,10 +695,17 @@ function_exists('posix_isatty'),

if (class_exists('Symfony\Component\Intl\Intl')) {
$this->addRecommendation(
\Symfony\Component\Intl\Intl::getIcuDataVersion() === \Symfony\Component\Intl\Intl::getIcuVersion(),
sprintf('intl ICU version installed on your system (%s) should match the ICU data bundled with Symfony (%s)', \Symfony\Component\Intl\Intl::getIcuVersion(), \Symfony\Component\Intl\Intl::getIcuDataVersion()),
'In most cases you should be fine, but please verify there is no inconsistencies between data provided by Symfony and the intl extension. See https://github.com/symfony/symfony/issues/15007 for an example of inconsistencies you might run into.'
\Symfony\Component\Intl\Intl::getIcuDataVersion() <= \Symfony\Component\Intl\Intl::getIcuVersion(),
sprintf('intl ICU version installed on your system is outdated (%s) and does not match the ICU data bundled with Symfony (%s)', \Symfony\Component\Intl\Intl::getIcuVersion(), \Symfony\Component\Intl\Intl::getIcuDataVersion()),
'To get the latest internationalization data upgrade the ICU system package and the intl PHP extension.'
);
if (\Symfony\Component\Intl\Intl::getIcuDataVersion() <= \Symfony\Component\Intl\Intl::getIcuVersion()) {
$this->addRecommendation(
\Symfony\Component\Intl\Intl::getIcuDataVersion() === \Symfony\Component\Intl\Intl::getIcuVersion(),
sprintf('intl ICU version installed on your system (%s) does not match the ICU data bundled with Symfony (%s)', \Symfony\Component\Intl\Intl::getIcuVersion(), \Symfony\Component\Intl\Intl::getIcuDataVersion()),
'To avoid internationalization data inconsistencies upgrade the symfony/intl component.'
);
}
}

$this->addPhpIniRecommendation(
Expand Down Expand Up @@ -718,9 +739,9 @@ function_exists('posix_isatty'),

if (strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') {
$this->addRecommendation(
$this->getRealpathCacheSize() > 1000,
'realpath_cache_size should be above 1024 in php.ini',
'Set "<strong>realpath_cache_size</strong>" to e.g. "<strong>1024</strong>" in php.ini<a href="#phpini">*</a> to improve performance on windows.'
$this->getRealpathCacheSize() >= 5 * 1024 * 1024,
'realpath_cache_size should be at least 5M in php.ini',
'Setting "<strong>realpath_cache_size</strong>" to e.g. "<strong>5242880</strong>" or "<strong>5M</strong>" in php.ini<a href="#phpini">*</a> may improve performance on Windows significantly in some cases.'
);
}

Expand Down Expand Up @@ -759,7 +780,11 @@ protected function getRealpathCacheSize()
{
$size = ini_get('realpath_cache_size');
$size = trim($size);
$unit = strtolower(substr($size, -1, 1));
$unit = '';
if (!ctype_digit($size)) {
$unit = strtolower(substr($size, -1, 1));
$size = (int) substr($size, 0, -1);
}
switch ($unit) {
case 'g':
return $size * 1024 * 1024 * 1024;
Expand All @@ -771,4 +796,28 @@ protected function getRealpathCacheSize()
return (int) $size;
}
}

/**
* Defines PHP required version from Symfony version.
*
* @return string|false The PHP required version or false if it could not be guessed
*/
protected function getPhpRequiredVersion()
{
if (!file_exists($path = __DIR__.'/../composer.lock')) {
return false;
}

$composerLock = json_decode(file_get_contents($path), true);
foreach ($composerLock['packages'] as $package) {
$name = $package['name'];
if ('symfony/symfony' !== $name && 'symfony/http-kernel' !== $name) {
continue;
}

return (int) $package['version'][1] > 2 ? self::REQUIRED_PHP_VERSION : self::LEGACY_REQUIRED_PHP_VERSION;
}

return false;
}
}
13 changes: 8 additions & 5 deletions app/check.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@

$messages = array();
foreach ($symfonyRequirements->getRequirements() as $req) {
/** @var $req Requirement */
if ($helpText = get_error_message($req, $lineSize)) {
echo_style('red', 'E');
$messages['error'][] = $helpText;
Expand Down Expand Up @@ -120,10 +119,14 @@ function echo_block($style, $title, $message)

echo PHP_EOL.PHP_EOL;

echo_style($style, str_repeat(' ', $width).PHP_EOL);
echo_style($style, str_pad(' ['.$title.']', $width, ' ', STR_PAD_RIGHT).PHP_EOL);
echo_style($style, str_pad($message, $width, ' ', STR_PAD_RIGHT).PHP_EOL);
echo_style($style, str_repeat(' ', $width).PHP_EOL);
echo_style($style, str_repeat(' ', $width));
echo PHP_EOL;
echo_style($style, str_pad(' ['.$title.']', $width, ' ', STR_PAD_RIGHT));
echo PHP_EOL;
echo_style($style, $message);
echo PHP_EOL;
echo_style($style, str_repeat(' ', $width));
echo PHP_EOL;
}

function has_color_support()
Expand Down
7 changes: 6 additions & 1 deletion app/config/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,9 @@ doctrine_mongodb:
default_database: "%mongodb.default_database%"
document_managers:
default:
auto_mapping: true
auto_mapping: true

# Bonus Task 1
#test: <-- non fonctionnel
test_interview:
ping: pong
7 changes: 5 additions & 2 deletions app/config/config_dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,8 @@ monolog:
# type: chromephp
# level: info

#swiftmailer:
# delivery_address: [email protected]
# Demande 2, on ne sert pas (pour le moment) de mailer donc on peut désactiver la configuration
# pour que le test PhpUnit passe
swiftmailer:
delivery_address: [email protected]
disable_delivery: true
4 changes: 2 additions & 2 deletions app/config/config_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ web_profiler:
toolbar: false
intercept_redirects: false

swiftmailer:
disable_delivery: true
#swiftmailer:
# disable_delivery: true
Loading