Add a lot of comments and detailed TODO comments

README.md

@@ -197,3 +197,7 @@ hooks:
     build:
         - 'npm install'
 ```
+
+## Contributing
+
+There are a lot of detailed `TODO` notes in the codebase. Feel free to work on these.

bin/php/build.sh

@@ -8,6 +8,10 @@
 
 set -e
 
+# TODO Make a `Makefile` and let us run this script using make
+# The idea is to hide the details of all the tasks behind a single tool.
+# The PHP version could be passed as a command argument instead of the variable below.
+
 PHP_VERSION_GIT_BRANCH=php-7.2.5
 
 echo "Build PHP Binary from current branch '$PHP_VERSION_GIT_BRANCH' on https://github.com/php/php-src"
@@ -23,3 +27,4 @@ tar czf $PHP_VERSION_GIT_BRANCH.tar.gz php
 rm php
 aws s3 cp $PHP_VERSION_GIT_BRANCH.tar.gz s3://bref-php/bin/
 rm $PHP_VERSION_GIT_BRANCH.tar.gz
+# TODO Automatically make the `.tar.gz` file public on AWS S3.

bref

@@ -24,7 +24,16 @@ if (file_exists(__DIR__ . '/vendor/autoload.php')) {
 $app = new Silly\Application('Deploy serverless PHP applications');
 
 $app->command('init', function (SymfonyStyle $io) {
+    /*
+     * TODO We should check that dependencies are correctly installed
+     * - check that `serverless` is installed
+     * - check that it is configured with environment variables
+     * If not, print a link to Bref's documentation and exit with an error.
+     */
     $fs = new Filesystem;
+    /*
+     * TODO Ask for a project name and configure it in `serverless.yml`.
+     */
     if (!file_exists('serverless.yml')) {
         $io->writeln('Creating serverless.yml');
         $fs->copy(__DIR__ . '/template/serverless.yml', 'serverless.yml');
@@ -34,11 +43,12 @@ $app->command('init', function (SymfonyStyle $io) {
         $fs->copy(__DIR__ . '/template/bref.php', 'bref.php');
     }
     $io->success([
-        'Project initialized and ready to deploy using "bref deploy"',
+        'Project initialized and ready to deploy using `bref deploy`',
         'If you are using git, you will need to commit the following files:',
         '- bref.php',
         '- serverless.yml',
-        'You can add `/.bref/` to your .gitignore:',
+        // TODO Do this automatically
+        'You can add `/.bref/` to your .gitignore.',
     ]);
 });
 
@@ -53,35 +63,57 @@ $app->command('deploy', function (SymfonyStyle $io) {
     // Parse .bref.yml
     $projectConfig = [];
     if ($fs->exists('.bref.yml')) {
+        /*
+         * TODO validate the content of the config, for example we should
+         * error if there are unknown keys. Using the Symfony Config component
+         * for that could make sense.
+         */
         $projectConfig = Yaml::parse(file_get_contents('.bref.yml'));
     }
 
     $io->writeln('Building the project in the `.bref/output` directory');
-    // TODO Mirror the directory instead of recreating it from scratch every time
-    // Blocked by https://github.com/symfony/symfony/pull/26399
+    /*
+     * TODO Mirror the directory instead of recreating it from scratch every time
+     * Blocked by https://github.com/symfony/symfony/pull/26399
+     * In the meantime we destroy `.bref/output` completely every time which
+     * is not efficient.
+     */
     $fs->remove('.bref/output');
     $fs->mkdir('.bref/output');
     $filesToCopy = new Finder;
     $filesToCopy->in('.')
         ->depth(0)
-        ->exclude('.bref')
+        ->exclude('.bref') // avoid a recursive copy
         ->ignoreDotFiles(false);
     foreach ($filesToCopy as $fileToCopy) {
         if (is_file($fileToCopy->getPathname())) {
             $fs->copy($fileToCopy->getPathname(), '.bref/output/' . $fileToCopy->getFilename());
         } else {
             $fs->mirror($fileToCopy->getPathname(), '.bref/output/' . $fileToCopy->getFilename(), null, [
-                    'copy_on_windows' => true, // Force to copy symlink content
+                'copy_on_windows' => true, // Force to copy symlink content
             ]);
         }
     }
 
-    // Cache PHP's binary in `.bref/bin/php`
+    // Cache PHP's binary in `.bref/bin/php` to avoid downloading it
+    // on every deploy.
+    /*
+     * TODO Allow choosing a PHP version instead of using directly the
+     * constant `PHP_TARGET_VERSION`. That could be done using the `.bref.yml`
+     * config file: there could be an option in that config, for example:
+     * php:
+     *     version: 7.2.2
+     */
     if (!$fs->exists('.bref/bin/php/php-' . PHP_TARGET_VERSION . '.tar.gz')) {
         $io->writeln('Downloading PHP in the `.bref/bin/` directory');
         $fs->mkdir('.bref/bin/php');
         $defaultUrl = 'https://s3.amazonaws.com/bref-php/bin/php-' . PHP_TARGET_VERSION . '.tar.gz';
-        // TODO document this option
+        /*
+         * TODO This option allows to customize the PHP binary used. It should be documented
+         * and probably moved to a dedicated option like:
+         * php:
+         *     url: 'https://s3.amazonaws.com/...'
+         */
         $url = $projectConfig['php'] ?? $defaultUrl;
         $commandRunner->run("curl -sSL $url -o .bref/bin/php/php-" . PHP_TARGET_VERSION . ".tar.gz");
     }
@@ -96,7 +128,12 @@ $app->command('deploy', function (SymfonyStyle $io) {
     $io->writeln('Installing composer dependencies');
     $commandRunner->run('cd .bref/output && composer install --no-dev --classmap-authoritative --no-scripts');
 
-    // TODO edit serverless.yml to auto-add .bref
+    /*
+     * TODO Edit the `serverless.yml` copy (in `.bref/output` to deploy these files:
+     * - bref.php
+     * - handler.js
+     * - .bref/**
+     */
 
     // Run build hooks defined in .bref.yml
     $buildHooks = $projectConfig['hooks']['build'] ?? [];
@@ -108,13 +145,17 @@ $app->command('deploy', function (SymfonyStyle $io) {
     $io->writeln('Uploading the lambda');
     $commandRunner->run('cd .bref/output && serverless deploy');
 
+    // Trigger a desktop notification
     $notifier = NotifierFactory::create();
     $notification = (new Notification)
         ->setTitle('Deployment success')
         ->setBody('Bref has deployed your application');
     $notifier->send($notification);
 });
 
+/**
+ * Run a CLI command in the remote environment.
+ */
 $app->command('cli [arguments]*', function (array $arguments, SymfonyStyle $io) {
     $commandRunner = new CommandRunner;
 

src/Application.php

@@ -19,6 +19,10 @@
  */
 class Application
 {
+    /**
+     * We should that directory to store the output file.
+     * See `writeLambdaOutput()`.
+     */
     private const BREF_DIRECTORY = '/tmp/.bref';
     private const OUTPUT_FILE_NAME = self::BREF_DIRECTORY . '/output.json';
 
@@ -39,6 +43,7 @@ class Application
 
     public function __construct()
     {
+        // Define default "demo" handlers
         $this->simpleHandler(function () {
             return 'Welcome to Bref! Define your handler using $application->simpleHandler()';
         });
@@ -47,7 +52,8 @@ public function __construct()
     }
 
     /**
-     * Set the handler that will handle simple invocations of the lambda (through `serverless invoke`).
+     * Set the handler that will handle simple invocations of the lambda
+     * (through `serverless invoke` for example).
      *
      * @param callable $handler This callable takes a $event parameter (array) and must return anything serializable to JSON.
      */
@@ -58,6 +64,9 @@ public function simpleHandler(callable $handler) : void
 
     /**
      * Set the handler that will handle HTTP requests.
+     *
+     * The handler must be a PSR-15 request handler, it can be any
+     * framework that is compatible with PSR-15 for example.
      */
     public function httpHandler(RequestHandlerInterface $handler) : void
     {
@@ -66,6 +75,13 @@ public function httpHandler(RequestHandlerInterface $handler) : void
 
     /**
      * Set the handler that will handle CLI requests.
+     *
+     * CLI requests are local invocations of `bref cli <command>`:
+     * the command will be run in production remotely using this handler.
+     *
+     * The handler must be an instance of a Symfony Console application.
+     * That can also be a Silly (https://github.com/mnapoli/silly) application
+     * since Silly is based on the Symfony Console.
      */
     public function cliHandler(\Symfony\Component\Console\Application $console) : void
     {
@@ -77,6 +93,9 @@ public function cliHandler(\Symfony\Component\Console\Application $console) : vo
 
     /**
      * Run the application.
+     *
+     * The application will detect how the lambda is being invoked (HTTP,
+     * CLI, direct invocation, etc.) and execute the proper handler.
      */
     public function run() : void
     {
@@ -130,17 +149,28 @@ private function ensureTempDirectoryExists() : void
 
     private function readLambdaEvent() : array
     {
+        // The lambda event is passed as JSON by `handler.js` as a CLI argument
         global $argv;
         return json_decode($argv[1], true) ?: [];
     }
 
     private function writeLambdaOutput(string $json) : void
     {
+        /*
+         * TODO Avoid using a file for the output?
+         * Avoiding using a file would be (most probably) faster and cleaner.
+         * Maybe stdout could be used to pass the output back to `handler.js`
+         * but any `echo` in the code would mess up the JSON output.
+         * That maybe could be mitigated using output buffering, I haven't
+         * explored that solution yet.
+         */
         file_put_contents(self::OUTPUT_FILE_NAME, $json);
     }
 
     private function isRunningInAwsLambda() : bool
     {
+        // LAMBDA_TASK_ROOT is a constant defined by AWS
+        // TODO: use a solution that would work with other hosts?
         return getenv('LAMBDA_TASK_ROOT') !== false;
     }
 }

src/Bridge/Psr7/RequestFactory.php

@@ -29,10 +29,13 @@ public static function fromLambdaEvent(array $event) : ServerRequestInterface
         $uri = $event['requestContext']['path'] ?? '/';
         $headers = $event['headers'] ?? [];
         $protocolVersion = $event['requestContext']['protocol'] ?? '1.1';
-        // TODO
+        // TODO Parse HTTP headers for cookies.
         $cookies = [];
 
         $contentType = $headers['Content-Type'] ?? null;
+        /*
+         * TODO Multipart form uploads are not supported yet.
+         */
         if ($method === 'POST' && $contentType === 'application/x-www-form-urlencoded') {
             parse_str($bodyString, $parsedBody);
         }

template/serverless.yml

@@ -19,7 +19,7 @@ functions:
   # By default we create one "main" function
   main:
     handler: handler.handle
-    timeout: 20
+    timeout: 20 # Timeout in seconds, the default is 6 seconds
     # The function will match all HTTP URLs
     events:
       - http: 'ANY /'