docs: add changelog for 4.2.5

Closes #17632

.circleci/config.yml

@@ -1,20 +1,42 @@
+defaults: &defaults
+  working_directory: ~/ng
+  docker:
+    - image: angular/ngcontainer
+
 version: 2
 jobs:
+  lint:
+    <<: *defaults
+    steps:
+      - checkout:
+          # After checkout, rebase on top of master.
+          # Similar to travis behavior, but not quite the same.
+          # See https://discuss.circleci.com/t/1662
+          post: git pull --ff-only origin "refs/pull/${CI_PULL_REQUEST//*pull\//}/merge"
+      - restore_cache:
+          key: angular-{{ .Branch }}-{{ checksum "npm-shrinkwrap.json" }}
+
+      - run: npm install
+      - run: npm run postinstall
+      - run: ./node_modules/.bin/gulp lint
+
   build:
-    working_directory: ~/ng
+    <<: *defaults
-    docker:
-      - image: angular/ngcontainer
     steps:
       - checkout
       - restore_cache:
           key: angular-{{ .Branch }}-{{ checksum "npm-shrinkwrap.json" }}
-      - run: npm install
+
-      - run: npm run postinstall
+      - run: bazel run @io_bazel_rules_typescript_node//:bin/npm install
-      - run: ./node_modules/.bin/gulp lint
+      - run: bazel build ...
-      # Build twice, workaround for
-      # https://github.com/bazelbuild/bazel/issues/3114
-      - run: bazel build ... || bazel build ...
       - save_cache:
           key: angular-{{ .Branch }}-{{ checksum "npm-shrinkwrap.json" }}
           paths:
             - "node_modules"
+
+workflows:
+  version: 2
+  default_workflow:
+    jobs:
+      - lint
+      - build

.travis.yml

@@ -37,6 +37,10 @@ env:
     # This is needed for publishing builds to the "aio-staging" and "angular-io" firebase projects.
     # This token was generated using the aio-deploy@angular.io account using `firebase login:ci` and password from valentine
     - secure: "L5CyQmpwWtoR4Qi4xlWQh/cL1M6ZeJL4W4QAr4HdKFMgYt9h+Whqkymyh2NxwmCbPvWa7yUd+OiLQUDCY7L2VIg16hTwoe2CgYDyQA0BEwLzxtRrJXl93TfwMlrUx5JSIzAccD6D4sjtz8kSFMomK2Nls33xOXOukwyhVMjd0Cg="
+    # ANGULAR_PAYLOAD_FIREBASE_TOKEN
+    # This is for payload size data to "angular-payload-size" firebase project
+    # This token was generated using the payload@angular.io account using `firebase login:ci` and password from valentine
+    - secure: "SxotP/ymNy6uWAVbfwM9BlwETPEBpkRvU/F7fCtQDDic99WfQHzzUSQqHTk8eKk3GrGAOSL09vT0WfStQYEIGEoS5UHWNgOnelxhw+d5EnaoB8vQ0dKQBTK092hQg4feFprr+B/tCasyMV6mVwpUzZMbIJNn/Rx7H5g1bp+Gkfg="
   matrix:
     # Order: a slower build first, so that we don't occupy an idle travis worker waiting for others to complete.
     - CI_MODE=e2e
@@ -72,4 +76,5 @@ script:
   - ./scripts/ci/angular.sh
   # all the scripts under this line will not quickly abort in case ${TRAVIS_TEST_RESULT} is 1 (job failure)
   - ./scripts/ci/cleanup.sh
-  - ./scripts/ci/print-logs.sh
+  # Disabled so we can debug Travis build failures on Master seeming to coming from printing logs
+  # - ./scripts/ci/print-logs.sh

CHANGELOG.md

@@ -1,3 +1,43 @@
+<a name="4.2.5"></a>
+## [4.2.5](https://github.com/angular/angular/compare/4.2.4...4.2.5) (2017-06-30)
+
+
+### Bug Fixes
+
+* **animations:** do not delay style() values before a stagger() runs ([7559b78](https://github.com/angular/angular/commit/7559b78)), closes [#17412](https://github.com/angular/angular/issues/17412)
+* **animations:** do not remove container nodes when children are queried by a parent animation ([ec4ae60](https://github.com/angular/angular/commit/ec4ae60)), closes [#17746](https://github.com/angular/angular/issues/17746)
+* **animations:** do not validate style overlap errors in different transitions ([6909171](https://github.com/angular/angular/commit/6909171))
+* **animations:** properly collect :enter nodes that exist within multi-level DOM trees ([79b6346](https://github.com/angular/angular/commit/79b6346)), closes [#17632](https://github.com/angular/angular/issues/17632)
+* **core:** add needed closure compiler warning suppression ([f31b0d6](https://github.com/angular/angular/commit/f31b0d6))
+
+
+
+<a name="4.3.0-beta.0"></a>
+# [4.3.0-beta.0](https://github.com/angular/angular/compare/4.2.1...4.3.0-beta.0) (2017-06-22)
+
+
+### Bug Fixes
+
+* **animations:** compute removal node height correctly ([185075d](https://github.com/angular/angular/commit/185075d))
+* **animations:** do not treat a `0` animation state as `void` ([451257a](https://github.com/angular/angular/commit/451257a))
+* **animations:** properly collect :enter nodes in a partially updated collection ([6ca4692](https://github.com/angular/angular/commit/6ca4692)), closes [#17440](https://github.com/angular/angular/issues/17440)
+* **animations:** remove duplicate license header ([e096a85](https://github.com/angular/angular/commit/e096a85))
+* **compiler:** avoid emitting self importing factories ([4352dd2](https://github.com/angular/angular/commit/4352dd2))
+* **compiler-cli:** find lazy routes in nested module import arrays ([8c89cc4](https://github.com/angular/angular/commit/8c89cc4))
+* **core:** argument destructuring sometimes breaks strictNullChecks ([c59c390](https://github.com/angular/angular/commit/c59c390))
+* **forms:** roll back breaking change with min/max directives ([3e685f9](https://github.com/angular/angular/commit/3e685f9)), closes [#17491](https://github.com/angular/angular/issues/17491)
+* **language-service:** infer `any` `ngForOf` of type `any` ([f194f18](https://github.com/angular/angular/commit/f194f18))
+* **language-service:** rollup `tslib` into the language service package ([4e6be15](https://github.com/angular/angular/commit/4e6be15))
+* **router:** update the version placeholder so that it gets replaced during the build ([d3c92a3](https://github.com/angular/angular/commit/d3c92a3)), closes [#17403](https://github.com/angular/angular/issues/17403)
+* **tsc-wrapped:** skip collecting metadata for default functions ([46ddf50](https://github.com/angular/angular/commit/46ddf50))
+
+
+### Features
+
+* **core:** update zone.js to 0.8.12 ([5ac3919](https://github.com/angular/angular/commit/5ac3919))
+
+
+
 <a name="4.2.4"></a>
 ## [4.2.4](https://github.com/angular/angular/compare/4.2.3...4.2.4) (2017-06-21)
 
@@ -668,7 +708,7 @@ Please give this RC a try and test it with your projects! We have spent a signif
 ## What’s New
 
 ### View Engine
-We’ve made changes under to hood to what AOT generated code looks like. These changes should reduce the size of the generated code for your components by more than half in some cases. Read the [Design Doc](https://docs.google.com/document/d/195L4WaDSoI_kkW094LlShH6gT3B7K1GZpSBnnLkQR-g/preview) for the View Engine updates.
+We’ve made changes under the hood to what AOT generated code looks like. These changes should reduce the size of the generated code for your components by more than half in some cases. Read the [Design Doc](https://docs.google.com/document/d/195L4WaDSoI_kkW094LlShH6gT3B7K1GZpSBnnLkQR-g/preview) for the View Engine updates.
 
 
 ### Enhanced `*ngIf` syntax

CONTRIBUTING.md

@@ -17,15 +17,15 @@ Help us keep Angular open and inclusive. Please read and follow our [Code of Con
 
 ## <a name="question"></a> Got a Question or Problem?
 
-Please, do not open issues for the general support questions as we want to keep GitHub issues for bug reports and feature requests. You've got much better chances of getting your question answered on [StackOverflow](https://stackoverflow.com/questions/tagged/angular) where the questions should be tagged with tag `angular`.
+Please, do not open issues for the general support questions as we want to keep GitHub issues for bug reports and feature requests. You've got much better chances of getting your question answered on [Stack Overflow](https://stackoverflow.com/questions/tagged/angular) where the questions should be tagged with tag `angular`.
 
-StackOverflow is a much better place to ask questions since:
+Stack Overflow is a much better place to ask questions since:
 
-- there are thousands of people willing to help on StackOverflow
+- there are thousands of people willing to help on Stack Overflow
 - questions and answers stay available for public viewing so your question / answer might help someone else
-- StackOverflow's voting system assures that the best answers are prominently visible.
+- Stack Overflow's voting system assures that the best answers are prominently visible.
 
-To save your and our time we will be systematically closing all the issues that are requests for general support and redirecting people to StackOverflow.
+To save your and our time we will be systematically closing all the issues that are requests for general support and redirecting people to Stack Overflow.
 
 If you would like to chat about the question in real-time, you can reach out via [our gitter channel][gitter].
 
@@ -198,8 +198,7 @@ Must be one of the following:
 * **fix**: A bug fix
 * **perf**: A code change that improves performance
 * **refactor**: A code change that neither fixes a bug nor adds a feature
-* **style**: Changes that do not affect the meaning of the code (white-space, formatting, missing
+* **style**: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
-  semi-colons, etc)
 * **test**: Adding missing tests or correcting existing tests
 
 ### Scope
@@ -223,7 +222,7 @@ The following is the list of supported scopes:
 * **upgrade**
 * **tsc-wrapped**
 
-There is currently few exception to the "use package name" rule:
+There are currently a few exceptions to the "use package name" rule:
 
 * **packaging**: used for changes that change the npm package layout in all of our packages, e.g. public path changes, package.json changes done to all packages, d.ts file/format changes, changes to bundles, etc.
 * **changelog**: used for updating the release notes in CHANGELOG.md

WORKSPACE

@@ -1,12 +1,11 @@
 load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository")
 
 git_repository(
-    name = "io_bazel_rules_typescript",
+   name = "io_bazel_rules_typescript",
-    remote = "https://github.com/bazelbuild/rules_typescript.git",
+   remote = "https://github.com/bazelbuild/rules_typescript.git",
-    commit = "804c5da",
+   commit = "3a8404d",
 )
 
-load("@io_bazel_rules_typescript//:defs.bzl", "node_repositories", "npm_install")
+load("@io_bazel_rules_typescript//:defs.bzl", "node_repositories")
 
-node_repositories()
+node_repositories(package_json = "//:package.json")
-npm_install(package_json = "//:package.json")

aio/README.md

@@ -60,6 +60,9 @@ More specifically, there are sub-folders that contain particular types of conten
 
 We use the [dgeni](https://github.com/angular/dgeni) tool to convert these files into docs that can be viewed in the doc-viewer.
 
+The [Authors Style Guide](https://angular.io/guide/docs-style-guide) prescribes guidelines for
+writing guide pages, explains how to use the documentation classes and components, and how to markup sample source code to produce code snippets.
+
 ### Generating the complete docs
 
 The main task for generating the docs is `yarn docs`. This will process all the source files (API and other),

aio/aio-builds-setup/dockerbuild/Dockerfile

@@ -29,6 +29,8 @@ ARG      AIO_NGINX_PORT_HTTPS=443
 ARG TEST_AIO_NGINX_PORT_HTTPS=4433
 ARG      AIO_REPO_SLUG=angular/angular
 ARG TEST_AIO_REPO_SLUG=test-repo/test-slug
+ARG      AIO_TRUSTED_PR_LABEL="aio: preview"
+ARG TEST_AIO_TRUSTED_PR_LABEL="aio: preview"
 ARG      AIO_UPLOAD_HOSTNAME=upload.localhost
 ARG TEST_AIO_UPLOAD_HOSTNAME=upload.localhost
 ARG      AIO_UPLOAD_MAX_SIZE=20971520
@@ -48,6 +50,7 @@ ENV AIO_BUILDS_DIR=$AIO_BUILDS_DIR                     TEST_AIO_BUILDS_DIR=$TEST
     AIO_REPO_SLUG=$AIO_REPO_SLUG                       TEST_AIO_REPO_SLUG=$TEST_AIO_REPO_SLUG                       \
     AIO_SCRIPTS_JS_DIR=/usr/share/aio-scripts-js                                                                    \
     AIO_SCRIPTS_SH_DIR=/usr/share/aio-scripts-sh                                                                    \
+    AIO_TRUSTED_PR_LABEL=$AIO_TRUSTED_PR_LABEL         TEST_AIO_TRUSTED_PR_LABEL=$TEST_AIO_TRUSTED_PR_LABEL         \
     AIO_UPLOAD_HOSTNAME=$AIO_UPLOAD_HOSTNAME           TEST_AIO_UPLOAD_HOSTNAME=$TEST_AIO_UPLOAD_HOSTNAME           \
     AIO_UPLOAD_MAX_SIZE=$AIO_UPLOAD_MAX_SIZE           TEST_AIO_UPLOAD_MAX_SIZE=$TEST_AIO_UPLOAD_MAX_SIZE           \
     AIO_UPLOAD_PORT=$AIO_UPLOAD_PORT                   TEST_AIO_UPLOAD_PORT=$TEST_AIO_UPLOAD_PORT                   \

aio/aio-builds-setup/dockerbuild/nginx/aio-builds.conf

@@ -15,7 +15,7 @@ server {
 
 # Serve PR-preview requests
 server {
-  server_name "~^pr(?<pr>[1-9][0-9]*)-(?<sha>[0-9a-f]{40})\.";
+  server_name "~^pr(?<pr>[1-9][0-9]*)-(?<sha>[0-9a-f]{7,40})\.";
 
   listen {{$AIO_NGINX_PORT_HTTPS}} ssl http2;
   listen [::]:{{$AIO_NGINX_PORT_HTTPS}} ssl http2;

aio/aio-builds-setup/dockerbuild/scripts-js/lib/clean-up/build-cleaner.ts

@@ -2,6 +2,7 @@
 import * as fs from 'fs';
 import * as path from 'path';
 import * as shell from 'shelljs';
+import {HIDDEN_DIR_PREFIX} from '../common/constants';
 import {GithubPullRequests} from '../common/github-pull-requests';
 import {assertNotMissingOrEmpty} from '../common/utils';
 
@@ -31,8 +32,9 @@ export class BuildCleaner {
         }
 
         const buildNumbers = files.
-          map(Number).       // Convert string to number
+          map(name => name.replace(HIDDEN_DIR_PREFIX, '')).   // Remove the "hidden dir" prefix
-          filter(Boolean);   // Ignore NaN (or 0), because they are not builds
+          map(Number).                                        // Convert string to number
+          filter(Boolean);                                    // Ignore NaN (or 0), because they are not builds
 
         resolve(buildNumbers);
       });
@@ -49,9 +51,11 @@ export class BuildCleaner {
 
   protected removeDir(dir: string) {
     try {
-      // Undocumented signature (see https://github.com/shelljs/shelljs/pull/663).
+      if (shell.test('-d', dir)) {
-      (shell as any).chmod('-R', 'a+w', dir);
+        // Undocumented signature (see https://github.com/shelljs/shelljs/pull/663).
-      shell.rm('-rf', dir);
+        (shell as any).chmod('-R', 'a+w', dir);
+        shell.rm('-rf', dir);
+      }
     } catch (err) {
       console.error(`ERROR: Unable to remove '${dir}' due to:`, err);
     }
@@ -64,8 +68,14 @@ export class BuildCleaner {
     console.log(`Open pull requests: ${openPrNumbers.length}`);
     console.log(`Removing ${toRemove.length} build(s): ${toRemove.join(', ')}`);
 
+    // Try removing public dirs.
     toRemove.
       map(num => path.join(this.buildsDir, String(num))).
       forEach(dir => this.removeDir(dir));
+
+    // Try removing hidden dirs.
+    toRemove.
+      map(num => path.join(this.buildsDir, HIDDEN_DIR_PREFIX + String(num))).
+      forEach(dir => this.removeDir(dir));
   }
 }

aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/constants.ts

@@ -0,0 +1,3 @@
+// Constants
+export const HIDDEN_DIR_PREFIX = 'hidden--';
+export const SHORT_SHA_LEN = 7;

aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-api.ts

@@ -63,7 +63,7 @@ export class GithubApi {
         return items;
       }
 
-      return this.getPaginated(pathname, baseParams, currentPage + 1).then(moreItems => [...items, ...moreItems]);
+      return this.getPaginated<T>(pathname, baseParams, currentPage + 1).then(moreItems => [...items, ...moreItems]);
     });
   }
 

aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/github-pull-requests.ts

@@ -6,6 +6,7 @@ import {GithubApi} from './github-api';
 export interface PullRequest  {
   number: number;
   user: {login: string};
+  labels: {name: string}[];
 }
 
 export type PullRequestState = 'all' | 'closed' | 'open';
@@ -30,7 +31,8 @@ export class GithubPullRequests extends GithubApi {
   }
 
   public fetch(pr: number): Promise<PullRequest> {
-    return this.get<PullRequest>(`/repos/${this.repoSlug}/pulls/${pr}`);
+    // Using the `/issues/` URL, because the `/pulls/` one does not provide labels.
+    return this.get<PullRequest>(`/repos/${this.repoSlug}/issues/${pr}`);
   }
 
   public fetchAll(state: PullRequestState = 'all'): Promise<PullRequest[]> {

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/build-creator.ts

@@ -4,8 +4,9 @@ import {EventEmitter} from 'events';
 import * as fs from 'fs';
 import * as path from 'path';
 import * as shell from 'shelljs';
+import {HIDDEN_DIR_PREFIX, SHORT_SHA_LEN} from '../common/constants';
 import {assertNotMissingOrEmpty} from '../common/utils';
-import {CreatedBuildEvent} from './build-events';
+import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events';
 import {UploadError} from './upload-error';
 
 // Classes
@@ -17,13 +18,46 @@ export class BuildCreator extends EventEmitter {
   }
 
   // Methods - Public
-  public create(pr: string, sha: string, archivePath: string): Promise<any> {
+  public changePrVisibility(pr: string, makePublic: boolean): Promise<void> {
-    const prDir = path.join(this.buildsDir, pr);
+    const {oldPrDir, newPrDir} = this.getCandidatePrDirs(pr, makePublic);
+
+    return Promise.
+      all([this.exists(oldPrDir), this.exists(newPrDir)]).
+      then(([oldPrDirExisted, newPrDirExisted]) => {
+        if (!oldPrDirExisted) {
+          throw new UploadError(404, `Request to move non-existing directory '${oldPrDir}' to '${newPrDir}'.`);
+        } else if (newPrDirExisted) {
+          throw new UploadError(409, `Request to move '${oldPrDir}' to existing directory '${newPrDir}'.`);
+        }
+
+        return Promise.resolve().
+          then(() => shell.mv(oldPrDir, newPrDir)).
+          then(() => this.listShasByDate(newPrDir)).
+          then(shas => this.emit(ChangedPrVisibilityEvent.type, new ChangedPrVisibilityEvent(+pr, shas, makePublic))).
+          then(() => undefined);
+      }).
+      catch(err => {
+        if (!(err instanceof UploadError)) {
+          err = new UploadError(500, `Error while making PR ${pr} ${makePublic ? 'public' : 'hidden'}.\n${err}`);
+        }
+
+        throw err;
+      });
+  }
+
+  public create(pr: string, sha: string, archivePath: string, isPublic: boolean): Promise<void> {
+    // Use only part of the SHA for more readable URLs.
+    sha = sha.substr(0, SHORT_SHA_LEN);
+
+    const {oldPrDir: otherVisPrDir, newPrDir: prDir} = this.getCandidatePrDirs(pr, isPublic);
     const shaDir = path.join(prDir, sha);
     let dirToRemoveOnError: string;
 
-    return Promise.
+    return Promise.resolve().
-      all([this.exists(prDir), this.exists(shaDir)]).
+      then(() => this.exists(otherVisPrDir)).
+      // If the same PR exists with different visibility, update the visibility first.
+      then(otherVisPrDirExisted => (otherVisPrDirExisted && this.changePrVisibility(pr, isPublic)) as any).
+      then(() => Promise.all([this.exists(prDir), this.exists(shaDir)])).
       then(([prDirExisted, shaDirExisted]) => {
         if (shaDirExisted) {
           throw new UploadError(409, `Request to overwrite existing directory: ${shaDir}`);
@@ -34,7 +68,8 @@ export class BuildCreator extends EventEmitter {
         return Promise.resolve().
           then(() => shell.mkdir('-p', shaDir)).
           then(() => this.extractArchive(archivePath, shaDir)).
-          then(() => this.emit(CreatedBuildEvent.type, new CreatedBuildEvent(+pr, sha)));
+          then(() => this.emit(CreatedBuildEvent.type, new CreatedBuildEvent(+pr, sha, isPublic))).
+          then(() => undefined);
       }).
       catch(err => {
         if (dirToRemoveOnError) {
@@ -78,4 +113,26 @@ export class BuildCreator extends EventEmitter {
       });
     });
   }
+
+  protected getCandidatePrDirs(pr: string, isPublic: boolean) {
+    const hiddenPrDir = path.join(this.buildsDir, HIDDEN_DIR_PREFIX + pr);
+    const publicPrDir = path.join(this.buildsDir, pr);
+
+    const oldPrDir = isPublic ? hiddenPrDir : publicPrDir;
+    const newPrDir = isPublic ? publicPrDir : hiddenPrDir;
+
+    return {oldPrDir, newPrDir};
+  }
+
+  protected listShasByDate(inputDir: string): Promise<string[]> {
+    return Promise.resolve().
+      then(() => shell.ls('-l', inputDir) as any as Promise<(fs.Stats & {name: string})[]>).
+      // Keep directories only.
+      // (Also, convert to standard Array - ShellJS provides custom `sort()` method for sorting file contents.)
+      then(items => items.filter(item => item.isDirectory())).
+      // Sort by modification date.
+      then(items => items.sort((a, b) => a.mtime.getTime() - b.mtime.getTime())).
+      // Return directory names.
+      then(items => items.map(item => item.name));
+  }
 }

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/build-events.ts

@@ -1,15 +1,16 @@
 // Classes
-export class BuildEvent {
+export class ChangedPrVisibilityEvent {
+  // Properties - Public, Static
+  public static type = 'pr.changedVisibility';
+
   // Constructor
-  constructor(public type: string, public pr: number, public sha: string) {}
+  constructor(public pr: number, public shas: string[], public isPublic: boolean) {}
 }
 
-export class CreatedBuildEvent extends BuildEvent {
+export class CreatedBuildEvent {
   // Properties - Public, Static
   public static type = 'build.created';
 
   // Constructor
-  constructor(pr: number, sha: string) {
+  constructor(public pr: number, public sha: string, public isPublic: boolean) {}
-    super(CreatedBuildEvent.type, pr, sha);
-  }
 }

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/build-verifier.ts

@@ -1,6 +1,6 @@
 // Imports
 import * as jwt from 'jsonwebtoken';
-import {GithubPullRequests} from '../common/github-pull-requests';
+import {GithubPullRequests, PullRequest} from '../common/github-pull-requests';
 import {GithubTeams} from '../common/github-teams';
 import {assertNotMissingOrEmpty} from '../common/utils';
 import {UploadError} from './upload-error';
@@ -11,6 +11,12 @@ interface JwtPayload {
   'pull-request': number;
 }
 
+// Enums
+export enum BUILD_VERIFICATION_STATUS {
+  verifiedAndTrusted,
+  verifiedNotTrusted,
+}
+
 // Classes
 export class BuildVerifier {
   // Properties - Protected
@@ -19,27 +25,27 @@ export class BuildVerifier {
 
   // Constructor
   constructor(protected secret: string, githubToken: string, protected repoSlug: string, organization: string,
-              protected allowedTeamSlugs: string[]) {
+              protected allowedTeamSlugs: string[], protected trustedPrLabel: string) {
     assertNotMissingOrEmpty('secret', secret);
     assertNotMissingOrEmpty('githubToken', githubToken);
     assertNotMissingOrEmpty('repoSlug', repoSlug);
     assertNotMissingOrEmpty('organization', organization);
     assertNotMissingOrEmpty('allowedTeamSlugs', allowedTeamSlugs && allowedTeamSlugs.join(''));
+    assertNotMissingOrEmpty('trustedPrLabel', trustedPrLabel);
 
     this.githubPullRequests = new GithubPullRequests(githubToken, repoSlug);
     this.githubTeams = new GithubTeams(githubToken, organization);
   }
 
   // Methods - Public
-  public getPrAuthorTeamMembership(pr: number): Promise<{author: string, isMember: boolean}> {
+  public getPrIsTrusted(pr: number): Promise<boolean> {
     return Promise.resolve().
       then(() => this.githubPullRequests.fetch(pr)).
-      then(prInfo => prInfo.user.login).
+      then(prInfo => this.hasLabel(prInfo, this.trustedPrLabel) ||
-      then(author => this.githubTeams.isMemberBySlug(author, this.allowedTeamSlugs).
+                     this.githubTeams.isMemberBySlug(prInfo.user.login, this.allowedTeamSlugs));
-        then(isMember => ({author, isMember})));
   }
 
-  public verify(expectedPr: number, authHeader: string): Promise<void> {
+  public verify(expectedPr: number, authHeader: string): Promise<BUILD_VERIFICATION_STATUS> {
     return Promise.resolve().
       then(() => this.extractJwtString(authHeader)).
       then(jwtString => this.verifyJwt(expectedPr, jwtString)).
@@ -52,9 +58,13 @@ export class BuildVerifier {
     return input.replace(/^token +/i, '');
   }
 
+  protected hasLabel(prInfo: PullRequest, label: string) {
+    return prInfo.labels.some(labelObj => labelObj.name === label);
+  }
+
   protected verifyJwt(expectedPr: number, token: string): Promise<JwtPayload> {
     return new Promise((resolve, reject) => {
-      jwt.verify(token, this.secret, {issuer: 'Travis CI, GmbH'}, (err, payload) => {
+      jwt.verify(token, this.secret, {issuer: 'Travis CI, GmbH'}, (err, payload: JwtPayload) => {
         if (err) {
           reject(err.message || err);
         } else if (payload.slug !== this.repoSlug) {
@@ -68,11 +78,10 @@ export class BuildVerifier {
     });
   }
 
-  protected verifyPr(pr: number): Promise<void> {
+  protected verifyPr(pr: number): Promise<BUILD_VERIFICATION_STATUS> {
-    return this.getPrAuthorTeamMembership(pr).
+    return this.getPrIsTrusted(pr).
-      then(({author, isMember}) => isMember ? Promise.resolve() : Promise.reject(
+      then(isTrusted => Promise.resolve(isTrusted ?
-        `User '${author}' is not an active member of any of the following teams: ` +
+          BUILD_VERIFICATION_STATUS.verifiedAndTrusted :
-        `${this.allowedTeamSlugs.join(', ')}`,
+          BUILD_VERIFICATION_STATUS.verifiedNotTrusted));
-      ));
   }
 }

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/index-preverify-pr.ts

@@ -12,28 +12,28 @@ function _main() {
   const repoSlug = getEnvVar('AIO_REPO_SLUG');
   const organization = getEnvVar('AIO_GITHUB_ORGANIZATION');
   const allowedTeamSlugs = getEnvVar('AIO_GITHUB_TEAM_SLUGS').split(',');
+  const trustedPrLabel = getEnvVar('AIO_TRUSTED_PR_LABEL');
   const pr = +getEnvVar('AIO_PREVERIFY_PR');
 
-  const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, organization, allowedTeamSlugs);
+  const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, organization, allowedTeamSlugs,
+                                          trustedPrLabel);
 
   // Exit codes:
-  // - 0: The PR author is a member.
+  // - 0: The PR can be automatically trusted (i.e. author belongs to trusted team or PR has the "trusted PR" label).
   // - 1: An error occurred.
-  // - 2: The PR author is not a member.
+  // - 2: The PR cannot be automatically trusted.
-  buildVerifier.getPrAuthorTeamMembership(pr).
+  buildVerifier.getPrIsTrusted(pr).
-    then(({author, isMember}) => {
+    then(isTrusted => {
-      if (isMember) {
+      if (!isTrusted) {
-        process.exit(0);
+        console.warn(
-      } else {
+            `The PR cannot be automatically verified, because it doesn't have the "${trustedPrLabel}" label and the ` +
-        const errorMessage = `User '${author}' is not an active member of any of the following teams: ` +
+            `the author is not an active member of any of the following teams: ${allowedTeamSlugs.join(', ')}`);
-                             `${allowedTeamSlugs.join(', ')}`;
-        onError(errorMessage, 2);
       }
-    }).
-    catch(err => onError(err, 1));
-}
 
-function onError(err: string, exitCode: number) {
+      process.exit(isTrusted ? 0 : 2);
-  console.error(err);
+    }).
-  process.exit(exitCode || 1);
+    catch(err => {
+      console.error(err);
+      process.exit(1);
+    });
 }

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/index-test.ts

@@ -1,10 +1,24 @@
 // Imports
 import {GithubPullRequests} from '../common/github-pull-requests';
-import {BuildVerifier} from './build-verifier';
+import {BUILD_VERIFICATION_STATUS, BuildVerifier} from './build-verifier';
+import {UploadError} from './upload-error';
 
 // Run
 // TODO(gkalpak): Add e2e tests to cover these interactions as well.
 GithubPullRequests.prototype.addComment = () => Promise.resolve();
-BuildVerifier.prototype.verify = () => Promise.resolve();
+BuildVerifier.prototype.verify = (expectedPr: number, authHeader: string) => {
+  switch (authHeader) {
+    case 'FAKE_VERIFICATION_ERROR':
+      // For e2e tests, fake a verification error.
+      return Promise.reject(new UploadError(403, `Error while verifying upload for PR ${expectedPr}: Test`));
+    case 'FAKE_VERIFIED_NOT_TRUSTED':
+      // For e2e tests, fake a `verifiedNotTrusted` verification status.
+      return Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedNotTrusted);
+    default:
+      // For e2e tests, default to `verifiedAndTrusted` verification status.
+      return Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedAndTrusted);
+  }
+};
+
 // tslint:disable-next-line: no-var-requires
 require('./index');

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/index.ts

@@ -10,6 +10,7 @@ const AIO_GITHUB_TEAM_SLUGS = getEnvVar('AIO_GITHUB_TEAM_SLUGS');
 const AIO_GITHUB_TOKEN = getEnvVar('AIO_GITHUB_TOKEN');
 const AIO_PREVIEW_DEPLOYMENT_TOKEN = getEnvVar('AIO_PREVIEW_DEPLOYMENT_TOKEN');
 const AIO_REPO_SLUG = getEnvVar('AIO_REPO_SLUG');
+const AIO_TRUSTED_PR_LABEL = getEnvVar('AIO_TRUSTED_PR_LABEL');
 const AIO_UPLOAD_HOSTNAME = getEnvVar('AIO_UPLOAD_HOSTNAME');
 const AIO_UPLOAD_PORT = +getEnvVar('AIO_UPLOAD_PORT');
 const AIO_WWW_USER = getEnvVar('AIO_WWW_USER');
@@ -29,6 +30,7 @@ function _main() {
       githubToken: AIO_GITHUB_TOKEN,
       repoSlug: AIO_REPO_SLUG,
       secret: AIO_PREVIEW_DEPLOYMENT_TOKEN,
+      trustedPrLabel: AIO_TRUSTED_PR_LABEL,
     }).
     listen(AIO_UPLOAD_PORT, AIO_UPLOAD_HOSTNAME);
 }

aio/aio-builds-setup/dockerbuild/scripts-js/lib/upload-server/upload-server-factory.ts

@@ -4,8 +4,8 @@ import * as http from 'http';
 import {GithubPullRequests} from '../common/github-pull-requests';
 import {assertNotMissingOrEmpty} from '../common/utils';
 import {BuildCreator} from './build-creator';
-import {CreatedBuildEvent} from './build-events';
+import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events';
-import {BuildVerifier} from './build-verifier';
+import {BUILD_VERIFICATION_STATUS, BuildVerifier} from './build-verifier';
 import {UploadError} from './upload-error';
 
 // Constants
@@ -21,6 +21,7 @@ interface UploadServerConfig {
   githubToken: string;
   repoSlug: string;
   secret: string;
+  trustedPrLabel: string;
 }
 
 // Classes
@@ -34,14 +35,16 @@ class UploadServerFactory {
     githubToken,
     repoSlug,
     secret,
+    trustedPrLabel,
   }: UploadServerConfig): http.Server {
     assertNotMissingOrEmpty('domainName', domainName);
 
-    const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, githubOrganization, githubTeamSlugs);
+    const buildVerifier = new BuildVerifier(secret, githubToken, repoSlug, githubOrganization, githubTeamSlugs,
+                                            trustedPrLabel);
     const buildCreator = this.createBuildCreator(buildsDir, githubToken, repoSlug, domainName);
 
     const middleware = this.createMiddleware(buildVerifier, buildCreator);
-    const httpServer = http.createServer(middleware);
+    const httpServer = http.createServer(middleware as any);
 
     httpServer.on('listening', () => {
       const info = httpServer.address();
@@ -56,12 +59,24 @@ class UploadServerFactory {
                                domainName: string): BuildCreator {
     const buildCreator = new BuildCreator(buildsDir);
     const githubPullRequests = new GithubPullRequests(githubToken, repoSlug);
+    const postPreviewsComment = (pr: number, shas: string[]) => {
+      const body = shas.
+        map(sha => `You can preview ${sha} at https://pr${pr}-${sha}.${domainName}/.`).
+        join('\n');
 
-    buildCreator.on(CreatedBuildEvent.type, ({pr, sha}: CreatedBuildEvent) => {
+      return githubPullRequests.addComment(pr, body);
-      const body = `The angular.io preview for ${sha} is available [here][1].\n\n` +
+    };
-                   `[1]: https://pr${pr}-${sha}.${domainName}/`;
 
-      githubPullRequests.addComment(pr, body);
+    buildCreator.on(CreatedBuildEvent.type, ({pr, sha, isPublic}: CreatedBuildEvent) => {
+      if (isPublic) {
+        postPreviewsComment(pr, [sha]);
+      }
+    });
+
+    buildCreator.on(ChangedPrVisibilityEvent.type, ({pr, shas, isPublic}: ChangedPrVisibilityEvent) => {
+      if (isPublic && shas.length) {
+        postPreviewsComment(pr, shas);
+      }
     });
 
     return buildCreator;
@@ -80,13 +95,14 @@ class UploadServerFactory {
         this.throwRequestError(401, `Missing or empty '${AUTHORIZATION_HEADER}' header`, req);
       } else if (!archive) {
         this.throwRequestError(400, `Missing or empty '${X_FILE_HEADER}' header`, req);
+      } else {
+        buildVerifier.
+          verify(+pr, authHeader).
+          then(verStatus => verStatus === BUILD_VERIFICATION_STATUS.verifiedAndTrusted).
+          then(isPublic => buildCreator.create(pr, sha, archive, isPublic).
+            then(() => res.sendStatus(isPublic ? 201 : 202))).
+          catch(err => this.respondWithError(res, err));
       }
-
-      buildVerifier.
-        verify(+pr, authHeader).
-        then(() => buildCreator.create(pr, sha, archive)).
-        then(() => res.sendStatus(201)).
-        catch(err => this.respondWithError(res, err));
     });
     middleware.get(/^\/health-check\/?$/, (_req, res) => res.sendStatus(200));
     middleware.get('*', req => this.throwRequestError(404, 'Unknown resource', req));

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/helper.ts

@@ -4,6 +4,7 @@ import * as fs from 'fs';
 import * as http from 'http';
 import * as path from 'path';
 import * as shell from 'shelljs';
+import {HIDDEN_DIR_PREFIX, SHORT_SHA_LEN} from '../common/constants';
 import {getEnvVar} from '../common/utils';
 
 // Constans
@@ -31,10 +32,10 @@ class Helper {
   public get nginxHostname() { return TEST_AIO_NGINX_HOSTNAME; }
   public get nginxPortHttp() { return TEST_AIO_NGINX_PORT_HTTP; }
   public get nginxPortHttps() { return TEST_AIO_NGINX_PORT_HTTPS; }
-  public get wwwUser() { return WWW_USER; }
   public get uploadHostname() { return TEST_AIO_UPLOAD_HOSTNAME; }
   public get uploadPort() { return TEST_AIO_UPLOAD_PORT; }
   public get uploadMaxSize() { return TEST_AIO_UPLOAD_MAX_SIZE; }
+  public get wwwUser() { return WWW_USER; }
 
   // Properties - Protected
   protected cleanUpFns: CleanUpFn[] = [];
@@ -50,6 +51,12 @@ class Helper {
   }
 
   // Methods - Public
+  public buildExists(pr: string, sha = '', isPublic = true, legacy = false): boolean {
+    const prDir = this.getPrDir(pr, isPublic);
+    const dir = !sha ? prDir : this.getShaDir(prDir, sha, legacy);
+    return fs.existsSync(dir);
+  }
+
   public cleanUp() {
     while (this.cleanUpFns.length) {
       // Clean-up fns remove themselves from the list.
@@ -62,11 +69,11 @@ class Helper {
   }
 
   public createDummyArchive(pr: string, sha: string, archivePath: string): CleanUpFn {
-    const inputDir = path.join(this.buildsDir, 'uploaded', pr, sha);
+    const inputDir = this.getShaDir(this.getPrDir(`uploaded/${pr}`, true), sha);
     const cmd1 = `tar --create --gzip --directory "${inputDir}" --file "${archivePath}" .`;
     const cmd2 = `chown ${this.wwwUser} ${archivePath}`;
 
-    const cleanUpTemp = this.createDummyBuild(`uploaded/${pr}`, sha, true);
+    const cleanUpTemp = this.createDummyBuild(`uploaded/${pr}`, sha, true, true);
     shell.exec(cmd1);
     shell.exec(cmd2);
     cleanUpTemp();
@@ -74,9 +81,9 @@ class Helper {
     return this.createCleanUpFn(() => shell.rm('-rf', archivePath));
   }
 
-  public createDummyBuild(pr: string, sha: string, force = false): CleanUpFn {
+  public createDummyBuild(pr: string, sha: string, isPublic = true, force = false, legacy = false): CleanUpFn {
-    const prDir = path.join(this.buildsDir, pr);
+    const prDir = this.getPrDir(pr, isPublic);
-    const shaDir = path.join(prDir, sha);
+    const shaDir = this.getShaDir(prDir, sha, legacy);
     const idxPath = path.join(shaDir, 'index.html');
     const barPath = path.join(shaDir, 'foo', 'bar.js');
 
@@ -87,8 +94,8 @@ class Helper {
     return this.createCleanUpFn(() => shell.rm('-rf', prDir));
   }
 
-  public deletePrDir(pr: string) {
+  public deletePrDir(pr: string, isPublic = true) {
-    const prDir = path.join(this.buildsDir, pr);
+    const prDir = this.getPrDir(pr, isPublic);
 
     if (fs.existsSync(prDir)) {
       // Undocumented signature (see https://github.com/shelljs/shelljs/pull/663).
@@ -97,8 +104,22 @@ class Helper {
     }
   }
 
-  public readBuildFile(pr: string, sha: string, relFilePath: string): string {
+  public getPrDir(pr: string, isPublic: boolean): string {
-    const absFilePath = path.join(this.buildsDir, pr, sha, relFilePath);
+    const prDirName = isPublic ? pr : HIDDEN_DIR_PREFIX + pr;
+    return path.join(this.buildsDir, prDirName);
+  }
+
+  public getShaDir(prDir: string, sha: string, legacy = false): string {
+    return path.join(prDir, legacy ? sha : this.getShordSha(sha));
+  }
+
+  public getShordSha(sha: string): string {
+    return sha.substr(0, SHORT_SHA_LEN);
+  }
+
+  public readBuildFile(pr: string, sha: string, relFilePath: string, isPublic = true, legacy = false): string {
+    const shaDir = this.getShaDir(this.getPrDir(pr, isPublic), sha, legacy);
+    const absFilePath = path.join(shaDir, relFilePath);
     return fs.readFileSync(absFilePath, 'utf8');
   }
 
@@ -129,7 +150,8 @@ class Helper {
       const [headers, body] = result.stdout.
         split(/(?:\r?\n){2,}/).
         map(s => s.trim()).
-        slice(-2);
+        slice(-2);   // In case of redirect, discard the previous headers.
+                     // Only keep the last to sections (final headers and body).
 
       if (!result.success) {
         console.log('Stdout:', result.stdout);
@@ -143,8 +165,10 @@ class Helper {
     };
   }
 
-  public writeBuildFile(pr: string, sha: string, relFilePath: string, content: string): CleanUpFn {
+  public writeBuildFile(pr: string, sha: string, relFilePath: string, content: string, isPublic = true,
-    const absFilePath = path.join(this.buildsDir, pr, sha, relFilePath);
+                        legacy = false): CleanUpFn {
+    const shaDir = this.getShaDir(this.getPrDir(pr, isPublic), sha, legacy);
+    const absFilePath = path.join(shaDir, relFilePath);
     return this.writeFile(absFilePath, {content}, true);
   }
 

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/nginx.e2e.ts

@@ -31,111 +31,184 @@ describe(`nginx`, () => {
   });
 
 
-  h.runForAllSupportedSchemes((scheme, port) => describe(`nginx (on ${scheme.toUpperCase()})`, () => {
+  h.runForAllSupportedSchemes((scheme, port) => describe(`(on ${scheme.toUpperCase()})`, () => {
     const hostname = h.nginxHostname;
     const host = `${hostname}:${port}`;
     const pr = '9';
     const sha9 = '9'.repeat(40);
     const sha0 = '0'.repeat(40);
+    const shortSha9 = h.getShordSha(sha9);
+    const shortSha0 = h.getShordSha(sha0);
 
 
     describe(`pr<pr>-<sha>.${host}/*`, () => {
 
-      beforeEach(() => {
+      describe('(for public builds)', () => {
-        h.createDummyBuild(pr, sha9);
+
-        h.createDummyBuild(pr, sha0);
+        beforeEach(() => {
+          h.createDummyBuild(pr, sha9);
+          h.createDummyBuild(pr, sha0);
+        });
+
+
+        it('should return /index.html', done => {
+          const origin = `${scheme}://pr${pr}-${shortSha9}.${host}`;
+          const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
+
+          Promise.all([
+            h.runCmd(`curl -iL ${origin}/index.html`).then(h.verifyResponse(200, bodyRegex)),
+            h.runCmd(`curl -iL ${origin}/`).then(h.verifyResponse(200, bodyRegex)),
+            h.runCmd(`curl -iL ${origin}`).then(h.verifyResponse(200, bodyRegex)),
+          ]).then(done);
+        });
+
+
+        it('should return /index.html (for legacy builds)', done => {
+          const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
+          const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
+
+          h.createDummyBuild(pr, sha9, true, false, true);
+
+          Promise.all([
+            h.runCmd(`curl -iL ${origin}/index.html`).then(h.verifyResponse(200, bodyRegex)),
+            h.runCmd(`curl -iL ${origin}/`).then(h.verifyResponse(200, bodyRegex)),
+            h.runCmd(`curl -iL ${origin}`).then(h.verifyResponse(200, bodyRegex)),
+          ]).then(done);
+        });
+
+
+        it('should return /foo/bar.js', done => {
+          const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /foo/bar\\.js$`);
+
+          h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo/bar.js`).
+            then(h.verifyResponse(200, bodyRegex)).
+            then(done);
+        });
+
+
+        it('should return /foo/bar.js (for legacy builds)', done => {
+          const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
+          const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /foo/bar\\.js$`);
+
+          h.createDummyBuild(pr, sha9, true, false, true);
+
+          h.runCmd(`curl -iL ${origin}/foo/bar.js`).
+            then(h.verifyResponse(200, bodyRegex)).
+            then(done);
+        });
+
+
+        it('should respond with 403 for directories', done => {
+          Promise.all([
+            h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo/`).then(h.verifyResponse(403)),
+            h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo`).then(h.verifyResponse(403)),
+          ]).then(done);
+        });
+
+
+        it('should respond with 404 for unknown paths to files', done => {
+          h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}/foo/baz.css`).
+            then(h.verifyResponse(404)).
+            then(done);
+        });
+
+
+        it('should rewrite to \'index.html\' for unknown paths that don\'t look like files', done => {
+          const origin = `${scheme}://pr${pr}-${shortSha9}.${host}`;
+          const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
+
+          Promise.all([
+            h.runCmd(`curl -iL ${origin}/foo/baz`).then(h.verifyResponse(200, bodyRegex)),
+            h.runCmd(`curl -iL ${origin}/foo/baz/`).then(h.verifyResponse(200, bodyRegex)),
+          ]).then(done);
+        });
+
+
+        it('should respond with 404 for unknown PRs/SHAs', done => {
+          const otherPr = 54321;
+          const otherShortSha = h.getShordSha('8'.repeat(40));
+
+          Promise.all([
+            h.runCmd(`curl -iL ${scheme}://pr${pr}9-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://pr${otherPr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}9.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://pr${pr}-${otherShortSha}.${host}`).then(h.verifyResponse(404)),
+          ]).then(done);
+        });
+
+
+        it('should respond with 404 if the subdomain format is wrong', done => {
+          Promise.all([
+            h.runCmd(`curl -iL ${scheme}://xpr${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://prx${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://xx${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://p${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://r${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://${pr}-${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://pr${pr}${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://pr${pr}_${shortSha9}.${host}`).then(h.verifyResponse(404)),
+          ]).then(done);
+        });
+
+
+        it('should reject PRs with leading zeros', done => {
+          h.runCmd(`curl -iL ${scheme}://pr0${pr}-${shortSha9}.${host}`).
+            then(h.verifyResponse(404)).
+            then(done);
+        });
+
+
+        it('should accept SHAs with leading zeros (but not trim the zeros)', done => {
+          const bodyRegex9 = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
+          const bodyRegex0 = new RegExp(`^PR: ${pr} | SHA: ${sha0} | File: /index\\.html$`);
+
+          Promise.all([
+            h.runCmd(`curl -iL ${scheme}://pr${pr}-0${shortSha9}.${host}`).then(h.verifyResponse(404)),
+            h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha9}.${host}`).then(h.verifyResponse(200, bodyRegex9)),
+            h.runCmd(`curl -iL ${scheme}://pr${pr}-${shortSha0}.${host}`).then(h.verifyResponse(200, bodyRegex0)),
+          ]).then(done);
+        });
+
       });
 
 
-      it('should return /index.html', done => {
+      describe('(for hidden builds)', () => {
-        const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
-        const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
 
-        Promise.all([
+        it('should respond with 404 for any file or directory', done => {
-          h.runCmd(`curl -iL ${origin}/index.html`).then(h.verifyResponse(200, bodyRegex)),
+          const origin = `${scheme}://pr${pr}-${shortSha9}.${host}`;
-          h.runCmd(`curl -iL ${origin}/`).then(h.verifyResponse(200, bodyRegex)),
+          const assert404 = h.verifyResponse(404);
-          h.runCmd(`curl -iL ${origin}`).then(h.verifyResponse(200, bodyRegex)),
+
-        ]).then(done);
+          h.createDummyBuild(pr, sha9, false);
-      });
+          expect(h.buildExists(pr, sha9, false)).toBe(true);
+
+          Promise.all([
+            h.runCmd(`curl -iL ${origin}/index.html`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/`).then(assert404),
+            h.runCmd(`curl -iL ${origin}`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo/bar.js`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo/`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo`).then(assert404),
+          ]).then(done);
+        });
 
 
-      it('should return /foo/bar.js', done => {
+        it('should respond with 404 for any file or directory (for legacy builds)', done => {
-        const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /foo/bar\\.js$`);
+          const origin = `${scheme}://pr${pr}-${sha9}.${host}`;
+          const assert404 = h.verifyResponse(404);
 
-        h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/bar.js`).
+          h.createDummyBuild(pr, sha9, false, false, true);
-          then(h.verifyResponse(200, bodyRegex)).
+          expect(h.buildExists(pr, sha9, false, true)).toBe(true);
-          then(done);
-      });
 
+          Promise.all([
+            h.runCmd(`curl -iL ${origin}/index.html`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/`).then(assert404),
+            h.runCmd(`curl -iL ${origin}`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo/bar.js`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo/`).then(assert404),
+            h.runCmd(`curl -iL ${origin}/foo`).then(assert404),
+          ]).then(done);
+        });
 
-      it('should respond with 403 for directories', done => {
-        Promise.all([
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/`).then(h.verifyResponse(403)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo`).then(h.verifyResponse(403)),
-        ]).then(done);
-      });
-
-
-      it('should respond with 404 for unknown paths to files', done => {
-        h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/baz.css`).
-          then(h.verifyResponse(404)).
-          then(done);
-      });
-
-
-      it('should rewrite to \'index.html\' for unknown paths that don\'t look like files', done => {
-        const bodyRegex = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
-
-        Promise.all([
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/baz`).then(h.verifyResponse(200, bodyRegex)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}/foo/baz/`).then(h.verifyResponse(200, bodyRegex)),
-        ]).then(done);
-      });
-
-
-      it('should respond with 404 for unknown PRs/SHAs', done => {
-        const otherPr = 54321;
-        const otherSha = '8'.repeat(40);
-
-        Promise.all([
-          h.runCmd(`curl -iL ${scheme}://pr${pr}9-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://pr${otherPr}-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}9.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${otherSha}.${host}`).then(h.verifyResponse(404)),
-        ]).then(done);
-      });
-
-
-      it('should respond with 404 if the subdomain format is wrong', done => {
-        Promise.all([
-          h.runCmd(`curl -iL ${scheme}://xpr${pr}-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://prx${pr}-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://xx${pr}-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://p${pr}-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://r${pr}-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://${pr}-${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}_${sha9}.${host}`).then(h.verifyResponse(404)),
-        ]).then(done);
-      });
-
-
-      it('should reject PRs with leading zeros', done => {
-        h.runCmd(`curl -iL ${scheme}://pr0${pr}-${sha9}.${host}`).
-          then(h.verifyResponse(404)).
-          then(done);
-      });
-
-
-      it('should accept SHAs with leading zeros (but not trim the zeros)', done => {
-        const bodyRegex9 = new RegExp(`^PR: ${pr} | SHA: ${sha9} | File: /index\\.html$`);
-        const bodyRegex0 = new RegExp(`^PR: ${pr} | SHA: ${sha0} | File: /index\\.html$`);
-
-        Promise.all([
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-0${sha9}.${host}`).then(h.verifyResponse(404)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha9}.${host}`).then(h.verifyResponse(200, bodyRegex9)),
-          h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha0}.${host}`).then(h.verifyResponse(200, bodyRegex0)),
-        ]).then(done);
       });
 
     });
@@ -246,7 +319,7 @@ describe(`nginx`, () => {
 
     describe(`${host}/*`, () => {
 
-      it('should respond with 404 for unkown URLs (even if the resource exists)', done => {
+      it('should respond with 404 for unknown URLs (even if the resource exists)', done => {
         ['index.html', 'foo.js', 'foo/index.html'].forEach(relFilePath => {
           const absFilePath = path.join(h.buildsDir, relFilePath);
           h.writeFile(absFilePath, {content: `File: /${relFilePath}`});

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/server-integration.e2e.ts

@@ -12,73 +12,189 @@ h.runForAllSupportedSchemes((scheme, port) => describe(`integration (on ${scheme
   const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz');
 
   const getFile = (pr: string, sha: string, file: string) =>
-    h.runCmd(`curl -iL ${scheme}://pr${pr}-${sha}.${host}/${file}`);
+    h.runCmd(`curl -iL ${scheme}://pr${pr}-${h.getShordSha(sha)}.${host}/${file}`);
-  const uploadBuild = (pr: string, sha: string, archive: string) => {
+  const uploadBuild = (pr: string, sha: string, archive: string, authHeader = 'Token FOO') => {
-    const curlPost = 'curl -iLX POST --header "Authorization: Token FOO"';
+    // Using `FAKE_VERIFICATION_ERROR` or `FAKE_VERIFIED_NOT_TRUSTED` as `authHeader`,
+    // we can fake the response of the overwritten `BuildVerifier.verify()` method.
+    // (See 'lib/upload-server/index-test.ts'.)
+    const curlPost = `curl -iLX POST --header "Authorization: ${authHeader}"`;
     return h.runCmd(`${curlPost} --data-binary "@${archive}" ${scheme}://${host}/create-build/${pr}/${sha}`);
   };
 
   beforeEach(() => jasmine.DEFAULT_TIMEOUT_INTERVAL = 10000);
   afterEach(() => {
     h.deletePrDir(pr9);
+    h.deletePrDir(pr9, false);
     h.cleanUp();
   });
 
 
-  it('should be able to upload and serve a build for a new PR', done => {
+  describe('for a new PR', () => {
-    const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
-    const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
-    const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
 
-    h.createDummyArchive(pr9, sha9, archivePath);
+    it('should be able to upload and serve a public build', done => {
+      const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath).
+        then(() => Promise.all([
+          getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)),
+          getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)),
+        ])).
+        then(done);
+    });
+
+
+    it('should be able to upload but not serve a hidden build', done => {
+      const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFIED_NOT_TRUSTED').
+        then(() => Promise.all([
+          getFile(pr9, sha9, 'index.html').then(h.verifyResponse(404)),
+          getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(404)),
+        ])).
+        then(() => {
+          expect(h.buildExists(pr9, sha9)).toBe(false);
+          expect(h.buildExists(pr9, sha9, false)).toBe(true);
+          expect(h.readBuildFile(pr9, sha9, 'index.html', false)).toMatch(idxContentRegex9);
+          expect(h.readBuildFile(pr9, sha9, 'foo/bar.js', false)).toMatch(barContentRegex9);
+        }).
+        then(done);
+    });
+
+
+    it('should reject an upload if verification fails', done => {
+      const errorRegex9 = new RegExp(`Error while verifying upload for PR ${pr9}: Test`);
+
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFICATION_ERROR').
+        then(h.verifyResponse(403, errorRegex9)).
+        then(() => {
+          expect(h.buildExists(pr9)).toBe(false);
+          expect(h.buildExists(pr9, '', false)).toBe(false);
+        }).
+        then(done);
+    });
 
-    uploadBuild(pr9, sha9, archivePath).
-      then(() => Promise.all([
-        getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)),
-        getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)),
-      ])).
-      then(done);
   });
 
 
-  it('should be able to upload and serve a build for an existing PR', done => {
+  describe('for an existing PR', () => {
-    const regexPrefix0 = `^PR: ${pr9} \\| SHA: ${sha0} \\| File:`;
-    const idxContentRegex0 = new RegExp(`${regexPrefix0} \\/index\\.html$`);
-    const barContentRegex0 = new RegExp(`${regexPrefix0} \\/foo\\/bar\\.js$`);
 
-    const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
+    it('should be able to upload and serve a public build', done => {
-    const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const regexPrefix0 = `^PR: ${pr9} \\| SHA: ${sha0} \\| File:`;
-    const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+      const idxContentRegex0 = new RegExp(`${regexPrefix0} \\/index\\.html$`);
+      const barContentRegex0 = new RegExp(`${regexPrefix0} \\/foo\\/bar\\.js$`);
 
-    h.createDummyBuild(pr9, sha0);
+      const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
-    h.createDummyArchive(pr9, sha9, archivePath);
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
 
-    uploadBuild(pr9, sha9, archivePath).
+      h.createDummyBuild(pr9, sha0);
-      then(() => Promise.all([
+      h.createDummyArchive(pr9, sha9, archivePath);
-        getFile(pr9, sha0, 'index.html').then(h.verifyResponse(200, idxContentRegex0)),
+
-        getFile(pr9, sha0, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex0)),
+      uploadBuild(pr9, sha9, archivePath).
-        getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)),
+        then(() => Promise.all([
-        getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)),
+          getFile(pr9, sha0, 'index.html').then(h.verifyResponse(200, idxContentRegex0)),
-      ])).
+          getFile(pr9, sha0, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex0)),
-      then(done);
+          getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)),
-  });
+          getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)),
+        ])).
+        then(done);
+    });
 
 
-  it('should not be able to overwrite a build', done => {
+    it('should be able to upload but not serve a hidden build', done => {
-    const regexPrefix9 = `^PR: ${pr9} \\| SHA: ${sha9} \\| File:`;
+      const regexPrefix0 = `^PR: ${pr9} \\| SHA: ${sha0} \\| File:`;
-    const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const idxContentRegex0 = new RegExp(`${regexPrefix0} \\/index\\.html$`);
-    const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+      const barContentRegex0 = new RegExp(`${regexPrefix0} \\/foo\\/bar\\.js$`);
 
-    h.createDummyBuild(pr9, sha9);
+      const regexPrefix9 = `^PR: uploaded\\/${pr9} \\| SHA: ${sha9} \\| File:`;
-    h.createDummyArchive(pr9, sha9, archivePath);
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyBuild(pr9, sha0, false);
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFIED_NOT_TRUSTED').
+        then(() => Promise.all([
+          getFile(pr9, sha0, 'index.html').then(h.verifyResponse(404)),
+          getFile(pr9, sha0, 'foo/bar.js').then(h.verifyResponse(404)),
+          getFile(pr9, sha9, 'index.html').then(h.verifyResponse(404)),
+          getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(404)),
+        ])).
+        then(() => {
+          expect(h.buildExists(pr9, sha9)).toBe(false);
+          expect(h.buildExists(pr9, sha9, false)).toBe(true);
+          expect(h.readBuildFile(pr9, sha0, 'index.html', false)).toMatch(idxContentRegex0);
+          expect(h.readBuildFile(pr9, sha0, 'foo/bar.js', false)).toMatch(barContentRegex0);
+          expect(h.readBuildFile(pr9, sha9, 'index.html', false)).toMatch(idxContentRegex9);
+          expect(h.readBuildFile(pr9, sha9, 'foo/bar.js', false)).toMatch(barContentRegex9);
+        }).
+        then(done);
+    });
+
+
+    it('should reject an upload if verification fails', done => {
+      const errorRegex9 = new RegExp(`Error while verifying upload for PR ${pr9}: Test`);
+
+      h.createDummyBuild(pr9, sha0);
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFICATION_ERROR').
+        then(h.verifyResponse(403, errorRegex9)).
+        then(() => {
+          expect(h.buildExists(pr9)).toBe(true);
+          expect(h.buildExists(pr9, sha0)).toBe(true);
+          expect(h.buildExists(pr9, sha9)).toBe(false);
+        }).
+        then(done);
+
+    });
+
+
+    it('should not be able to overwrite an existing public build', done => {
+      const regexPrefix9 = `^PR: ${pr9} \\| SHA: ${sha9} \\| File:`;
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyBuild(pr9, sha9);
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath).
+        then(h.verifyResponse(409)).
+        then(() => Promise.all([
+          getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)),
+          getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)),
+        ])).
+        then(done);
+    });
+
+
+    it('should not be able to overwrite an existing hidden build', done => {
+      const regexPrefix9 = `^PR: ${pr9} \\| SHA: ${sha9} \\| File:`;
+      const idxContentRegex9 = new RegExp(`${regexPrefix9} \\/index\\.html$`);
+      const barContentRegex9 = new RegExp(`${regexPrefix9} \\/foo\\/bar\\.js$`);
+
+      h.createDummyBuild(pr9, sha9, false);
+      h.createDummyArchive(pr9, sha9, archivePath);
+
+      uploadBuild(pr9, sha9, archivePath, 'FAKE_VERIFIED_NOT_TRUSTED').
+        then(h.verifyResponse(409)).
+        then(() => {
+          expect(h.readBuildFile(pr9, sha9, 'index.html', false)).toMatch(idxContentRegex9);
+          expect(h.readBuildFile(pr9, sha9, 'foo/bar.js', false)).toMatch(barContentRegex9);
+        }).
+        then(done);
+    });
 
-    uploadBuild(pr9, sha9, archivePath).
-      then(h.verifyResponse(409)).
-      then(() => Promise.all([
-        getFile(pr9, sha9, 'index.html').then(h.verifyResponse(200, idxContentRegex9)),
-        getFile(pr9, sha9, 'foo/bar.js').then(h.verifyResponse(200, barContentRegex9)),
-      ])).
-      then(done);
   });
 
 }));

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/upload-server.e2e.ts

@@ -19,7 +19,8 @@ describe('upload-server (on HTTP)', () => {
   describe(`${host}/create-build/<pr>/<sha>`, () => {
     const authorizationHeader = `--header "Authorization: Token FOO"`;
     const xFileHeader = `--header "X-File: ${h.buildsDir}/snapshot.tar.gz"`;
-    const curl = `curl -iL ${authorizationHeader} ${xFileHeader}`;
+    const defaultHeaders = `${authorizationHeader} ${xFileHeader}`;
+    const curl = (url: string, headers = defaultHeaders) => `curl -iL ${headers} ${url}`;
 
 
     it('should disallow non-GET requests', done => {
@@ -42,8 +43,8 @@ describe('upload-server (on HTTP)', () => {
       const bodyRegex = /^Missing or empty 'AUTHORIZATION' header/;
 
       Promise.all([
-        h.runCmd(`curl -iL ${headers1} ${url}`).then(h.verifyResponse(401, bodyRegex)),
+        h.runCmd(curl(url, headers1)).then(h.verifyResponse(401, bodyRegex)),
-        h.runCmd(`curl -iL  ${headers2} ${url}`).then(h.verifyResponse(401, bodyRegex)),
+        h.runCmd(curl(url, headers2)).then(h.verifyResponse(401, bodyRegex)),
       ]).then(done);
     });
 
@@ -55,14 +56,25 @@ describe('upload-server (on HTTP)', () => {
       const bodyRegex = /^Missing or empty 'X-FILE' header/;
 
       Promise.all([
-        h.runCmd(`curl -iL ${headers1} ${url}`).then(h.verifyResponse(400, bodyRegex)),
+        h.runCmd(curl(url, headers1)).then(h.verifyResponse(400, bodyRegex)),
-        h.runCmd(`curl -iL ${headers2} ${url}`).then(h.verifyResponse(400, bodyRegex)),
+        h.runCmd(curl(url, headers2)).then(h.verifyResponse(400, bodyRegex)),
       ]).then(done);
     });
 
 
+    it('should reject requests for which the PR verification fails', done => {
+      const headers = `--header "Authorization: FAKE_VERIFICATION_ERROR" ${xFileHeader}`;
+      const url = `http://${host}/create-build/${pr}/${sha9}`;
+      const bodyRegex = new RegExp(`Error while verifying upload for PR ${pr}: Test`);
+
+      h.runCmd(curl(url, headers)).
+        then(h.verifyResponse(403, bodyRegex)).
+        then(done);
+    });
+
+
     it('should respond with 404 for unknown paths', done => {
-      const cmdPrefix = `${curl} http://${host}`;
+      const cmdPrefix = curl(`http://${host}`);
 
       Promise.all([
         h.runCmd(`${cmdPrefix}/foo/create-build/${pr}/${sha9}`).then(h.verifyResponse(404)),
@@ -78,7 +90,7 @@ describe('upload-server (on HTTP)', () => {
 
 
     it('should reject PRs with leading zeros', done => {
-      h.runCmd(`${curl} http://${host}/create-build/0${pr}/${sha9}`).
+      h.runCmd(curl(`http://${host}/create-build/0${pr}/${sha9}`)).
         then(h.verifyResponse(404)).
         then(done);
     });
@@ -86,129 +98,253 @@ describe('upload-server (on HTTP)', () => {
 
     it('should accept SHAs with leading zeros (but not trim the zeros)', done => {
       Promise.all([
-        h.runCmd(`${curl} http://${host}/create-build/${pr}/0${sha9}`).then(h.verifyResponse(404)),
+        h.runCmd(curl(`http://${host}/create-build/${pr}/0${sha9}`)).then(h.verifyResponse(404)),
-        h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).then(h.verifyResponse(500)),
+        h.runCmd(curl(`http://${host}/create-build/${pr}/${sha9}`)).then(h.verifyResponse(500)),
-        h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha0}`).then(h.verifyResponse(500)),
+        h.runCmd(curl(`http://${host}/create-build/${pr}/${sha0}`)).then(h.verifyResponse(500)),
       ]).then(done);
     });
 
 
-    it('should not overwrite existing builds', done => {
+    [true, false].forEach(isPublic => describe(`(for ${isPublic ? 'public' : 'hidden'} builds)`, () => {
-      h.createDummyBuild(pr, sha9);
+      const authorizationHeader2 = isPublic ?
-      expect(h.readBuildFile(pr, sha9, 'index.html')).toContain('index.html');
+        authorizationHeader : '--header "Authorization: FAKE_VERIFIED_NOT_TRUSTED"';
-
+      const cmdPrefix = curl('', `${authorizationHeader2} ${xFileHeader}`);
-      h.writeBuildFile(pr, sha9, 'index.html', 'My content');
-      expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content');
-
-      h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).
-        then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
-        then(() => expect(h.readBuildFile(pr, sha9, 'index.html')).toBe('My content')).
-        then(done);
-    });
 
 
-    it('should delete the PR directory on error (for new PR)', done => {
+      it('should not overwrite existing builds', done => {
-      const prDir = path.join(h.buildsDir, pr);
+        h.createDummyBuild(pr, sha9, isPublic);
+        expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain('index.html');
 
-      h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).
+        h.writeBuildFile(pr, sha9, 'index.html', 'My content', isPublic);
-        then(h.verifyResponse(500)).
+        expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content');
-        then(() => expect(fs.existsSync(prDir)).toBe(false)).
-        then(done);
-    });
 
-
+        h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
-    it('should only delete the SHA directory on error (for existing PR)', done => {
+          then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
-      const prDir = path.join(h.buildsDir, pr);
+          then(() => expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content')).
-      const shaDir = path.join(prDir, sha9);
+          then(done);
-
-      h.createDummyBuild(pr, sha0);
-
-      h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`).
-        then(h.verifyResponse(500)).
-        then(() => {
-          expect(fs.existsSync(shaDir)).toBe(false);
-          expect(fs.existsSync(prDir)).toBe(true);
-        }).
-        then(done);
-    });
-
-
-    describe('on successful upload', () => {
-      const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz');
-      let uploadPromise: Promise<CmdResult>;
-
-      beforeEach(() => {
-        h.createDummyArchive(pr, sha9, archivePath);
-        uploadPromise = h.runCmd(`${curl} http://${host}/create-build/${pr}/${sha9}`);
-      });
-      afterEach(() => h.deletePrDir(pr));
-
-
-      it('should respond with 201', done => {
-        uploadPromise.then(h.verifyResponse(201)).then(done);
       });
 
 
-      it('should extract the contents of the uploaded file', done => {
+      it('should not overwrite existing builds (even if the SHA is different)', done => {
-        uploadPromise.
+        // Since only the first few characters of the SHA are used, it is possible for two different
+        // SHAs to correspond to the same directory. In that case, we don't want the second SHA to
+        // overwrite the first.
+
+        const sha9Almost = sha9.replace(/.$/, '8');
+        expect(sha9Almost).not.toBe(sha9);
+
+        h.createDummyBuild(pr, sha9, isPublic);
+        expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain('index.html');
+
+        h.writeBuildFile(pr, sha9, 'index.html', 'My content', isPublic);
+        expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content');
+
+        h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9Almost}`).
+          then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
+          then(() => expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toBe('My content')).
+          then(done);
+      });
+
+
+      it('should delete the PR directory on error (for new PR)', done => {
+        h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
+          then(h.verifyResponse(500)).
+          then(() => expect(h.buildExists(pr, '', isPublic)).toBe(false)).
+          then(done);
+      });
+
+
+      it('should only delete the SHA directory on error (for existing PR)', done => {
+        h.createDummyBuild(pr, sha0, isPublic);
+
+        h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
+          then(h.verifyResponse(500)).
           then(() => {
-            expect(h.readBuildFile(pr, sha9, 'index.html')).toContain(`uploaded/${pr}`);
+            expect(h.buildExists(pr, sha9, isPublic)).toBe(false);
-            expect(h.readBuildFile(pr, sha9, 'foo/bar.js')).toContain(`uploaded/${pr}`);
+            expect(h.buildExists(pr, '', isPublic)).toBe(true);
           }).
           then(done);
       });
 
 
-      it(`should create files/directories owned by '${h.wwwUser}'`, done => {
+      describe('on successful upload', () => {
-        const shaDir = path.join(h.buildsDir, pr, sha9);
+        const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz');
-        const idxPath = path.join(shaDir, 'index.html');
+        const statusCode = isPublic ? 201 : 202;
-        const barPath = path.join(shaDir, 'foo', 'bar.js');
+        let uploadPromise: Promise<CmdResult>;
+
+        beforeEach(() => {
+          h.createDummyArchive(pr, sha9, archivePath);
+          uploadPromise = h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`);
+        });
+        afterEach(() => h.deletePrDir(pr, isPublic));
+
+
+        it(`should respond with ${statusCode}`, done => {
+          uploadPromise.then(h.verifyResponse(statusCode)).then(done);
+        });
+
+
+        it('should extract the contents of the uploaded file', done => {
+          uploadPromise.
+            then(() => {
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain(`uploaded/${pr}`);
+              expect(h.readBuildFile(pr, sha9, 'foo/bar.js', isPublic)).toContain(`uploaded/${pr}`);
+            }).
+            then(done);
+        });
+
+
+        it(`should create files/directories owned by '${h.wwwUser}'`, done => {
+          const prDir = h.getPrDir(pr, isPublic);
+          const shaDir = h.getShaDir(prDir, sha9);
+          const idxPath = path.join(shaDir, 'index.html');
+          const barPath = path.join(shaDir, 'foo', 'bar.js');
+
+          uploadPromise.
+            then(() => Promise.all([
+              h.runCmd(`find ${shaDir}`),
+              h.runCmd(`find ${shaDir} -user ${h.wwwUser}`),
+            ])).
+            then(([{stdout: allFiles}, {stdout: userFiles}]) => {
+              expect(userFiles).toBe(allFiles);
+              expect(userFiles).toContain(shaDir);
+              expect(userFiles).toContain(idxPath);
+              expect(userFiles).toContain(barPath);
+            }).
+            then(done);
+        });
+
+
+        it('should delete the uploaded file', done => {
+          expect(fs.existsSync(archivePath)).toBe(true);
+          uploadPromise.
+            then(() => expect(fs.existsSync(archivePath)).toBe(false)).
+            then(done);
+        });
+
+
+        it('should make the build directory non-writable', done => {
+          const prDir = h.getPrDir(pr, isPublic);
+          const shaDir = h.getShaDir(prDir, sha9);
+          const idxPath = path.join(shaDir, 'index.html');
+          const barPath = path.join(shaDir, 'foo', 'bar.js');
+
+          // See https://github.com/nodejs/node-v0.x-archive/issues/3045#issuecomment-4862588.
+          const isNotWritable = (fileOrDir: string) => {
+            const mode = fs.statSync(fileOrDir).mode;
+            // tslint:disable-next-line: no-bitwise
+            return !(mode & parseInt('222', 8));
+          };
+
+          uploadPromise.
+            then(() => {
+              expect(isNotWritable(shaDir)).toBe(true);
+              expect(isNotWritable(idxPath)).toBe(true);
+              expect(isNotWritable(barPath)).toBe(true);
+            }).
+            then(done);
+        });
+
+
+        it('should ignore a legacy 40-chars long build directory (even if it starts with the same chars)', done => {
+          // It is possible that 40-chars long build directories exist, if they had been deployed
+          // before implementing the shorter build directory names. In that case, we don't want the
+          // second (shorter) name to be considered the same as the old one (even if they originate
+          // from the same SHA).
+
+          h.createDummyBuild(pr, sha9, isPublic, false, true);
+          expect(h.readBuildFile(pr, sha9, 'index.html', isPublic, true)).toContain('index.html');
+
+          h.writeBuildFile(pr, sha9, 'index.html', 'My content', isPublic, true);
+          expect(h.readBuildFile(pr, sha9, 'index.html', isPublic, true)).toBe('My content');
+
+          h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha9}`).
+            then(h.verifyResponse(statusCode)).
+            then(() => {
+              expect(h.buildExists(pr, sha9, isPublic)).toBe(true);
+              expect(h.buildExists(pr, sha9, isPublic, true)).toBe(true);
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain('index.html');
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic, true)).toBe('My content');
+            }).
+            then(done);
+        });
 
-        uploadPromise.
-          then(() => Promise.all([
-            h.runCmd(`find ${shaDir}`),
-            h.runCmd(`find ${shaDir} -user ${h.wwwUser}`),
-          ])).
-          then(([{stdout: allFiles}, {stdout: userFiles}]) => {
-            expect(userFiles).toBe(allFiles);
-            expect(userFiles).toContain(shaDir);
-            expect(userFiles).toContain(idxPath);
-            expect(userFiles).toContain(barPath);
-          }).
-          then(done);
       });
 
 
-      it('should delete the uploaded file', done => {
+      describe('when the PR\'s visibility has changed', () => {
-        expect(fs.existsSync(archivePath)).toBe(true);
+        const archivePath = path.join(h.buildsDir, 'snapshot.tar.gz');
-        uploadPromise.
+        const statusCode = isPublic ? 201 : 202;
-          then(() => expect(fs.existsSync(archivePath)).toBe(false)).
-          then(done);
-      });
 
-
+        const checkPrVisibility = (isPublic2: boolean) => {
-      it('should make the build directory non-writable', done => {
+          expect(h.buildExists(pr, '', isPublic2)).toBe(true);
-        const shaDir = path.join(h.buildsDir, pr, sha9);
+          expect(h.buildExists(pr, '', !isPublic2)).toBe(false);
-        const idxPath = path.join(shaDir, 'index.html');
+          expect(h.buildExists(pr, sha0, isPublic2)).toBe(true);
-        const barPath = path.join(shaDir, 'foo', 'bar.js');
+          expect(h.buildExists(pr, sha0, !isPublic2)).toBe(false);
-
-        // See https://github.com/nodejs/node-v0.x-archive/issues/3045#issuecomment-4862588.
-        const isNotWritable = (fileOrDir: string) => {
-          const mode = fs.statSync(fileOrDir).mode;
-          // tslint:disable-next-line: no-bitwise
-          return !(mode & parseInt('222', 8));
         };
+        const uploadBuild = (sha: string) => h.runCmd(`${cmdPrefix} http://${host}/create-build/${pr}/${sha}`);
+
+        beforeEach(() => {
+          h.createDummyBuild(pr, sha0, !isPublic);
+          h.createDummyArchive(pr, sha9, archivePath);
+          checkPrVisibility(!isPublic);
+        });
+        afterEach(() => h.deletePrDir(pr, isPublic));
+
+
+        it('should update the PR\'s visibility', done => {
+          uploadBuild(sha9).
+            then(h.verifyResponse(statusCode)).
+            then(() => {
+              checkPrVisibility(isPublic);
+              expect(h.buildExists(pr, sha9, isPublic)).toBe(true);
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain(`uploaded/${pr}`);
+              expect(h.readBuildFile(pr, sha9, 'index.html', isPublic)).toContain(sha9);
+            }).
+            then(done);
+        });
+
+
+        it('should not overwrite existing builds (but keep the updated visibility)', done => {
+          expect(h.buildExists(pr, sha0, isPublic)).toBe(false);
+
+          uploadBuild(sha0).
+            then(h.verifyResponse(409, /^Request to overwrite existing directory/)).
+            then(() => {
+              checkPrVisibility(isPublic);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).toContain(pr);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).not.toContain(`uploaded/${pr}`);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).toContain(sha0);
+              expect(h.readBuildFile(pr, sha0, 'index.html', isPublic)).not.toContain(sha9);
+            }).
+            then(done);
+        });
+
+
+        it('should reject the request if it fails to update the PR\'s visibility', done => {
+          // One way to cause an error is to have both a public and a hidden directory for the same PR.
+          h.createDummyBuild(pr, sha0, isPublic);
+
+          expect(h.buildExists(pr, sha0, isPublic)).toBe(true);
+          expect(h.buildExists(pr, sha0, !isPublic)).toBe(true);
+
+          const errorRegex = new RegExp(`^Request to move '${h.getPrDir(pr, !isPublic)}' ` +
+                                        `to existing directory '${h.getPrDir(pr, isPublic)}'.`);
+
+          uploadBuild(sha9).
+            then(h.verifyResponse(409, errorRegex)).
+            then(() => {
+              expect(h.buildExists(pr, sha0, isPublic)).toBe(true);
+              expect(h.buildExists(pr, sha0, !isPublic)).toBe(true);
+              expect(h.buildExists(pr, sha9, isPublic)).toBe(false);
+              expect(h.buildExists(pr, sha9, !isPublic)).toBe(false);
+            }).
+            then(done);
+        });
 
-        uploadPromise.
-          then(() => {
-            expect(isNotWritable(shaDir)).toBe(true);
-            expect(isNotWritable(idxPath)).toBe(true);
-            expect(isNotWritable(barPath)).toBe(true);
-          }).
-          then(done);
       });
 
-    });
+    }));
 
   });
 

aio/aio-builds-setup/dockerbuild/scripts-js/test/clean-up/build-cleaner.spec.ts

@@ -1,7 +1,9 @@
 // Imports
 import * as fs from 'fs';
+import * as path from 'path';
 import * as shell from 'shelljs';
 import {BuildCleaner} from '../../lib/clean-up/build-cleaner';
+import {HIDDEN_DIR_PREFIX} from '../../lib/common/constants';
 import {GithubPullRequests} from '../../lib/common/github-pull-requests';
 
 // Tests
@@ -114,7 +116,7 @@ describe('BuildCleaner', () => {
 
     it('should resolve with the value returned by \'removeUnnecessaryBuilds()\'', done => {
       promise.then(result => {
-        expect(result).toBe('Test');
+        expect(result as any).toBe('Test');
         done();
       });
 
@@ -170,6 +172,16 @@ describe('BuildCleaner', () => {
     });
 
 
+    it('should remove `HIDDEN_DIR_PREFIX` from the filenames', done => {
+      promise.then(result => {
+        expect(result).toEqual([12, 34, 56]);
+        done();
+      });
+
+      readdirCb(null, [`${HIDDEN_DIR_PREFIX}12`, '34', `${HIDDEN_DIR_PREFIX}56`]);
+    });
+
+
     it('should ignore files with non-numeric (or zero) names', done => {
       promise.then(result => {
         expect(result).toEqual([12, 34, 56]);
@@ -230,10 +242,22 @@ describe('BuildCleaner', () => {
   describe('removeDir()', () => {
     let shellChmodSpy: jasmine.Spy;
     let shellRmSpy: jasmine.Spy;
+    let shellTestSpy: jasmine.Spy;
 
     beforeEach(() => {
       shellChmodSpy = spyOn(shell, 'chmod');
       shellRmSpy = spyOn(shell, 'rm');
+      shellTestSpy = spyOn(shell, 'test').and.returnValue(true);
+    });
+
+
+    it('should test if the directory exists (and return if is does not)', () => {
+      shellTestSpy.and.returnValue(false);
+      (cleaner as any).removeDir('/foo/bar');
+
+      expect(shellTestSpy).toHaveBeenCalledWith('-d', '/foo/bar');
+      expect(shellChmodSpy).not.toHaveBeenCalled();
+      expect(shellRmSpy).not.toHaveBeenCalled();
     });
 
 
@@ -287,17 +311,28 @@ describe('BuildCleaner', () => {
     it('should construct full paths to directories (by prepending \'buildsDir\')', () => {
       (cleaner as any).removeUnnecessaryBuilds([1, 2, 3], []);
 
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/1');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/1'));
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/2');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/2'));
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/3');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/3'));
+    });
+
+
+    it('should try removing hidden directories as well', () => {
+      (cleaner as any).removeUnnecessaryBuilds([1, 2, 3], []);
+
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}1`));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}2`));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}3`));
     });
 
 
     it('should remove the builds that do not correspond to open PRs', () => {
       (cleaner as any).removeUnnecessaryBuilds([1, 2, 3, 4], [2, 4]);
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledTimes(2);
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledTimes(4);
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/1');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/1'));
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/3');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/3'));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}1`));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}3`));
       cleanerRemoveDirSpy.calls.reset();
 
       (cleaner as any).removeUnnecessaryBuilds([1, 2, 3, 4], [1, 2, 3, 4]);
@@ -305,11 +340,15 @@ describe('BuildCleaner', () => {
       cleanerRemoveDirSpy.calls.reset();
 
       (cleaner as any).removeUnnecessaryBuilds([1, 2, 3, 4], []);
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledTimes(4);
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledTimes(8);
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/1');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/1'));
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/2');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/2'));
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/3');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/3'));
-      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith('/foo/bar/4');
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize('/foo/bar/4'));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}1`));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}2`));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}3`));
+      expect(cleanerRemoveDirSpy).toHaveBeenCalledWith(path.normalize(`/foo/bar/${HIDDEN_DIR_PREFIX}4`));
       cleanerRemoveDirSpy.calls.reset();
     });
 

aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-api.spec.ts

@@ -292,7 +292,7 @@ describe('GithubApi', () => {
 
 
     describe('onResponse', () => {
-      let promise: Promise<void>;
+      let promise: Promise<Object>;
       let respond: (statusCode: number) => IncomingMessage;
 
       beforeEach(() => {

aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts

@@ -66,7 +66,7 @@ describe('GithubPullRequests', () => {
 
     it('should resolve with the returned response', done => {
       prs.addComment(42, 'body').then(data => {
-        expect(data).toEqual('Test');
+        expect(data as any).toBe('Test');
         done();
       });
 
@@ -76,6 +76,30 @@ describe('GithubPullRequests', () => {
   });
 
 
+  describe('fetch()', () => {
+    let prs: GithubPullRequests;
+    let prsGetSpy: jasmine.Spy;
+
+    beforeEach(() => {
+      prs = new GithubPullRequests('12345', 'foo/bar');
+      prsGetSpy = spyOn(prs as any, 'get');
+    });
+
+
+    it('should call \'get()\' with the correct pathname', () => {
+      prs.fetch(42);
+      expect(prsGetSpy).toHaveBeenCalledWith('/repos/foo/bar/issues/42');
+    });
+
+
+    it('should forward the value returned by \'get()\'', () => {
+      prsGetSpy.and.returnValue('Test');
+      expect(prs.fetch(42) as any).toBe('Test');
+    });
+
+  });
+
+
   describe('fetchAll()', () => {
     let prs: GithubPullRequests;
     let prsGetPaginatedSpy: jasmine.Spy;
@@ -109,7 +133,7 @@ describe('GithubPullRequests', () => {
 
     it('should forward the value returned by \'getPaginated()\'', () => {
       prsGetPaginatedSpy.and.returnValue('Test');
-      expect(prs.fetchAll()).toBe('Test');
+      expect(prs.fetchAll() as any).toBe('Test');
     });
 
   });

aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-teams.spec.ts

@@ -38,7 +38,7 @@ describe('GithubTeams', () => {
 
     it('should forward the value returned by \'getPaginated()\'', () => {
       teamsGetPaginatedSpy.and.returnValue('Test');
-      expect(teams.fetchAll()).toBe('Test');
+      expect(teams.fetchAll() as any).toBe('Test');
     });
 
   });
@@ -50,12 +50,16 @@ describe('GithubTeams', () => {
 
     beforeEach(() => {
       teams = new GithubTeams('12345', 'foo');
-      teamsGetSpy = spyOn(teams, 'get');
+      teamsGetSpy = spyOn(teams, 'get').and.returnValue(Promise.resolve(null));
     });
 
 
-    it('should return a promise', () => {
+    it('should return a promise', done => {
-      expect(teams.isMemberById('user', [1])).toEqual(jasmine.any(Promise));
+      const promise = teams.isMemberById('user', [1]);
+      promise.then(done);   // Do not complete the test (and release the spies) synchronously
+                            // to avoid running the actual `get()`.
+
+      expect(promise).toEqual(jasmine.any(Promise));
     });
 
 
@@ -69,7 +73,6 @@ describe('GithubTeams', () => {
 
 
     it('should call \'get()\' with the correct pathname', done => {
-      teamsGetSpy.and.returnValue(Promise.resolve(null));
       teams.isMemberById('user', [1]).then(() => {
         expect(teamsGetSpy).toHaveBeenCalledWith('/teams/1/memberships/user');
         done();

aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/build-creator.spec.ts

@@ -2,9 +2,11 @@
 import * as cp from 'child_process';
 import {EventEmitter} from 'events';
 import * as fs from 'fs';
+import * as path from 'path';
 import * as shell from 'shelljs';
+import {SHORT_SHA_LEN} from '../../lib/common/constants';
 import {BuildCreator} from '../../lib/upload-server/build-creator';
-import {CreatedBuildEvent} from '../../lib/upload-server/build-events';
+import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/upload-server/build-events';
 import {UploadError} from '../../lib/upload-server/upload-error';
 import {expectToBeUploadError} from './helpers';
 
@@ -12,10 +14,13 @@ import {expectToBeUploadError} from './helpers';
 describe('BuildCreator', () => {
   const pr = '9';
   const sha = '9'.repeat(40);
+  const shortSha = sha.substr(0, SHORT_SHA_LEN);
   const archive = 'snapshot.tar.gz';
   const buildsDir = 'builds/dir';
-  const prDir = `${buildsDir}/${pr}`;
+  const hiddenPrDir = path.join(buildsDir, `hidden--${pr}`);
-  const shaDir = `${prDir}/${sha}`;
+  const publicPrDir = path.join(buildsDir, pr);
+  const hiddenShaDir = path.join(hiddenPrDir, shortSha);
+  const publicShaDir = path.join(publicPrDir, shortSha);
   let bc: BuildCreator;
 
   beforeEach(() => bc = new BuildCreator(buildsDir));
@@ -38,7 +43,160 @@ describe('BuildCreator', () => {
   });
 
 
+  describe('changePrVisibility()', () => {
+    let bcEmitSpy: jasmine.Spy;
+    let bcExistsSpy: jasmine.Spy;
+    let bcListShasByDate: jasmine.Spy;
+    let shellMvSpy: jasmine.Spy;
+
+    beforeEach(() => {
+      bcEmitSpy = spyOn(bc, 'emit');
+      bcExistsSpy = spyOn(bc as any, 'exists');
+      bcListShasByDate = spyOn(bc as any, 'listShasByDate');
+      shellMvSpy = spyOn(shell, 'mv');
+
+      bcExistsSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
+      bcListShasByDate.and.returnValue([]);
+    });
+
+
+    it('should return a promise', done => {
+      const promise = bc.changePrVisibility(pr, true);
+      promise.then(done);   // Do not complete the test (and release the spies) synchronously
+                            // to avoid running the actual `extractArchive()`.
+
+      expect(promise).toEqual(jasmine.any(Promise));
+    });
+
+
+    [true, false].forEach(makePublic => {
+      const oldPrDir = makePublic ? hiddenPrDir : publicPrDir;
+      const newPrDir = makePublic ? publicPrDir : hiddenPrDir;
+
+
+      it('should rename the directory', done => {
+        bc.changePrVisibility(pr, makePublic).
+          then(() => expect(shellMvSpy).toHaveBeenCalledWith(oldPrDir, newPrDir)).
+          then(done);
+      });
+
+
+      it('should emit a ChangedPrVisibilityEvent on success', done => {
+        let emitted = false;
+
+        bcEmitSpy.and.callFake((type: string, evt: ChangedPrVisibilityEvent) => {
+          expect(type).toBe(ChangedPrVisibilityEvent.type);
+          expect(evt).toEqual(jasmine.any(ChangedPrVisibilityEvent));
+          expect(evt.pr).toBe(+pr);
+          expect(evt.shas).toEqual(jasmine.any(Array));
+          expect(evt.isPublic).toBe(makePublic);
+
+          emitted = true;
+        });
+
+        bc.changePrVisibility(pr, makePublic).
+          then(() => expect(emitted).toBe(true)).
+          then(done);
+      });
+
+
+      it('should include all shas in the emitted event', done => {
+        const shas = ['foo', 'bar', 'baz'];
+        let emitted = false;
+
+        bcListShasByDate.and.returnValue(Promise.resolve(shas));
+        bcEmitSpy.and.callFake((type: string, evt: ChangedPrVisibilityEvent) => {
+          expect(bcListShasByDate).toHaveBeenCalledWith(newPrDir);
+
+          expect(type).toBe(ChangedPrVisibilityEvent.type);
+          expect(evt).toEqual(jasmine.any(ChangedPrVisibilityEvent));
+          expect(evt.pr).toBe(+pr);
+          expect(evt.shas).toBe(shas);
+          expect(evt.isPublic).toBe(makePublic);
+
+          emitted = true;
+        });
+
+        bc.changePrVisibility(pr, makePublic).
+          then(() => expect(emitted).toBe(true)).
+          then(done);
+      });
+
+
+      describe('on error', () => {
+
+        it('should abort and skip further operations if the old directory does not exist', done => {
+          bcExistsSpy.and.callFake((dir: string) => dir !== oldPrDir);
+          bc.changePrVisibility(pr, makePublic).catch(err => {
+            expectToBeUploadError(err, 404, `Request to move non-existing directory '${oldPrDir}' to '${newPrDir}'.`);
+            expect(shellMvSpy).not.toHaveBeenCalled();
+            expect(bcListShasByDate).not.toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
+        });
+
+
+        it('should abort and skip further operations if the new directory does already exist', done => {
+          bcExistsSpy.and.returnValue(true);
+          bc.changePrVisibility(pr, makePublic).catch(err => {
+            expectToBeUploadError(err, 409, `Request to move '${oldPrDir}' to existing directory '${newPrDir}'.`);
+            expect(shellMvSpy).not.toHaveBeenCalled();
+            expect(bcListShasByDate).not.toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
+        });
+
+
+        it('should abort and skip further operations if it fails to rename the directory', done => {
+          shellMvSpy.and.throwError('');
+          bc.changePrVisibility(pr, makePublic).catch(() => {
+            expect(shellMvSpy).toHaveBeenCalled();
+            expect(bcListShasByDate).not.toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
+        });
+
+
+        it('should abort and skip further operations if it fails to list the SHAs', done => {
+          bcListShasByDate.and.throwError('');
+          bc.changePrVisibility(pr, makePublic).catch(() => {
+            expect(shellMvSpy).toHaveBeenCalled();
+            expect(bcListShasByDate).toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
+        });
+
+
+        it('should reject with an UploadError', done => {
+          shellMvSpy.and.callFake(() => { throw 'Test'; });
+          bc.changePrVisibility(pr, makePublic).catch(err => {
+            expectToBeUploadError(err, 500, `Error while making PR ${pr} ${makePublic ? 'public' : 'hidden'}.\nTest`);
+            done();
+          });
+        });
+
+
+        it('should pass UploadError instances unmodified', done => {
+          shellMvSpy.and.callFake(() => { throw new UploadError(543, 'Test'); });
+          bc.changePrVisibility(pr, makePublic).catch(err => {
+            expectToBeUploadError(err, 543, 'Test');
+            done();
+          });
+        });
+
+      });
+
+    });
+
+  });
+
+
   describe('create()', () => {
+    let bcChangePrVisibilitySpy: jasmine.Spy;
     let bcEmitSpy: jasmine.Spy;
     let bcExistsSpy: jasmine.Spy;
     let bcExtractArchiveSpy: jasmine.Spy;
@@ -46,6 +204,7 @@ describe('BuildCreator', () => {
     let shellRmSpy: jasmine.Spy;
 
     beforeEach(() => {
+      bcChangePrVisibilitySpy = spyOn(bc, 'changePrVisibility');
       bcEmitSpy = spyOn(bc, 'emit');
       bcExistsSpy = spyOn(bc as any, 'exists');
       bcExtractArchiveSpy = spyOn(bc as any, 'extractArchive');
@@ -54,115 +213,192 @@ describe('BuildCreator', () => {
     });
 
 
-    it('should return a promise', done => {
+    [true, false].forEach(isPublic => {
-      const promise = bc.create(pr, sha, archive);
+      const otherVisPrDir = isPublic ? hiddenPrDir : publicPrDir;
-      promise.then(done);   // Do not complete the test (and release the spies) synchronously
+      const prDir = isPublic ? publicPrDir : hiddenPrDir;
-                            // to avoid running the actual `extractArchive()`.
+      const shaDir = isPublic ? publicShaDir : hiddenShaDir;
-
-      expect(promise).toEqual(jasmine.any(Promise));
-    });
 
 
-    it('should throw if the build does already exist', done => {
+      it('should return a promise', done => {
-      bcExistsSpy.and.returnValue(true);
+        const promise = bc.create(pr, sha, archive, isPublic);
-      bc.create(pr, sha, archive).catch(err => {
+        promise.then(done);   // Do not complete the test (and release the spies) synchronously
-        expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`);
+                              // to avoid running the actual `extractArchive()`.
-        done();
-      });
-    });
 
-
+        expect(promise).toEqual(jasmine.any(Promise));
-    it('should create the build directory (and any parent directories)', done => {
-      bc.create(pr, sha, archive).
-        then(() => expect(shellMkdirSpy).toHaveBeenCalledWith('-p', shaDir)).
-        then(done);
-    });
-
-
-    it('should extract the archive contents into the build directory', done => {
-      bc.create(pr, sha, archive).
-        then(() => expect(bcExtractArchiveSpy).toHaveBeenCalledWith(archive, shaDir)).
-        then(done);
-    });
-
-
-    it('should emit a CreatedBuildEvent on success', done => {
-      let emitted = false;
-
-      bcEmitSpy.and.callFake((type: string, evt: CreatedBuildEvent) => {
-        expect(type).toBe(CreatedBuildEvent.type);
-        expect(evt).toEqual(jasmine.any(CreatedBuildEvent));
-        expect(evt.pr).toBe(+pr);
-        expect(evt.sha).toBe(sha);
-
-        emitted = true;
       });
 
-      bc.create(pr, sha, archive).
+
-        then(() => expect(emitted).toBe(true)).
+      it('should not update the PR\'s visibility first if not necessary', done => {
-        then(done);
+        bc.create(pr, sha, archive, isPublic).
-    });
+          then(() => expect(bcChangePrVisibilitySpy).not.toHaveBeenCalled()).
+          then(done);
+      });
 
 
-    describe('on error', () => {
+      it('should update the PR\'s visibility first if necessary', done => {
+        bcChangePrVisibilitySpy.and.callFake(() => expect(shellMkdirSpy).not.toHaveBeenCalled());
+        bcExistsSpy.and.callFake((dir: string) => dir === otherVisPrDir);
 
-      it('should abort and skip further operations if it fails to create the directories', done => {
+        bc.create(pr, sha, archive, isPublic).
-        shellMkdirSpy.and.throwError('');
+          then(() => {
-        bc.create(pr, sha, archive).catch(() => {
+            expect(bcChangePrVisibilitySpy).toHaveBeenCalledWith(pr, isPublic);
-          expect(shellMkdirSpy).toHaveBeenCalled();
+            expect(shellMkdirSpy).toHaveBeenCalled();
-          expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
+          }).
-          expect(bcEmitSpy).not.toHaveBeenCalled();
+          then(done);
-          done();
+      });
+
+
+      it('should create the build directory (and any parent directories)', done => {
+        bc.create(pr, sha, archive, isPublic).
+          then(() => expect(shellMkdirSpy).toHaveBeenCalledWith('-p', shaDir)).
+          then(done);
+      });
+
+
+      it('should extract the archive contents into the build directory', done => {
+        bc.create(pr, sha, archive, isPublic).
+          then(() => expect(bcExtractArchiveSpy).toHaveBeenCalledWith(archive, shaDir)).
+          then(done);
+      });
+
+
+      it('should emit a CreatedBuildEvent on success', done => {
+        let emitted = false;
+
+        bcEmitSpy.and.callFake((type: string, evt: CreatedBuildEvent) => {
+          expect(type).toBe(CreatedBuildEvent.type);
+          expect(evt).toEqual(jasmine.any(CreatedBuildEvent));
+          expect(evt.pr).toBe(+pr);
+          expect(evt.sha).toBe(shortSha);
+          expect(evt.isPublic).toBe(isPublic);
+
+          emitted = true;
         });
+
+        bc.create(pr, sha, archive, isPublic).
+          then(() => expect(emitted).toBe(true)).
+          then(done);
       });
 
 
-      it('should abort and skip further operations if it fails to extract the archive', done => {
+      describe('on error', () => {
-        bcExtractArchiveSpy.and.throwError('');
+        let existsValues: {[dir: string]: boolean};
-        bc.create(pr, sha, archive).catch(() => {
+
-          expect(shellMkdirSpy).toHaveBeenCalled();
+        beforeEach(() => {
-          expect(bcExtractArchiveSpy).toHaveBeenCalled();
+          existsValues = {
-          expect(bcEmitSpy).not.toHaveBeenCalled();
+            [otherVisPrDir]: false,
-          done();
+            [prDir]: false,
+            [shaDir]: false,
+          };
+
+          bcExistsSpy.and.callFake((dir: string) => existsValues[dir]);
         });
-      });
 
 
-      it('should delete the PR directory (for new PR)', done => {
+        it('should abort and skip further operations if changing the PR\'s visibility fails', done => {
-        bcExtractArchiveSpy.and.throwError('');
+          const mockError = new UploadError(543, 'Test');
-        bc.create(pr, sha, archive).catch(() => {
+
-          expect(shellRmSpy).toHaveBeenCalledWith('-rf', prDir);
+          existsValues[otherVisPrDir] = true;
-          done();
+          bcChangePrVisibilitySpy.and.returnValue(Promise.reject(mockError));
+
+          bc.create(pr, sha, archive, isPublic).catch(err => {
+            expect(err).toBe(mockError);
+
+            expect(bcExistsSpy).toHaveBeenCalledTimes(1);
+            expect(shellMkdirSpy).not.toHaveBeenCalled();
+            expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+
+            done();
+          });
         });
-      });
 
 
-      it('should delete the SHA directory (for existing PR)', done => {
+        it('should abort and skip further operations if the build does already exist', done => {
-        bcExistsSpy.and.callFake((path: string) => path !== shaDir);
+          existsValues[shaDir] = true;
-        bcExtractArchiveSpy.and.throwError('');
+          bc.create(pr, sha, archive, isPublic).catch(err => {
-
+            expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`);
-        bc.create(pr, sha, archive).catch(() => {
+            expect(shellMkdirSpy).not.toHaveBeenCalled();
-          expect(shellRmSpy).toHaveBeenCalledWith('-rf', shaDir);
+            expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
-          done();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
         });
-      });
 
 
-      it('should reject with an UploadError', done => {
+        it('should detect existing build directory after visibility change', done => {
-        shellMkdirSpy.and.callFake(() => {throw 'Test'; });
+          existsValues[otherVisPrDir] = true;
-        bc.create(pr, sha, archive).catch(err => {
+          bcChangePrVisibilitySpy.and.callFake(() => existsValues[prDir] = existsValues[shaDir] = true);
-          expectToBeUploadError(err, 500, `Error while uploading to directory: ${shaDir}\nTest`);
+
-          done();
+          bc.create(pr, sha, archive, isPublic).catch(err => {
+            expectToBeUploadError(err, 409, `Request to overwrite existing directory: ${shaDir}`);
+            expect(shellMkdirSpy).not.toHaveBeenCalled();
+            expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
         });
-      });
 
 
-      it('should pass UploadError instances unmodified', done => {
+        it('should abort and skip further operations if it fails to create the directories', done => {
-        shellMkdirSpy.and.callFake(() => { throw new UploadError(543, 'Test'); });
+          shellMkdirSpy.and.throwError('');
-        bc.create(pr, sha, archive).catch(err => {
+          bc.create(pr, sha, archive, isPublic).catch(() => {
-          expectToBeUploadError(err, 543, 'Test');
+            expect(shellMkdirSpy).toHaveBeenCalled();
-          done();
+            expect(bcExtractArchiveSpy).not.toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
         });
+
+
+        it('should abort and skip further operations if it fails to extract the archive', done => {
+          bcExtractArchiveSpy.and.throwError('');
+          bc.create(pr, sha, archive, isPublic).catch(() => {
+            expect(shellMkdirSpy).toHaveBeenCalled();
+            expect(bcExtractArchiveSpy).toHaveBeenCalled();
+            expect(bcEmitSpy).not.toHaveBeenCalled();
+            done();
+          });
+        });
+
+
+        it('should delete the PR directory (for new PR)', done => {
+          bcExtractArchiveSpy.and.throwError('');
+          bc.create(pr, sha, archive, isPublic).catch(() => {
+            expect(shellRmSpy).toHaveBeenCalledWith('-rf', prDir);
+            done();
+          });
+        });
+
+
+        it('should delete the SHA directory (for existing PR)', done => {
+          existsValues[prDir] = true;
+          bcExtractArchiveSpy.and.throwError('');
+
+          bc.create(pr, sha, archive, isPublic).catch(() => {
+            expect(shellRmSpy).toHaveBeenCalledWith('-rf', shaDir);
+            done();
+          });
+        });
+
+
+        it('should reject with an UploadError', done => {
+          shellMkdirSpy.and.callFake(() => { throw 'Test'; });
+          bc.create(pr, sha, archive, isPublic).catch(err => {
+            expectToBeUploadError(err, 500, `Error while uploading to directory: ${shaDir}\nTest`);
+            done();
+          });
+        });
+
+
+        it('should pass UploadError instances unmodified', done => {
+          shellMkdirSpy.and.callFake(() => { throw new UploadError(543, 'Test'); });
+          bc.create(pr, sha, archive, isPublic).catch(err => {
+            expectToBeUploadError(err, 543, 'Test');
+            done();
+          });
+        });
+
       });
 
     });
@@ -317,4 +553,101 @@ describe('BuildCreator', () => {
 
   });
 
+
+  describe('listShasByDate()', () => {
+    let shellLsSpy: jasmine.Spy;
+    const lsResult = (name: string, mtimeMs: number, isDirectory = true) => ({
+      isDirectory: () => isDirectory,
+      mtime: new Date(mtimeMs),
+      name,
+    });
+
+    beforeEach(() => {
+      shellLsSpy = spyOn(shell, 'ls').and.returnValue([]);
+    });
+
+
+    it('should return a promise', done => {
+      const promise = (bc as any).listShasByDate('input/dir');
+      promise.then(done);   // Do not complete the test (and release the spies) synchronously
+                            // to avoid running the actual `ls()`.
+
+      expect(promise).toEqual(jasmine.any(Promise));
+    });
+
+
+    it('should `ls()` files with their metadata', done => {
+      (bc as any).listShasByDate('input/dir').
+        then(() => expect(shellLsSpy).toHaveBeenCalledWith('-l', 'input/dir')).
+        then(done);
+    });
+
+
+    it('should reject if listing files fails', done => {
+      shellLsSpy.and.returnValue(Promise.reject('Test'));
+      (bc as any).listShasByDate('input/dir').catch((err: string) => {
+        expect(err).toBe('Test');
+        done();
+      });
+    });
+
+
+    it('should return the filenames', done => {
+      shellLsSpy.and.returnValue(Promise.resolve([
+        lsResult('foo', 100),
+        lsResult('bar', 200),
+        lsResult('baz', 300),
+      ]));
+
+      (bc as any).listShasByDate('input/dir').
+        then((shas: string[]) => expect(shas).toEqual(['foo', 'bar', 'baz'])).
+        then(done);
+    });
+
+
+    it('should sort by date', done => {
+      shellLsSpy.and.returnValue(Promise.resolve([
+        lsResult('foo', 300),
+        lsResult('bar', 100),
+        lsResult('baz', 200),
+      ]));
+
+      (bc as any).listShasByDate('input/dir').
+        then((shas: string[]) => expect(shas).toEqual(['bar', 'baz', 'foo'])).
+        then(done);
+    });
+
+
+    it('should not break with ShellJS\' custom `sort()` method', done => {
+      const mockArray = [
+        lsResult('foo', 300),
+        lsResult('bar', 100),
+        lsResult('baz', 200),
+      ];
+      mockArray.sort = jasmine.createSpy('sort');
+
+      shellLsSpy.and.returnValue(Promise.resolve(mockArray));
+      (bc as any).listShasByDate('input/dir').
+        then((shas: string[]) => {
+          expect(shas).toEqual(['bar', 'baz', 'foo']);
+          expect(mockArray.sort).not.toHaveBeenCalled();
+        }).
+        then(done);
+    });
+
+
+    it('should only include directories', done => {
+      shellLsSpy.and.returnValue(Promise.resolve([
+        lsResult('foo', 100),
+        lsResult('bar', 200, false),
+        lsResult('baz', 300),
+      ]));
+
+      (bc as any).listShasByDate('input/dir').
+        then((shas: string[]) => expect(shas).toEqual(['foo', 'baz'])).
+        then(done);
+    });
+
+  });
+
 });

aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/build-events.spec.ts

@@ -1,15 +1,15 @@
 // Imports
-import {BuildEvent, CreatedBuildEvent} from '../../lib/upload-server/build-events';
+import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/upload-server/build-events';
 
 // Tests
-describe('BuildEvent', () => {
+describe('ChangedPrVisibilityEvent', () => {
-  let evt: BuildEvent;
+  let evt: ChangedPrVisibilityEvent;
 
-  beforeEach(() => evt = new BuildEvent('foo', 42, 'bar'));
+  beforeEach(() => evt = new ChangedPrVisibilityEvent(42, ['foo', 'bar'], true));
 
 
-  it('should have a \'type\' property', () => {
+  it('should have a static \'type\' property', () => {
-    expect(evt.type).toBe('foo');
+    expect(ChangedPrVisibilityEvent.type).toBe('pr.changedVisibility');
   });
 
 
@@ -18,8 +18,13 @@ describe('BuildEvent', () => {
   });
 
 
-  it('should have a \'sha\' property', () => {
+  it('should have a \'shas\' property', () => {
-    expect(evt.sha).toBe('bar');
+    expect(evt.shas).toEqual(['foo', 'bar']);
+  });
+
+
+  it('should have an \'isPublic\' property', () => {
+    expect(evt.isPublic).toBe(true);
   });
 
 });
@@ -28,7 +33,7 @@ describe('BuildEvent', () => {
 describe('CreatedBuildEvent', () => {
   let evt: CreatedBuildEvent;
 
-  beforeEach(() => evt = new CreatedBuildEvent(42, 'bar'));
+  beforeEach(() => evt = new CreatedBuildEvent(42, 'bar', true));
 
 
   it('should have a static \'type\' property', () => {
@@ -36,19 +41,6 @@ describe('CreatedBuildEvent', () => {
   });
 
 
-  it('should extend BuildEvent', () => {
-    expect(evt).toEqual(jasmine.any(CreatedBuildEvent));
-    expect(evt).toEqual(jasmine.any(BuildEvent));
-
-    expect(Object.getPrototypeOf(evt)).toBe(CreatedBuildEvent.prototype);
-  });
-
-
-  it('should automatically set the \'type\'', () => {
-    expect(evt.type).toBe(CreatedBuildEvent.type);
-  });
-
-
   it('should have a \'pr\' property', () => {
     expect(evt.pr).toBe(42);
   });
@@ -58,4 +50,9 @@ describe('CreatedBuildEvent', () => {
     expect(evt.sha).toBe('bar');
   });
 
+
+  it('should have an \'isPublic\' property', () => {
+    expect(evt.isPublic).toBe(true);
+  });
+
 });

aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/build-verifier.spec.ts

@@ -1,8 +1,8 @@
 // Imports
 import * as jwt from 'jsonwebtoken';
-import {GithubPullRequests} from '../../lib/common/github-pull-requests';
+import {GithubPullRequests, PullRequest} from '../../lib/common/github-pull-requests';
 import {GithubTeams} from '../../lib/common/github-teams';
-import {BuildVerifier} from '../../lib/upload-server/build-verifier';
+import {BUILD_VERIFICATION_STATUS, BuildVerifier} from '../../lib/upload-server/build-verifier';
 import {expectToBeUploadError} from './helpers';
 
 // Tests
@@ -13,14 +13,15 @@ describe('BuildVerifier', () => {
     organization: 'organization',
     repoSlug: 'repo/slug',
     secret: 'secret',
+    trustedPrLabel: 'trusted: pr-label',
   };
   let bv: BuildVerifier;
 
   // Helpers
   const createBuildVerifier = (partialConfig: Partial<typeof defaultConfig> = {}) => {
-    const cfg = {...defaultConfig, ...partialConfig};
+    const cfg = {...defaultConfig, ...partialConfig} as typeof defaultConfig;
     return new BuildVerifier(cfg.secret, cfg.githubToken, cfg.repoSlug, cfg.organization,
-                             cfg.allowedTeamSlugs);
+                             cfg.allowedTeamSlugs, cfg.trustedPrLabel);
   };
 
   beforeEach(() => bv = createBuildVerifier());
@@ -28,12 +29,13 @@ describe('BuildVerifier', () => {
 
   describe('constructor()', () => {
 
-    ['secret', 'githubToken', 'repoSlug', 'organization', 'allowedTeamSlugs'].forEach(param => {
+    ['secret', 'githubToken', 'repoSlug', 'organization', 'allowedTeamSlugs', 'trustedPrLabel'].
-      it(`should throw if '${param}' is missing or empty`, () => {
+      forEach(param => {
-        expect(() => createBuildVerifier({[param]: ''})).
+        it(`should throw if '${param}' is missing or empty`, () => {
-          toThrowError(`Missing or empty required parameter '${param}'!`);
+          expect(() => createBuildVerifier({[param]: ''})).
+            toThrowError(`Missing or empty required parameter '${param}'!`);
+        });
       });
-    });
 
 
     it('should throw if \'allowedTeamSlugs\' is an empty array', () => {
@@ -44,6 +46,122 @@ describe('BuildVerifier', () => {
   });
 
 
+  describe('getPrIsTrusted()', () => {
+    const pr = 9;
+    let mockPrInfo: PullRequest;
+    let prsFetchSpy: jasmine.Spy;
+    let teamsIsMemberBySlugSpy: jasmine.Spy;
+
+    beforeEach(() => {
+      mockPrInfo = {
+        labels: [
+          {name: 'foo'},
+          {name: 'bar'},
+        ],
+        number: 9,
+        user: {login: 'username'},
+      };
+
+      prsFetchSpy = spyOn(GithubPullRequests.prototype, 'fetch').
+        and.returnValue(Promise.resolve(mockPrInfo));
+
+      teamsIsMemberBySlugSpy = spyOn(GithubTeams.prototype, 'isMemberBySlug').
+        and.returnValue(Promise.resolve(true));
+    });
+
+
+    it('should return a promise', done => {
+      const promise = bv.getPrIsTrusted(pr);
+      promise.then(done);   // Do not complete the test (and release the spies) synchronously
+                            // to avoid running the actual `GithubTeams#isMemberBySlug()`.
+
+      expect(promise).toEqual(jasmine.any(Promise));
+    });
+
+
+    it('should fetch the corresponding PR', done => {
+      bv.getPrIsTrusted(pr).then(() => {
+        expect(prsFetchSpy).toHaveBeenCalledWith(pr);
+        done();
+      });
+    });
+
+
+    it('should fail if fetching the PR errors', done => {
+      prsFetchSpy.and.callFake(() => Promise.reject('Test'));
+      bv.getPrIsTrusted(pr).catch(err => {
+        expect(err).toBe('Test');
+        done();
+      });
+    });
+
+
+    describe('when the PR has the "trusted PR" label', () => {
+
+      beforeEach(() => mockPrInfo.labels.push({name: 'trusted: pr-label'}));
+
+
+      it('should resolve to true', done => {
+        bv.getPrIsTrusted(pr).then(isTrusted => {
+          expect(isTrusted).toBe(true);
+          done();
+        });
+      });
+
+
+      it('should not try to verify the author\'s membership status', done => {
+        bv.getPrIsTrusted(pr).then(() => {
+          expect(teamsIsMemberBySlugSpy).not.toHaveBeenCalled();
+          done();
+        });
+      });
+
+    });
+
+
+    describe('when the PR does not have the "trusted PR" label', () => {
+
+      it('should verify the PR author\'s membership in the specified teams', done => {
+        bv.getPrIsTrusted(pr).then(() => {
+          expect(teamsIsMemberBySlugSpy).toHaveBeenCalledWith('username', ['team1', 'team2']);
+          done();
+        });
+      });
+
+
+      it('should fail if verifying membership errors', done => {
+        teamsIsMemberBySlugSpy.and.callFake(() => Promise.reject('Test'));
+        bv.getPrIsTrusted(pr).catch(err => {
+          expect(err).toBe('Test');
+          done();
+        });
+      });
+
+
+      it('should resolve to true if the PR\'s author is a member', done => {
+        teamsIsMemberBySlugSpy.and.returnValue(Promise.resolve(true));
+
+        bv.getPrIsTrusted(pr).then(isTrusted => {
+          expect(isTrusted).toBe(true);
+          done();
+        });
+      });
+
+
+      it('should resolve to false if the PR\'s author is not a member', done => {
+        teamsIsMemberBySlugSpy.and.returnValue(Promise.resolve(false));
+
+        bv.getPrIsTrusted(pr).then(isTrusted => {
+          expect(isTrusted).toBe(false);
+          done();
+        });
+      });
+
+    });
+
+  });
+
+
   describe('verify()', () => {
     const pr = 9;
     const defaultJwt = {
@@ -53,22 +171,21 @@ describe('BuildVerifier', () => {
       'pull-request': pr,
       'slug': defaultConfig.repoSlug,
     };
-    let bvGetPrAuthorTeamMembership: jasmine.Spy;
+    let bvGetPrIsTrusted: jasmine.Spy;
 
     // Heleprs
     const createAuthHeader = (partialJwt: Partial<typeof defaultJwt> = {}, secret: string = defaultConfig.secret) =>
       `Token ${jwt.sign({...defaultJwt, ...partialJwt}, secret)}`;
 
     beforeEach(() => {
-      bvGetPrAuthorTeamMembership = spyOn(bv, 'getPrAuthorTeamMembership').
+      bvGetPrIsTrusted = spyOn(bv, 'getPrIsTrusted').and.returnValue(Promise.resolve(true));
-        and.returnValue(Promise.resolve({author: 'some-author', isMember: true}));
     });
 
 
     it('should return a promise', done => {
       const promise = bv.verify(pr, createAuthHeader());
       promise.then(done);   // Do not complete the test (and release the spies) synchronously
-                            // to avoid running the actual `bvGetPrAuthorTeamMembership()`.
+                            // to avoid running the actual `bvGetPrIsTrusted()`.
 
       expect(promise).toEqual(jasmine.any(Promise));
     });
@@ -148,16 +265,16 @@ describe('BuildVerifier', () => {
     });
 
 
-    it('should call \'getPrAuthorTeamMembership()\' if the token is valid', done => {
+    it('should call \'getPrIsTrusted()\' if the token is valid', done => {
       bv.verify(pr, createAuthHeader()).then(() => {
-        expect(bvGetPrAuthorTeamMembership).toHaveBeenCalledWith(pr);
+        expect(bvGetPrIsTrusted).toHaveBeenCalledWith(pr);
         done();
       });
     });
 
 
-    it('should fail if \'getPrAuthorTeamMembership()\' rejects', done => {
+    it('should fail if \'getPrIsTrusted()\' rejects', done => {
-      bvGetPrAuthorTeamMembership.and.callFake(() => Promise.reject('Test'));
+      bvGetPrIsTrusted.and.callFake(() => Promise.reject('Test'));
       bv.verify(pr, createAuthHeader()).catch(err => {
         expectToBeUploadError(err, 403, `Error while verifying upload for PR ${pr}: Test`);
         done();
@@ -165,97 +282,22 @@ describe('BuildVerifier', () => {
     });
 
 
-    it('should fail if \'getPrAuthorTeamMembership()\' reports no membership', done => {
+    it('should resolve to `verifiedNotTrusted` if \'getPrIsTrusted()\' returns false', done => {
-      const errorMessage = `Error while verifying upload for PR ${pr}: User 'test' is not an active member of any of ` +
+      bvGetPrIsTrusted.and.returnValue(Promise.resolve(false));
-                           'the following teams: team1, team2';
+      bv.verify(pr, createAuthHeader()).then(value => {
-
+        expect(value).toBe(BUILD_VERIFICATION_STATUS.verifiedNotTrusted);
-      bvGetPrAuthorTeamMembership.and.returnValue(Promise.resolve({author: 'test', isMember: false}));
-      bv.verify(pr, createAuthHeader()).catch(err => {
-        expectToBeUploadError(err, 403, errorMessage);
         done();
       });
     });
 
 
-    it('should succeed if everything checks outs', done => {
+    it('should resolve to `verifiedAndTrusted` if \'getPrIsTrusted()\' returns true', done => {
-      bv.verify(pr, createAuthHeader()).then(done);
+      bv.verify(pr, createAuthHeader()).then(value => {
-    });
+        expect(value).toBe(BUILD_VERIFICATION_STATUS.verifiedAndTrusted);
-
-  });
-
-
-  describe('getPrAuthorTeamMembership()', () => {
-    const pr = 9;
-    let prsFetchSpy: jasmine.Spy;
-    let teamsIsMemberBySlugSpy: jasmine.Spy;
-
-    beforeEach(() => {
-      prsFetchSpy = spyOn(GithubPullRequests.prototype, 'fetch').
-        and.returnValue(Promise.resolve({user: {login: 'username'}}));
-
-      teamsIsMemberBySlugSpy = spyOn(GithubTeams.prototype, 'isMemberBySlug').
-        and.returnValue(Promise.resolve(true));
-    });
-
-
-    it('should return a promise', done => {
-      const promise = bv.getPrAuthorTeamMembership(pr);
-      promise.then(done);   // Do not complete the test (and release the spies) synchronously
-                            // to avoid running the actual `GithubTeams#isMemberBySlug()`.
-
-      expect(promise).toEqual(jasmine.any(Promise));
-    });
-
-
-    it('should fetch the corresponding PR', done => {
-      bv.getPrAuthorTeamMembership(pr).then(() => {
-        expect(prsFetchSpy).toHaveBeenCalledWith(pr);
         done();
       });
     });
 
-
-    it('should fail if fetching the PR errors', done => {
-      prsFetchSpy.and.callFake(() => Promise.reject('Test'));
-      bv.getPrAuthorTeamMembership(pr).catch(err => {
-        expect(err).toBe('Test');
-        done();
-      });
-    });
-
-
-    it('should verify the PR author\'s membership in the specified teams', done => {
-      bv.getPrAuthorTeamMembership(pr).then(() => {
-        expect(teamsIsMemberBySlugSpy).toHaveBeenCalledWith('username', ['team1', 'team2']);
-        done();
-      });
-    });
-
-
-    it('should fail if verifying membership errors', done => {
-      teamsIsMemberBySlugSpy.and.callFake(() => Promise.reject('Test'));
-      bv.getPrAuthorTeamMembership(pr).catch(err => {
-        expect(err).toBe('Test');
-        done();
-      });
-    });
-
-
-    it('should return the PR\'s author and whether they are members', done => {
-      teamsIsMemberBySlugSpy.and.returnValues(Promise.resolve(true), Promise.resolve(false));
-
-      Promise.all([
-        bv.getPrAuthorTeamMembership(pr).then(({author, isMember}) => {
-          expect(author).toBe('username');
-          expect(isMember).toBe(true);
-        }),
-        bv.getPrAuthorTeamMembership(pr).then(({author, isMember}) => {
-          expect(author).toBe('username');
-          expect(isMember).toBe(false);
-        }),
-      ]).then(done);
-    });
-
   });
 
 });

aio/aio-builds-setup/dockerbuild/scripts-js/test/upload-server/upload-server-factory.spec.ts

@@ -4,8 +4,8 @@ import * as http from 'http';
 import * as supertest from 'supertest';
 import {GithubPullRequests} from '../../lib/common/github-pull-requests';
 import {BuildCreator} from '../../lib/upload-server/build-creator';
-import {CreatedBuildEvent} from '../../lib/upload-server/build-events';
+import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/upload-server/build-events';
-import {BuildVerifier} from '../../lib/upload-server/build-verifier';
+import {BUILD_VERIFICATION_STATUS, BuildVerifier} from '../../lib/upload-server/build-verifier';
 import {uploadServerFactory as usf} from '../../lib/upload-server/upload-server-factory';
 
 // Tests
@@ -18,11 +18,12 @@ describe('uploadServerFactory', () => {
     githubToken: '12345',
     repoSlug: 'repo/slug',
     secret: 'secret',
+    trustedPrLabel: 'trusted: pr-label',
   };
 
   // Helpers
   const createUploadServer = (partialConfig: Partial<typeof defaultConfig> = {}) =>
-    usf.create({...defaultConfig, ...partialConfig});
+    usf.create({...defaultConfig, ...partialConfig} as typeof defaultConfig);
 
 
   describe('create()', () => {
@@ -75,6 +76,12 @@ describe('uploadServerFactory', () => {
     });
 
 
+    it('should throw if \'trustedPrLabel\' is missing or empty', () => {
+      expect(() => createUploadServer({trustedPrLabel: ''})).
+        toThrowError('Missing or empty required parameter \'trustedPrLabel\'!');
+    });
+
+
     it('should return an http.Server', () => {
       const httpCreateServerSpy = spyOn(http, 'createServer').and.callThrough();
       const server = createUploadServer();
@@ -141,26 +148,71 @@ describe('uploadServerFactory', () => {
     });
 
 
-    it('should post a comment on GitHub on \'build.created\'', () => {
+    describe('on \'build.created\'', () => {
-      const prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment');
+      let prsAddCommentSpy: jasmine.Spy;
-      const commentBody = 'The angular.io preview for 1234567890 is available [here][1].\n\n' +
-                          '[1]: https://pr42-1234567890.domain.name/';
 
-      buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890'});
+      beforeEach(() => prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment'));
+
+
+      it('should post a comment on GitHub for public previews', () => {
+        const commentBody = 'You can preview 1234567890 at https://pr42-1234567890.domain.name/.';
+
+        buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890', isPublic: true});
+        expect(prsAddCommentSpy).toHaveBeenCalledWith(42, commentBody);
+      });
+
+
+      it('should not post a comment on GitHub for non-public previews', () => {
+        buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890', isPublic: false});
+        expect(prsAddCommentSpy).not.toHaveBeenCalled();
+      });
+
+    });
+
+
+    describe('on \'pr.changedVisibility\'', () => {
+      let prsAddCommentSpy: jasmine.Spy;
+
+      beforeEach(() => prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment'));
+
+
+      it('should post a comment on GitHub (for all SHAs) for PRs made public', () => {
+        const commentBody = 'You can preview 12345 at https://pr42-12345.domain.name/.\n' +
+                            'You can preview 67890 at https://pr42-67890.domain.name/.';
+
+        buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: ['12345', '67890'], isPublic: true});
+        expect(prsAddCommentSpy).toHaveBeenCalledWith(42, commentBody);
+      });
+
+
+      it('should not post a comment on GitHub if no SHAs were affected', () => {
+        buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: [], isPublic: true});
+        expect(prsAddCommentSpy).not.toHaveBeenCalled();
+      });
+
+
+      it('should not post a comment on GitHub for PRs made non-public', () => {
+        buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: ['12345', '67890'], isPublic: false});
+        expect(prsAddCommentSpy).not.toHaveBeenCalled();
+      });
 
-      expect(prsAddCommentSpy).toHaveBeenCalledWith(42, commentBody);
     });
 
 
     it('should pass the correct \'githubToken\' and \'repoSlug\' to GithubPullRequests', () => {
       const prsAddCommentSpy = spyOn(GithubPullRequests.prototype, 'addComment');
 
-      buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890'});
+      buildCreator.emit(CreatedBuildEvent.type, {pr: 42, sha: '1234567890', isPublic: true});
-      const prs = prsAddCommentSpy.calls.mostRecent().object;
+      buildCreator.emit(ChangedPrVisibilityEvent.type, {pr: 42, shas: ['12345', '67890'], isPublic: true});
 
+      const allCalls = prsAddCommentSpy.calls.all();
+      const prs = allCalls[0].object;
+
+      expect(prsAddCommentSpy).toHaveBeenCalledTimes(2);
+      expect(prs).toBe(allCalls[1].object);
       expect(prs).toEqual(jasmine.any(GithubPullRequests));
-      expect((prs as any).repoSlug).toBe('repo/slug');
+      expect(prs.repoSlug).toBe('repo/slug');
-      expect((prs as any).requestHeaders.Authorization).toContain('12345');
+      expect(prs.requestHeaders.Authorization).toContain('12345');
     });
 
   });
@@ -184,6 +236,7 @@ describe('uploadServerFactory', () => {
         defaultConfig.repoSlug,
         defaultConfig.githubOrganization,
         defaultConfig.githubTeamSlugs,
+        defaultConfig.trustedPrLabel,
       );
       buildCreator = new BuildCreator(defaultConfig.buildsDir);
       agent = supertest.agent((usf as any).createMiddleware(buildVerifier, buildCreator));
@@ -199,7 +252,8 @@ describe('uploadServerFactory', () => {
       let buildCreatorCreateSpy: jasmine.Spy;
 
       beforeEach(() => {
-        buildVerifierVerifySpy = spyOn(buildVerifier, 'verify').and.returnValue(Promise.resolve());
+        const verStatus = BUILD_VERIFICATION_STATUS.verifiedAndTrusted;
+        buildVerifierVerifySpy = spyOn(buildVerifier, 'verify').and.returnValue(Promise.resolve(verStatus));
         buildCreatorCreateSpy = spyOn(buildCreator, 'create').and.returnValue(Promise.resolve());
       });
 
@@ -284,14 +338,17 @@ describe('uploadServerFactory', () => {
 
 
       it('should call \'BuildCreator#create()\' with the correct arguments', done => {
-        const req = agent.
+        buildVerifierVerifySpy.and.returnValues(
-          get(`/create-build/${pr}/${sha}`).
+            Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedAndTrusted),
-          set('AUTHORIZATION', 'foo').
+            Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedNotTrusted));
-          set('X-FILE', 'bar');
 
-        promisifyRequest(req).
+        const req1 = agent.get(`/create-build/${pr}/${sha}`).set('AUTHORIZATION', 'foo').set('X-FILE', 'bar');
-          then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'bar')).
+        const req2 = agent.get(`/create-build/${pr}/${sha}`).set('AUTHORIZATION', 'foo').set('X-FILE', 'bar');
-          then(done, done.fail);
+
+        Promise.all([
+          promisifyRequest(req1).then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'bar', true)),
+          promisifyRequest(req2).then(() => expect(buildCreatorCreateSpy).toHaveBeenCalledWith(pr, sha, 'bar', false)),
+        ]).then(done, done.fail);
       });
 
 
@@ -307,7 +364,7 @@ describe('uploadServerFactory', () => {
       });
 
 
-      it('should respond with 201 on successful upload', done => {
+      it('should respond with 201 on successful upload (for public builds)', done => {
         const req = agent.
           get(`/create-build/${pr}/${sha}`).
           set('AUTHORIZATION', 'foo').
@@ -318,6 +375,18 @@ describe('uploadServerFactory', () => {
       });
 
 
+      it('should respond with 202 on successful upload (for hidden builds)', done => {
+        buildVerifierVerifySpy.and.returnValue(Promise.resolve(BUILD_VERIFICATION_STATUS.verifiedNotTrusted));
+        const req = agent.
+          get(`/create-build/${pr}/${sha}`).
+          set('AUTHORIZATION', 'foo').
+          set('X-FILE', 'bar').
+          expect(202, http.STATUS_CODES[202]);
+
+        verifyRequests([req], done);
+      });
+
+
       it('should reject PRs with leading zeros', done => {
         verifyRequests([agent.get(`/create-build/0${pr}/${sha}`).expect(404)], done);
       });

aio/aio-builds-setup/dockerbuild/scripts-sh/clean-up.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-set -e -o pipefail
+set -eu -o pipefail
 
 # Set up env variables
 export AIO_GITHUB_TOKEN=$(head -c -1 /aio-secrets/GITHUB_TOKEN 2>/dev/null)

aio/aio-builds-setup/dockerbuild/scripts-sh/health-check.sh

@@ -1,5 +1,6 @@
 #!/bin/bash
-set +e -o pipefail
+# Using `+e` so that all checks are run and we get a complete report (even if some checks failed).
+set +e -u -o pipefail
 
 
 # Variables

aio/aio-builds-setup/dockerbuild/scripts-sh/init.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-set -e -o pipefail
+set -eu -o pipefail
 
 exec >> /var/log/aio/init.log
 exec 2>&1

aio/aio-builds-setup/dockerbuild/scripts-sh/upload-server-prod.sh

@@ -1,9 +1,9 @@
 #!/bin/bash
-set -e -o pipefail
+set -eu -o pipefail
 
 # Set up env variables for production
-export AIO_GITHUB_TOKEN=$(head -c -1 /aio-secrets/GITHUB_TOKEN 2>/dev/null)
+export AIO_GITHUB_TOKEN=$(head -c -1 /aio-secrets/GITHUB_TOKEN 2>/dev/null || echo "MISSING_GITHUB_TOKEN")
-export AIO_PREVIEW_DEPLOYMENT_TOKEN=$(head -c -1 /aio-secrets/PREVIEW_DEPLOYMENT_TOKEN 2>/dev/null)
+export AIO_PREVIEW_DEPLOYMENT_TOKEN=$(head -c -1 /aio-secrets/PREVIEW_DEPLOYMENT_TOKEN 2>/dev/null || echo "MISSING_PREVIEW_DEPLOYMENT_TOKEN")
 
 # Start the upload-server instance
 # TODO(gkalpak): Ideally, the upload server should be run as a non-privileged user.

aio/aio-builds-setup/dockerbuild/scripts-sh/upload-server-test.sh

@@ -1,13 +1,13 @@
 #!/bin/bash
-set -e -o pipefail
+set -eu -o pipefail
 
 # Set up env variables for testing
 export AIO_BUILDS_DIR=$TEST_AIO_BUILDS_DIR
 export AIO_DOMAIN_NAME=$TEST_AIO_DOMAIN_NAME
 export AIO_GITHUB_ORGANIZATION=$TEST_AIO_GITHUB_ORGANIZATION
 export AIO_GITHUB_TEAM_SLUGS=$TEST_AIO_GITHUB_TEAM_SLUGS
-export AIO_PREVIEW_DEPLOYMENT_TOKEN=$TEST_AIO_PREVIEW_DEPLOYMENT_TOKEN
 export AIO_REPO_SLUG=$TEST_AIO_REPO_SLUG
+export AIO_TRUSTED_PR_LABEL=$TEST_AIO_TRUSTED_PR_LABEL
 export AIO_UPLOAD_HOSTNAME=$TEST_AIO_UPLOAD_HOSTNAME
 export AIO_UPLOAD_PORT=$TEST_AIO_UPLOAD_PORT
 

aio/aio-builds-setup/dockerbuild/scripts-sh/verify-setup.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-set -e -o pipefail
+set -eu -o pipefail
 
 logFile=/var/log/aio/verify-setup.log
 uploadServerLogFile=/var/log/aio/upload-server-verify-setup.log

aio/aio-builds-setup/docs/_TOC.md

@@ -4,7 +4,8 @@
 ## Overview
 - [General overview](overview--general.md)
 - [Security model](overview--security-model.md)
-- [Available Commands](overview--scripts-and-commands.md)
+- [Available scripts and commands](overview--scripts-and-commands.md)
+- [HTTP status codes](overview--http-status-codes.md)
 
 
 ## Setting up the VM

aio/aio-builds-setup/docs/image-config--environment-variables.md

@@ -17,7 +17,7 @@ you don't need to specify values for those.
   The domain name of the server.
 
 - `AIO_GITHUB_ORGANIZATION`:
-  The GitHub organization whose teams arew whitelisted for accepting uploads.
+  The GitHub organization whose teams are whitelisted for accepting uploads.
   See also `AIO_GITHUB_TEAM_SLUGS`.
 
 - `AIO_GITHUB_TEAM_SLUGS`:
@@ -39,6 +39,11 @@ you don't need to specify values for those.
 - `AIO_REPO_SLUG`:
   The repository slug (in the form `<user>/<repo>`) for which PRs will be uploaded.
 
+- `AIO_TRUSTED_PR_LABEL`:
+  The PR whose presence indicates the PR has been manually verified and is allowed to have its
+  build artifacts publicly served. This is useful for enabling previews for any PR (not only those
+  from trusted authors).
+
 - `AIO_UPLOAD_HOSTNAME`:
   The internal hostname for accessing the Node.js upload-server. This is used by nginx for
   delegating upload requests and also for performing a periodic health-check.

aio/aio-builds-setup/docs/overview--general.md

@@ -33,36 +33,51 @@ container:
 
 
 ### On CI (Travis)
-- Build job completes successfully (i.e. build succeeds and tests pass).
+- Build job completes successfully.
 - The CI script checks whether the build job was initiated by a PR against the angular/angular
   master branch.
-- The CI script checks whether the PR has touched any files inside the angular.io project directory
+- The CI script checks whether the PR has touched any files that might affect the angular.io app
-  (currently `aio/`).
+  (currently the `aio/` or `packages/` directories, ignoring spec files).
-- The CI script checks whether the author of the PR is a member of one of the whitelisted GitHub
+- Optionally, the CI script can check whether the PR can be automatically verified (i.e. if the
-  teams (and therefore allowed to upload).
+  author of the PR is a member of one of the whitelisted GitHub teams or the PR has the specified
+  "trusted PR" label).
   **Note:**
   For security reasons, the same checks will be performed on the server as well. This is an optional
-  step with the purpose of:
+  step that can be used in case one wants to apply special logic depending on the outcome of the
-  1. Avoiding the wasted overhead associated with uploads that are going to be rejected (e.g.
+  pre-verification. For example:
-     building the artifacts, sending them to the server, running checks on the server, etc).
+  1. One might want to deploy automatically verified PRs only. In that case, the pre-verification
-  2. Avoiding failing the build (due to an error response from the server) or requiring additional
+     helps avoid the wasted overhead associated with uploads that are going to be rejected (e.g.
-     logic for detecting the reasons of the failure.
+     building the artifacts, sending them to the server, running checks on the server, detecting the
-- The CI script gzip and upload the build artifacts to the server.
+     reasons of deployment failure and whether to fail the build, etc).
+  2. One might want to apply additional logic (e.g. different tests) depending on whether the PR is
+     automatically verified or not).
+- The CI script gzips and uploads the build artifacts to the server.
 
 More info on how to set things up on CI can be found [here](misc--integrate-with-ci.md).
 
 
 ### Uploading build artifacts
-- nginx receives upload request.
+- nginx receives the upload request.
 - nginx checks that the uploaded gzip archive does not exceed the specified max file size, stores it
   in a temporary location and passes the filepath to the Node.js upload-server.
-- The upload-server verifies that the uploaded file is not trying to overwrite an existing build,
+- The upload-server runs several checks to determine whether the request should be accepted and
-  and runs several checks to determine whether the request should be accepted (more details can be
+  whether it should be publicly accessible or stored for later verification (more details can be
   found [here](overview--security-model.md)).
-- The upload-server deploys the artifacts to a sub-directory named after the PR number and SHA:
+- The upload-server changes the "visibility" of the associated PR, if necessary. For example, if
-  `<PR>/<SHA>/`
+  builds for the same PR had been previously deployed as non-public and the current build has been
-- The upload-server posts a comment on the corresponding PR on GitHub mentioning the SHA and the
+  automatically verified, all previous builds are made public as well.
-  the link where the preview can be found.
+  If the PR transitions from "non-public" to "public", the upload-server posts a comment on the
+  corresponding PR on GitHub mentioning the SHAs and the links where the previews can be found.
+- The upload-server verifies that the uploaded file is not trying to overwrite an existing build.
+- The upload-server deploys the artifacts to a sub-directory named after the PR number and the first
+  few characters of the SHA: `<PR>/<SHA>/`
+  (Non-publicly accessible PRs will be stored in a different location, but again derived from the PR
+  number and SHA.)
+- If the PR is publicly accessible, the upload-server posts a comment on the corresponding PR on
+  GitHub mentioning the SHA and the link where the preview can be found.
+
+More info on the possible HTTP status codes and their meaning can be found
+[here](overview--http-status-codes.md).
 
 
 ### Serving build artifacts
@@ -71,6 +86,9 @@ More info on how to set things up on CI can be found [here](misc--integrate-with
 - nginx maps the subdomain to the correct sub-directory and serves the resource.
   E.g.: `/<PR>/<SHA>/path/to/resource`
 
+Again, more info on the possible HTTP status codes and their meaning can be found
+[here](overview--http-status-codes.md).
+
 
 ### Removing obsolete artifacts
 In order to avoid flooding the disk with unnecessary build artifacts, there is a cronjob that runs a

aio/aio-builds-setup/docs/overview--http-status-codes.md

@@ -0,0 +1,66 @@
+# Overview - HTTP Status Codes
+
+
+This is a list of all the possible HTTP status codes returned by the nginx anf upload servers, along
+with a bried explanation of what they mean:
+
+
+## `http://*.ngbuilds.io/*`
+
+- **307 (Temporary Redirect)**:
+  All non-HTTPS requests. 308 (Permanent Redirect) would be more appropriate, but is not supported
+  by all agents (e.g. cURL).
+
+
+## `https://pr<pr>-<sha>.ngbuilds.io/*`
+
+- **200 (OK)**:
+  File was found or URL was rewritten to `/index.html` (i.e. all paths that have no `.` in final
+  segment).
+
+- **403 (Forbidden)**:
+  Trying to access a sub-directory.
+
+- **404 (Not Found)**:
+  File not found.
+
+
+## `https://ngbuilds.io/create-build/<pr>/<sha>`
+
+- **201 (Created)**:
+  Build deployed successfully and is publicly available.
+
+- **202 (Accepted)**:
+  Build not automatically verifiable. Stored for later deployment (after re-verification).
+
+- **400 (Bad Request)**:
+  No payload.
+
+- **401 (Unauthorized)**:
+  No `AUTHORIZATION` header.
+
+- **403 (Forbidden)**:
+  Unable to verify build (e.g. invalid JWT token, or unable to talk to 3rd-party APIs, etc).
+
+- **404 (Not Found)**:
+  Tried to change PR visibility but the source directory did not exist.
+  (Currently, this can only happen as a rare race condition during build deployment.)
+
+- **405 (Method Not Allowed)**:
+  Request method other than POST.
+
+- **409 (Conflict)**:
+  Request to overwrite existing directory (e.g. deploy existing build or change PR visibility when
+  the destination directory does already exist).
+
+- **413 (Payload Too Large)**:
+  Payload larger than size specified in `AIO_UPLOAD_MAX_SIZE`.
+
+
+## `https://*.ngbuilds.io/*`
+
+- **404 (Not Found)**:
+  Request not matched by the above rules.
+
+- **500 (Internal Server Error)**:
+  Error while processing a request matched by the above rules.

aio/aio-builds-setup/docs/overview--scripts-and-commands.md

@@ -12,20 +12,22 @@ available:
   Can be used for creating a preconfigured docker image.
   See [here](vm-setup--create-docker-image.md) for more info.
 
-- `test.sh`
+- `test.sh`:
   Can be used for running the tests for `<aio-builds-setup-dir>/dockerbuild/scripts-js/`. This is
   useful for CI integration. See [here](misc--integrate-with-ci.md) for more info.
 
-- `travis-preverify-pr.sh`
+- `travis-preverify-pr.sh`:
-  Can be used for "preverifying" a PR before uploading the artifacts to the server. It checks that
+  Can be used for "pre-verifying" a PR before uploading the artifacts to the server. It checks
-  the author of the PR is a member of one of the specified GitHub teams and therefore allowed to
+  whether the author of the PR is a member of one of the specified GitHub teams (therefore allowed
-  upload build artifacts. This is useful for CI integration. See [here](misc--integrate-with-ci.md)
+  to upload build artifacts) or the PR has the specified "trusted PR" label (meaning it has been
-  for more info.
+  manually verified by a trusted member). This is useful for CI integration.
+  See [here](misc--integrate-with-ci.md) for more info.
 
-- `update-preview-server.sh`
+- `update-preview-server.sh`:
   Can be used for updating the docker container (and image) based on the latest changes checked out
   from a git repository. See [here](vm-setup--update-docker-container.md) for more info.
 
+
 ## Commands
 The following commands are available globally from inside the docker container. They are either used
 by the container to perform its various operations or can be used ad-hoc, mainly for testing

aio/aio-builds-setup/docs/overview--security-model.md

@@ -41,12 +41,13 @@ part of the CI setup and serving them publicly.
 The implemented approach can be broken up to the following sub-tasks:
 
 1. Verify which PR the uploaded artifacts correspond to.
-2. Determine the author of the PR.
+2. Fetch the PR's metadata, including author and labels.
-3. Check whether the PR author is a member of some whitelisted GitHub team.
+3. Check whether the PR can be automatically verified as "trusted" (based on its author or labels).
-4. Deploy the artifacts to the corresponding PR's directory.
+4. If necessary, update the corresponding PR's verification status.
-5. Prevent overwriting previously deployed artifacts (which ensures that the guarantees established
+5. Deploy the artifacts to the corresponding PR's directory.
+6. Prevent overwriting previously deployed artifacts (which ensures that the guarantees established
    during deployment will remain valid until the artifacts are removed).
-6. Prevent uploaded files from accessing anything outside their directory.
+7. Prevent uploaded files from accessing anything outside their directory.
 
 
 ### Implementation details
@@ -65,35 +66,51 @@ This section describes how each of the aforementioned sub-tasks is accomplished:
    _There are currently certain limitation in the implementation of the JWT addon._
    _See the next section for more details._
 
-2. **Determine the author of the PR.**
+2. **Fetch the PR's metadata, including author and labels**.
 
-   Once we have securely associated the uploaded artifaacts to a PR, we retrieve the PR's metadata -
+   Once we have securely associated the uploaded artifacts to a PR, we retrieve the PR's metadata -
-   including the author's username - using the [GitHub API](https://developer.github.com/v3/).
+   including the author's username and the labels - using the
+   [GitHub API](https://developer.github.com/v3/).
    To avoid rate-limit restrictions, we use a Personal Access Token (issued by
    [@mary-poppins](https://github.com/mary-poppins)).
 
-3. **Check whether the PR author is a member of some whitelisted GitHub team.**
+3. **Check whether the PR can be automatically verified as "trusted"**.
 
-   Again using the GitHub API, we can verify the author's membership in one of the
+   "Trusted" means that we are confident that the build artifacts are suitable for being deployed
-   whitelisted/trusted GitHub teams. For this operation, we need a PErsonal Access Token with the
+   and publicly accessible on the preview server. There are two ways to check that:
-   `read:org` scope issued by a user that can "see" the specified GitHub organization.
+   1. We can verify that the PR has a pre-determined label, which marks it as "safe for preview".
-   Here too, we use token by @mary-poppins.
+      Such a label can only have been added by a maintainer (with the necessary rights) and
+      designates that they have manually verified the PR contents.
+   2. We can verify (again using the GitHub API) the author's membership in one of the
+      whitelisted/trusted GitHub teams. For this operation, we need a Personal Access Token with the
+      `read:org` scope issued by a user that can "see" the specified GitHub organization.
+      Here too, we use the token by @mary-poppins.
 
-4. **Deploy the artifacts to the corresponding PR's directory.**
+4. **If necessary update the corresponding PR's verification status**.
+
+   Once we have determined whether the PR is considered "trusted", we update its "visibility" (i.e.
+   whether it is publicly accessible or not), based on the new verification status. For example, if
+   a PR was initially considered "not trusted" but the check triggered by a new build determined
+   otherwise, the PR (and all the previously uploaded previews) are made public. It works the same
+   way if a PR has gone from "trusted" to "not trusted".
+
+5. **Deploy the artifacts to the corresponding PR's directory.**
 
    With the preceeding steps, we have verified that the uploaded artifacts have been uploaded by
-   Travis and correspond to a PR whose author is a member of a trusted team. Essentially, as long as
+   Travis. Additionally, we have determined whether the PR can be trusted to have its previews
-   sub-tasks 1, 2 and 3 can be securely accomplished, it is possible to "project" the trust we have
+   publicly accessible or whether further verification is necessary. The artifacts will be stored to
-   in a team's members through the PR and Travis to the build artifacts.
+   the PR's directory, but will not be publicly accessible unless the PR has been verified.
+   Essentially, as long as sub-tasks 1, 2 and 3 can be securely accomplished, it is possible to
+   "project" the trust we have in a team's members through the PR and Travis to the build artifacts.
 
-5. **Prevent overwriting previously deployed artifacts**.
+6. **Prevent overwriting previously deployed artifacts**.
 
-   In order to enforce this restriction (and ensure that the deployed artifacts validity is
+   In order to enforce this restriction (and ensure that the deployed artifacts' validity is
    preserved throughout their "lifetime"), the server that handles the upload (currently a Node.js
    Express server) rejects uploads that target an existing directory.
    _Note: A PR can contain multiple uploads; one for each SHA that was built on Travis._
 
-6. **Prevent uploaded files from accessing anything outside their directory.**
+7. **Prevent uploaded files from accessing anything outside their directory.**
 
    Nginx (which is used to serve the uploaded artifacts) has been configured to not follow symlinks
    outside of the directory where the build artifacts are stored.
@@ -104,6 +121,11 @@ This section describes how each of the aforementioned sub-tasks is accomplished:
 - Each trusted PR author has full control over the content that is uploaded for their PRs. Part of
   the security model relies on the trustworthiness of these authors.
 
+- Adding the specified label on a PR and marking it as trusted, gives the author full control over
+  the content that is uploaded for the specific PR (e.g. by pushing more commits to it). The user
+  adding the label is responsible for ensuring that this control is not abused and that the PR is
+  either closed (one way of another) or the access is revoked.
+
 - If anyone gets access to the `PREVIEW_DEPLOYMENT_TOKEN` (a.k.a. `NGBUILDS_IO_KEY` on
   angular/angular) variable generated for each Travis job, they will be able to impersonate the
   corresponding PR's author on the preview server for as long as the token is valid (currently 90

aio/aio-builds-setup/docs/vm-setup--create-docker-image.md

@@ -25,7 +25,7 @@ The following commands would create a docker image from GitHub repo `foo/bar` to
     --build-arg AIO_REPO_SLUG=foo/bar \
     --build-arg AIO_DOMAIN_NAME=foobar-builds.io \
     --build-arg AIO_GITHUB_ORGANIZATION=foo \
-    --build-arg AIO_GITHUB_TEMA_SLUGS=bar-core,bar-docs-authors
+    --build-arg AIO_GITHUB_TEAM_SLUGS=bar-core,bar-docs-authors
   ```
 
 A full list of the available environment variables can be found

aio/aio-builds-setup/scripts/create-image.sh

@@ -5,6 +5,14 @@ set -eux -o pipefail
 source "`dirname $0`/_env.sh"
 readonly defaultImageNameAndTag="aio-builds:latest"
 
+# Build `scripts-js/`
+# (Necessary, because only `scripts-js/dist/` is copied to the docker image.)
+(
+  cd "$SCRIPTS_JS_DIR"
+  yarn install
+  yarn build
+)
+
 # Create docker image
 readonly nameAndOptionalTag=${1:-$defaultImageNameAndTag}
 sudo docker build --tag $nameAndOptionalTag ${@:2} $DOCKERBUILD_DIR

aio/aio-builds-setup/scripts/travis-preverify-pr.sh

@@ -16,5 +16,11 @@ AIO_GITHUB_ORGANIZATION="angular" \
 AIO_GITHUB_TEAM_SLUGS="angular-core,aio-contributors" \
 AIO_GITHUB_TOKEN=$(echo ${GITHUB_TEAM_MEMBERSHIP_CHECK_KEY} | rev) \
 AIO_REPO_SLUG=$TRAVIS_REPO_SLUG \
+AIO_TRUSTED_PR_LABEL="aio: preview" \
 AIO_PREVERIFY_PR=$TRAVIS_PULL_REQUEST \
 node "$SCRIPTS_JS_DIR/dist/lib/upload-server/index-preverify-pr"
+
+# Exit codes:
+# - 0: The PR can be automatically trusted (i.e. author belongs to trusted team or PR has the "trusted PR" label).
+# - 1: An error occurred.
+# - 2: The PR cannot be automatically trusted.

aio/content/examples/aot-compiler/rollup-config.js

@@ -1,8 +1,7 @@
 // #docregion
-import rollup      from 'rollup'
+import nodeResolve from 'rollup-plugin-node-resolve';
-import nodeResolve from 'rollup-plugin-node-resolve'
 import commonjs    from 'rollup-plugin-commonjs';
-import uglify      from 'rollup-plugin-uglify'
+import uglify      from 'rollup-plugin-uglify';
 
 // #docregion config
 export default {
@@ -30,5 +29,5 @@ export default {
       uglify()
       // #enddocregion uglify
   ]
-}
+};
 // #enddocregion config

aio/content/examples/dynamic-component-loader/src/app/ad-banner.component.ts

@@ -24,7 +24,7 @@ export class AdBannerComponent implements AfterViewInit, OnDestroy {
   subscription: any;
   interval: any;
 
-  constructor(private _componentFactoryResolver: ComponentFactoryResolver) { }
+  constructor(private componentFactoryResolver: ComponentFactoryResolver) { }
 
   ngAfterViewInit() {
     this.loadComponent();
@@ -39,7 +39,7 @@ export class AdBannerComponent implements AfterViewInit, OnDestroy {
     this.currentAddIndex = (this.currentAddIndex + 1) % this.ads.length;
     let adItem = this.ads[this.currentAddIndex];
 
-    let componentFactory = this._componentFactoryResolver.resolveComponentFactory(adItem.component);
+    let componentFactory = this.componentFactoryResolver.resolveComponentFactory(adItem.component);
 
     let viewContainerRef = this.adHost.viewContainerRef;
     viewContainerRef.clear();

aio/content/examples/ngmodule/src/app/crisis/crisis-detail.component.ts

@@ -14,6 +14,6 @@ export class CrisisDetailComponent implements OnInit {
   constructor(private route: ActivatedRoute) {  }
 
   ngOnInit() {
-    this.id = parseInt(this.route.snapshot.params['id'], 10);
+    this.id = parseInt(this.route.snapshot.paramMap.get('id'), 10);
   }
 }

aio/content/examples/ngmodule/src/app/hero/hero-detail.component.ts

@@ -25,7 +25,7 @@ export class HeroDetailComponent implements OnInit {
     private heroService: HeroService) { }
 
   ngOnInit() {
-    let id = parseInt(this.route.snapshot.params['id'], 10);
+    let id = parseInt(this.route.snapshot.paramMap.get('id'), 10);
     this.heroService.getHero(id).then(hero => this.hero = hero);
   }
 }

aio/content/examples/router/src/app/admin/admin-dashboard.component.2.ts

@@ -22,8 +22,8 @@ export class AdminDashboardComponent implements OnInit {
   ngOnInit() {
     // Capture the session ID if available
     this.sessionId = this.route
-      .queryParams
+      .queryParamMap
-      .map(params => params['session_id'] || 'None');
+      .map(params => params.get('session_id') || 'None');
 
     // Capture the fragment if available
     this.token = this.route

aio/content/examples/router/src/app/admin/admin-dashboard.component.ts

@@ -36,8 +36,8 @@ export class AdminDashboardComponent implements OnInit {
   ngOnInit() {
     // Capture the session ID if available
     this.sessionId = this.route
-      .queryParams
+      .queryParamMap
-      .map(params => params['session_id'] || 'None');
+      .map(params => params.get('session_id') || 'None');
 
     // Capture the fragment if available
     this.token = this.route

aio/content/examples/router/src/app/app-routing.module.1.ts

@@ -17,7 +17,10 @@ const appRoutes: Routes = [
 
 @NgModule({
   imports: [
-    RouterModule.forRoot(appRoutes)
+    RouterModule.forRoot(
+      appRoutes,
+      { enableTracing: true } // <-- debugging purposes only
+    )
   ],
   exports: [
     RouterModule

aio/content/examples/router/src/app/app-routing.module.2.ts

@@ -15,7 +15,10 @@ const appRoutes: Routes = [
 
 @NgModule({
   imports: [
-    RouterModule.forRoot(appRoutes)
+    RouterModule.forRoot(
+      appRoutes,
+      { enableTracing: true } // <-- debugging purposes only
+    )
   ],
   exports: [
     RouterModule

aio/content/examples/router/src/app/app-routing.module.3.ts

@@ -22,7 +22,10 @@ const appRoutes: Routes = [
 
 @NgModule({
   imports: [
-    RouterModule.forRoot(appRoutes)
+    RouterModule.forRoot(
+      appRoutes,
+      { enableTracing: true } // <-- debugging purposes only
+    )
   ],
   exports: [
     RouterModule

aio/content/examples/router/src/app/app-routing.module.4.ts

@@ -18,7 +18,10 @@ const appRoutes: Routes = [
 
 @NgModule({
   imports: [
-    RouterModule.forRoot(appRoutes)
+    RouterModule.forRoot(
+      appRoutes,
+      { enableTracing: true } // <-- debugging purposes only
+    )
   ],
   exports: [
     RouterModule

aio/content/examples/router/src/app/app-routing.module.5.ts

@@ -33,7 +33,10 @@ const appRoutes: Routes = [
 
 @NgModule({
   imports: [
-    RouterModule.forRoot(appRoutes)
+    RouterModule.forRoot(
+      appRoutes,
+      { enableTracing: true } // <-- debugging purposes only
+    )
   ],
   exports: [
     RouterModule

aio/content/examples/router/src/app/app-routing.module.6.ts

@@ -37,9 +37,12 @@ const appRoutes: Routes = [
   imports: [
     // #docregion forRoot
     RouterModule.forRoot(
-      appRoutes
+      appRoutes,
       // #enddocregion preload-v1
-      , { preloadingStrategy: PreloadAllModules }
+      {
+        enableTracing: true, // <-- debugging purposes only
+        preloadingStrategy: PreloadAllModules
+      }
       // #docregion preload-v1
     )
     // #enddocregion forRoot

aio/content/examples/router/src/app/app-routing.module.ts

@@ -36,7 +36,11 @@ const appRoutes: Routes = [
   imports: [
     RouterModule.forRoot(
       appRoutes,
-      { preloadingStrategy: SelectivePreloadingStrategy }
+      {
+        enableTracing: true, // <-- debugging purposes only
+        preloadingStrategy: SelectivePreloadingStrategy,
+
+      }
     )
   ],
   exports: [

aio/content/examples/router/src/app/app.module.0.ts

@@ -26,7 +26,10 @@ const appRoutes: Routes = [
 
 @NgModule({
   imports: [
-    RouterModule.forRoot(appRoutes)
+    RouterModule.forRoot(
+      appRoutes,
+      { enableTracing: true } // <-- debugging purposes only
+    )
     // other imports here
   ],
 // #enddocregion

aio/content/examples/router/src/app/app.module.1.ts

@@ -33,7 +33,10 @@ const appRoutes: Routes = [
   imports: [
     BrowserModule,
     FormsModule,
-    RouterModule.forRoot(appRoutes)
+    RouterModule.forRoot(
+      appRoutes,
+      { enableTracing: true } // <-- debugging purposes only
+    )
   ],
   declarations: [
     AppComponent,

aio/content/examples/router/src/app/can-deactivate-guard.service.1.ts

@@ -15,7 +15,7 @@ export class CanDeactivateGuard implements CanDeactivate<CrisisDetailComponent>
     state: RouterStateSnapshot
   ): Promise<boolean> | boolean {
     // Get the Crisis Center ID
-    console.log(route.params['id']);
+    console.log(route.paramMap.get('id'));
 
     // Get the current URL
     console.log(state.url);

aio/content/examples/router/src/app/crisis-center/crisis-center-home.component.ts

@@ -1,13 +1,9 @@
 // #docregion
-// #docplaster
 import { Component } from '@angular/core';
 
-// #docregion minus-imports
 @Component({
   template: `
     <p>Welcome to the Crisis Center</p>
   `
 })
 export class CrisisCenterHomeComponent { }
-// #enddocregion minus-imports
-// #enddocregion

aio/content/examples/router/src/app/crisis-center/crisis-center-routing.module.1.ts

@@ -31,6 +31,7 @@ const crisisCenterRoutes: Routes = [
     ]
   }
 ];
+// #enddocregion routes
 
 @NgModule({
   imports: [

aio/content/examples/router/src/app/crisis-center/crisis-center.component.ts

@@ -1,8 +1,6 @@
 // #docregion
-// #docplaster
 import { Component } from '@angular/core';
 
-// #docregion minus-imports
 @Component({
   template:  `
     <h2>CRISIS CENTER</h2>
@@ -10,5 +8,3 @@ import { Component } from '@angular/core';
   `
 })
 export class CrisisCenterComponent { }
-// #enddocregion minus-imports
-// #enddocregion

aio/content/examples/router/src/app/crisis-center/crisis-detail-resolver.service.ts

@@ -10,7 +10,7 @@ export class CrisisDetailResolver implements Resolve<Crisis> {
   constructor(private cs: CrisisService, private router: Router) {}
 
   resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Promise<Crisis> {
-    let id = route.params['id'];
+    let id = route.paramMap.get('id');
 
     return this.cs.getCrisis(id).then(crisis => {
       if (crisis) {

aio/content/examples/router/src/app/crisis-center/crisis-detail.component.1.ts

@@ -2,7 +2,7 @@
 // #docregion
 import 'rxjs/add/operator/switchMap';
 import { Component, OnInit, HostBinding } from '@angular/core';
-import { ActivatedRoute, Router, Params } from '@angular/router';
+import { ActivatedRoute, Router, ParamMap } from '@angular/router';
 
 import { slideInDownAnimation }   from '../animations';
 import { Crisis, CrisisService }  from './crisis.service';
@@ -44,8 +44,8 @@ export class CrisisDetailComponent implements OnInit {
 
   // #docregion ngOnInit
   ngOnInit() {
-    this.route.params
+    this.route.paramMap
-      .switchMap((params: Params) => this.service.getCrisis(params['id']))
+      .switchMap((params: ParamMap) => this.service.getCrisis(params.get('id')))
       .subscribe((crisis: Crisis) => {
         if (crisis) {
           this.editName = crisis.name;

aio/content/examples/router/src/app/crisis-center/crisis-list.component.1.ts

@@ -1,7 +1,7 @@
 import 'rxjs/add/operator/do';
 import 'rxjs/add/operator/switchMap';
 import { Component, OnInit }              from '@angular/core';
-import { ActivatedRoute, Router, Params } from '@angular/router';
+import { ActivatedRoute, Router, ParamMap } from '@angular/router';
 
 import { Crisis, CrisisService } from './crisis.service';
 import { Observable }            from 'rxjs/Observable';
@@ -31,9 +31,9 @@ export class CrisisListComponent implements OnInit {
   ) {}
 
   ngOnInit() {
-    this.crises = this.route.params
+    this.crises = this.route.paramMap
-      .switchMap((params: Params) => {
+      .switchMap((params: ParamMap) => {
-        this.selectedId = +params['id'];
+        this.selectedId = +params.get('id');
         return this.service.getCrises();
       });
   }

aio/content/examples/router/src/app/crisis-center/crisis-list.component.ts

@@ -1,7 +1,7 @@
 // #docregion
 import 'rxjs/add/operator/switchMap';
 import { Component, OnInit }      from '@angular/core';
-import { ActivatedRoute, Router, Params } from '@angular/router';
+import { ActivatedRoute, Router, ParamMap } from '@angular/router';
 
 import { Observable }            from 'rxjs/Observable';
 
@@ -38,9 +38,9 @@ export class CrisisListComponent implements OnInit {
   }
 
   ngOnInit() {
-    this.crises = this.route.params
+    this.crises = this.route.paramMap
-      .switchMap((params: Params) => {
+      .switchMap((params: ParamMap) => {
-        this.selectedId = +params['id'];
+        this.selectedId = +params.get('id');
         return this.service.getCrises();
       });
   }

aio/content/examples/router/src/app/heroes/hero-detail.component.1.ts

@@ -5,7 +5,7 @@ import 'rxjs/add/operator/switchMap';
 // #enddocregion rxjs-operator-import
 import { Component, OnInit } from '@angular/core';
 // #docregion imports
-import { Router, ActivatedRoute, Params } from '@angular/router';
+import { Router, ActivatedRoute, ParamMap } from '@angular/router';
 // #enddocregion imports
 
 import { Hero, HeroService } from './hero.service';
@@ -40,9 +40,9 @@ export class HeroDetailComponent implements OnInit  {
 
   // #docregion ngOnInit
   ngOnInit() {
-    this.route.params
+    this.route.paramMap
-      // (+) converts string 'id' to a number
+      .switchMap((params: ParamMap) =>
-      .switchMap((params: Params) => this.service.getHero(+params['id']))
+        this.service.getHero(params.get('id')))
       .subscribe((hero: Hero) => this.hero = hero);
   }
   // #enddocregion ngOnInit

aio/content/examples/router/src/app/heroes/hero-detail.component.2.ts

@@ -33,8 +33,7 @@ export class HeroDetailComponent implements OnInit  {
 
   // #docregion snapshot
   ngOnInit() {
-    // (+) converts string 'id' to a number
+    let id = this.route.snapshot.paramMap.get('id');
-    let id = +this.route.snapshot.params['id'];
 
     this.service.getHero(id)
       .then((hero: Hero) => this.hero = hero);

aio/content/examples/router/src/app/heroes/hero-detail.component.ts

@@ -4,7 +4,7 @@
 import 'rxjs/add/operator/switchMap';
 // #enddocregion rxjs-operator-import
 import { Component, OnInit, HostBinding } from '@angular/core';
-import { Router, ActivatedRoute, Params } from '@angular/router';
+import { Router, ActivatedRoute, ParamMap } from '@angular/router';
 
 import { slideInDownAnimation } from '../animations';
 
@@ -47,9 +47,9 @@ export class HeroDetailComponent implements OnInit {
 
   // #docregion ngOnInit
   ngOnInit() {
-    this.route.params
+    this.route.paramMap
-      // (+) converts string 'id' to a number
+      .switchMap((params: ParamMap) =>
-      .switchMap((params: Params) => this.service.getHero(+params['id']))
+        this.service.getHero(params.get('id')))
       .subscribe((hero: Hero) => this.hero = hero);
   }
   // #enddocregion ngOnInit

aio/content/examples/router/src/app/heroes/hero-list.component.ts

@@ -7,7 +7,7 @@ import { Observable } from 'rxjs/Observable';
 // #enddocregion rxjs-imports
 import { Component, OnInit } from '@angular/core';
 // #docregion import-router
-import { Router, ActivatedRoute, Params } from '@angular/router';
+import { Router, ActivatedRoute, ParamMap } from '@angular/router';
 // #enddocregion import-router
 
 import { Hero, HeroService }  from './hero.service';
@@ -41,9 +41,10 @@ export class HeroListComponent implements OnInit {
   ) {}
 
   ngOnInit() {
-    this.heroes = this.route.params
+    this.heroes = this.route.paramMap
-      .switchMap((params: Params) => {
+      .switchMap((params: ParamMap) => {
-        this.selectedId = +params['id'];
+        // (+) before `params.get()` turns the string into a number
+        this.selectedId = +params.get('id');
         return this.service.getHeroes();
       });
   }

aio/content/examples/router/src/app/heroes/hero.service.ts

@@ -22,6 +22,7 @@ export class HeroService {
 
   getHero(id: number | string) {
     return heroesPromise
+      // (+) before `id` turns the string into a number
       .then(heroes => heroes.find(hero => hero.id === +id));
   }
 }

aio/content/examples/router/src/app/login.component.ts

@@ -38,7 +38,7 @@ export class LoginComponent {
         // Set our navigation extras object
         // that passes on our global query params and fragment
         let navigationExtras: NavigationExtras = {
-          preserveQueryParams: true,
+          queryParamsHandling: 'preserve',
           preserveFragment: true
         };
 

aio/content/examples/testing/src/app/hero/hero-detail.component.no-testbed.spec.ts

@@ -15,7 +15,7 @@ describe('HeroDetailComponent - no TestBed', () => {
   beforeEach((done: any) => {
     expectedHero = new Hero(42, 'Bubba');
     activatedRoute = new ActivatedRouteStub();
-    activatedRoute.testParams = { id: expectedHero.id };
+    activatedRoute.testParamMap = { id: expectedHero.id };
 
     router = jasmine.createSpyObj('router', ['navigate']);
 

aio/content/examples/testing/src/app/hero/hero-detail.component.spec.ts

@@ -53,7 +53,7 @@ function overrideSetup() {
   // #enddocregion hds-spy
 
   // the `id` value is irrelevant because ignored by service stub
-  beforeEach(() => activatedRoute.testParams = { id: 99999 } );
+  beforeEach(() => activatedRoute.testParamMap = { id: 99999 } );
 
   // #docregion setup-override
   beforeEach( async(() => {
@@ -158,7 +158,7 @@ function heroModuleSetup() {
 
     beforeEach( async(() => {
       expectedHero = firstHero;
-      activatedRoute.testParams = { id: expectedHero.id };
+      activatedRoute.testParamMap = { id: expectedHero.id };
       createComponent();
     }));
 
@@ -229,7 +229,7 @@ function heroModuleSetup() {
   // #docregion route-bad-id
   describe('when navigate to non-existant hero id', () => {
     beforeEach( async(() => {
-      activatedRoute.testParams = { id: 99999 };
+      activatedRoute.testParamMap = { id: 99999 };
       createComponent();
     }));
 
@@ -279,7 +279,7 @@ function formsModuleSetup() {
 
   it('should display 1st hero\'s name', fakeAsync(() => {
     const expectedHero = firstHero;
-    activatedRoute.testParams = { id: expectedHero.id };
+    activatedRoute.testParamMap = { id: expectedHero.id };
     createComponent().then(() => {
       expect(page.nameDisplay.textContent).toBe(expectedHero.name);
     });
@@ -307,7 +307,7 @@ function sharedModuleSetup() {
 
   it('should display 1st hero\'s name', fakeAsync(() => {
     const expectedHero = firstHero;
-    activatedRoute.testParams = { id: expectedHero.id };
+    activatedRoute.testParamMap = { id: expectedHero.id };
     createComponent().then(() => {
       expect(page.nameDisplay.textContent).toBe(expectedHero.name);
     });

aio/content/examples/testing/src/app/hero/hero-detail.component.ts

@@ -29,7 +29,7 @@ export class HeroDetailComponent implements OnInit {
   // #docregion ng-on-init
   ngOnInit(): void {
     // get hero when `id` param changes
-    this.route.params.subscribe(p => this.getHero(p && p['id']));
+    this.route.paramMap.subscribe(p => this.getHero(p.has('id') && p.get('id')));
   }
   // #enddocregion ng-on-init
 

aio/content/examples/testing/src/testing/router-stubs.ts

@@ -30,28 +30,29 @@ export class RouterStub {
 }
 
 
-// Only implements params and part of snapshot.params
+// Only implements params and part of snapshot.paramMap
 // #docregion activated-route-stub
 import { BehaviorSubject } from 'rxjs/BehaviorSubject';
+import { convertToParamMap, ParamMap } from '@angular/router';
 
 @Injectable()
 export class ActivatedRouteStub {
 
-  // ActivatedRoute.params is Observable
+  // ActivatedRoute.paramMap is Observable
-  private subject = new BehaviorSubject(this.testParams);
+  private subject = new BehaviorSubject(convertToParamMap(this.testParamMap));
-  params = this.subject.asObservable();
+  paramMap = this.subject.asObservable();
 
   // Test parameters
-  private _testParams: {};
+  private _testParamMap: ParamMap;
-  get testParams() { return this._testParams; }
+  get testParamMap() { return this._testParamMap; }
-  set testParams(params: {}) {
+  set testParamMap(params: {}) {
-    this._testParams = params;
+    this._testParamMap = convertToParamMap(params);
-    this.subject.next(params);
+    this.subject.next(this._testParamMap);
   }
 
-  // ActivatedRoute.snapshot.params
+  // ActivatedRoute.snapshot.paramMap
   get snapshot() {
-    return { params: this.testParams };
+    return { paramMap: this.testParamMap };
   }
 }
 // #enddocregion activated-route-stub

aio/content/examples/toh-pt5/src/app/hero-detail.component.1.ts

@@ -5,7 +5,7 @@
 // #docregion added-imports
 // Keep the Input import for now, you'll remove it later:
 import { Component, Input, OnInit } from '@angular/core';
-import { ActivatedRoute, Params }   from '@angular/router';
+import { ActivatedRoute, ParamMap } from '@angular/router';
 import { Location }                 from '@angular/common';
 
 import { HeroService } from './hero.service';
@@ -17,7 +17,7 @@ import { Hero } from './hero';
 @Component({})
 export class HeroDetailComponent implements OnInit {
   @Input() hero: Hero;
-  bogus: Params;
+  bogus: ParamMap;
 
   constructor(
     private heroService: HeroService,

aio/content/examples/toh-pt5/src/app/hero-detail.component.ts

@@ -2,9 +2,9 @@
 // #docregion , v2, rxjs-import
 import 'rxjs/add/operator/switchMap';
 // #enddocregion rxjs-import
-import { Component, OnInit }      from '@angular/core';
+import { Component, OnInit }        from '@angular/core';
-import { ActivatedRoute, Params } from '@angular/router';
+import { ActivatedRoute, ParamMap } from '@angular/router';
-import { Location }               from '@angular/common';
+import { Location }                 from '@angular/common';
 
 import { Hero }         from './hero';
 import { HeroService }  from './hero.service';
@@ -32,8 +32,8 @@ export class HeroDetailComponent implements OnInit {
 
   // #docregion ngOnInit
   ngOnInit(): void {
-    this.route.params
+    this.route.paramMap
-      .switchMap((params: Params) => this.heroService.getHero(+params['id']))
+      .switchMap((params: ParamMap) => this.heroService.getHero(+params.get('id')))
       .subscribe(hero => this.hero = hero);
   }
   // #enddocregion ngOnInit

aio/content/examples/toh-pt6/src/app/hero-detail.component.ts

@@ -1,8 +1,8 @@
 // #docregion
 import 'rxjs/add/operator/switchMap';
-import { Component, OnInit }      from '@angular/core';
+import { Component, OnInit }        from '@angular/core';
-import { ActivatedRoute, Params } from '@angular/router';
+import { ActivatedRoute, ParamMap } from '@angular/router';
-import { Location }               from '@angular/common';
+import { Location }                 from '@angular/common';
 
 import { Hero }        from './hero';
 import { HeroService } from './hero.service';
@@ -22,8 +22,8 @@ export class HeroDetailComponent implements OnInit {
   ) {}
 
   ngOnInit(): void {
-    this.route.params
+    this.route.paramMap
-      .switchMap((params: Params) => this.heroService.getHero(+params['id']))
+      .switchMap((params: ParamMap) => this.heroService.getHero(+params.get('id')))
       .subscribe(hero => this.hero = hero);
   }
 

aio/content/examples/toh-pt6/src/app/hero-search.service.ts

@@ -14,7 +14,7 @@ export class HeroSearchService {
 
   search(term: string): Observable<Hero[]> {
     return this.http
-               .get(`app/heroes/?name=${term}`)
+               .get(`api/heroes/?name=${term}`)
                .map(response => response.json().data as Hero[]);
   }
 }

aio/content/examples/universal/src/app/hero-detail.component.ts

@@ -1,7 +1,7 @@
 // #docregion
 import 'rxjs/add/operator/switchMap';
 import { Component, OnInit }      from '@angular/core';
-import { ActivatedRoute, Params } from '@angular/router';
+import { ActivatedRoute, ParamMap } from '@angular/router';
 import { Location }               from '@angular/common';
 
 import { Hero }        from './hero';
@@ -22,8 +22,8 @@ export class HeroDetailComponent implements OnInit {
   ) {}
 
   ngOnInit(): void {
-    this.route.params
+    this.route.paramMap
-      .switchMap((params: Params) => this.heroService.getHero(+params['id']))
+      .switchMap((params: ParamMap) => this.heroService.getHero(+params.get('id')))
       .subscribe(hero => this.hero = hero);
   }
 

aio/content/examples/universal/src/app/hero-search.service.ts

@@ -14,7 +14,7 @@ export class HeroSearchService {
 
   search(term: string): Observable<Hero[]> {
     return this.http
-               .get(`app/heroes/?name=${term}`)
+               .get(`api/heroes/?name=${term}`)
                .map(response => response.json().data as Hero[]);
   }
 }

aio/content/examples/upgrade-phonecat-2-hybrid/README.md

@@ -6,8 +6,6 @@ The following changes from vanilla Phonecat are applied:
 * Karma config for unit tests is in karma.conf.ajs.js because the boilerplate
   Karma config is not compatible with the way AngularJS tests need to be run.
   The shell script run-unit-tests.sh can be used to run the unit tests.
-* There's a `package.ajs.json`, which is not used to run anything but only to
-  show an example of changing the PhoneCat http-server root path.
 * Also for the Karma shim, there is a `karma-test-shim.1.js` file which isn't
   used but is shown in the test appendix.
 * Instead of using Bower, AngularJS and its dependencies are fetched from a CDN

aio/content/examples/upgrade-phonecat-2-hybrid/package.ajs.json

@@ -1,37 +0,0 @@
-{
-  "name": "angular-phonecat",
-  "private": true,
-  "version": "0.0.0",
-  "description": "A tutorial application for AngularJS",
-  "repository": "https://github.com/angular/angular-phonecat",
-  "license": "MIT",
-  "devDependencies": {
-    "bower": "^1.7.7",
-    "http-server": "^0.9.0",
-    "jasmine-core": "^2.4.1",
-    "karma": "^0.13.22",
-    "karma-chrome-launcher": "^0.2.3",
-    "karma-firefox-launcher": "^0.1.7",
-    "karma-jasmine": "^0.3.8",
-    "protractor": "^3.2.2",
-    "shelljs": "^0.6.0"
-  },
-  "scripts": {
-    "postinstall": "bower install",
-
-    "prestart": "npm install",
-    "start": "http-server -a localhost -p 8000 -c-1 ./",
-
-    "pretest": "npm install",
-    "test": "karma start karma.conf.js",
-    "test-single-run": "karma start karma.conf.js --single-run",
-
-    "preupdate-webdriver": "npm install",
-    "update-webdriver": "webdriver-manager update",
-
-    "preprotractor": "npm run update-webdriver",
-    "protractor": "protractor e2e-tests/protractor.conf.js",
-
-    "update-index-async": "node -e \"require('shelljs/global'); sed('-i', /\\/\\/@@NG_LOADER_START@@[\\s\\S]*\\/\\/@@NG_LOADER_END@@/, '//@@NG_LOADER_START@@\\n' + sed(/sourceMappingURL=angular-loader.min.js.map/,'sourceMappingURL=bower_components/angular-loader/angular-loader.min.js.map','app/bower_components/angular-loader/angular-loader.min.js') + '\\n//@@NG_LOADER_END@@', 'app/index-async.html');\""
-  }
-}

aio/content/examples/upgrade-phonecat-3-final/app/phone-detail/phone-detail.component.ts

@@ -14,7 +14,7 @@ export class PhoneDetailComponent {
   mainImageUrl: string;
 
   constructor(activatedRoute: ActivatedRoute, phone: Phone) {
-    phone.get(activatedRoute.snapshot.params['phoneId'])
+    phone.get(activatedRoute.snapshot.paramMap.get('phoneId'))
       .subscribe((p: PhoneData) => {
         this.phone = p;
         this.setImage(p.images[0]);

aio/content/examples/upgrade-phonecat-3-final/run-unit-tests.sh

@@ -1,7 +1,6 @@
-## The boilerplate Karma configuration won't work with Angular 1 tests since
+## The boilerplate Karma configuration won't work with AngularJS tests
-## a specific loading configuration is needed for them.
+## which require their own special loading configuration, `karma.conf.ng1.js`.
-## We keep one in karma.conf.ng1.js. This scripts runs the ng1 tests with
+## This scripts runs the AngularJS tests with that AngularJS config.
-## that config.
 
 PATH=$(npm bin):$PATH
 tsc && karma start karma.conf.ng1.js

aio/content/guide/docs-style-guide.md

@@ -2,12 +2,35 @@
 
 This page presents design and layout guidelines for Angular documentation pages.  These guidelines should be followed by all guide page authors. Deviations must be approved by the documentation editor.
 
-For clarity and precision, every guideline on this page is illustrated with a working example 
+Most guide pages should have [accompanying sample code](#from-code-samples) with 
-followed by the page markup for that example.
+[special markup](#source-code-markup) for the code snippets on the page. 
+Code samples should adhere to the 
+[style guide for Angular applications](guide/styleguide "Application Code Style Guide") 
+because readers expect consistency.
+
+For clarity and precision, every guideline on _this_ page is illustrated with a working example, 
+followed by the page markup for that example ... as shown here.
 
 ```html
-  ... followed by the page markup for that example.
+  followed by the page markup for that example ... as shown here.
 ```
+
+## Doc generation and tooling
+
+To make changes to the documentation pages and sample code, clone the [Angular github repository](https://github.com/angular/angular "Angular repo") and go to the `aio/` folder.
+
+The [aio/README.md](https://github.com/angular/angular/blob/master/aio/README.md "AIO ReadMe") explains how to install and use the tools to edit and test your changes. 
+
+Here are a few essential commands for guide page authors.
+
+1. `yarn setup` — installs packages; builds docs, plunkers, and zips.
+
+1. `yarn docs-watch -- --watch-only` — watches for saved content changes and refreshes the browser. The (optional) `--watch-only` flag skips the initial docs rebuild.
+
+1. `yarn start`  —  starts the doc viewer application so you can see your local changes in the browser.
+
+1.  http://localhost:4200/  —  browse to the app running locally.
+
 ## Guide pages
 
  All but a few guide pages are [markdown](https://daringfireball.net/projects/markdown/syntax "markdown") files with an `.md` extension. 
@@ -254,7 +277,7 @@ Whatever the source, the doc viewer renders them as "code snippets", either indi
 
 {@a code-example}
 
-<h3 class="no-toc">Code example</h3>
+### Code example
 
 You can display a simple, inline code snippet with the markdown backtick syntax.
 We generally prefer to display a code snippet with the Angular documentation _code-example_ component
@@ -406,8 +429,7 @@ Here's the markup for an "avoid" example in the
 </code-example>
 
 {@a code-tabs}
-
+### Code Tabs
-<h3 class="no-toc">Code Tabs</h3> 
 
 Code tabs display code much like Code examples do.  The added advantage is that they can display mutiple code samples within a tabbed interface.  Each tab is displayed using Code-pane.
 
@@ -421,30 +443,52 @@ Code tabs display code much like Code examples do.  The added advantage is that
 * `title` - seen in the header of a tab
 * `linenums` - overrides the `linenums` property at the `code-tabs` level for this particular pane. The value can be `true`, `false` or a number indicating the starting line number. If not specified, line numbers are are enabled only when there are 10 or more lines.
 
-The example below uses the source code for a small application that bundles with Webpack techniques.  This creates multiple tabs, each with its own title and be appropriately color coded.  By default, line numbers are shown.  Line number display can be specified using the linenums attribute at the code tab or code pane level.  The example below shows us how to display line numbers in just one code pane. 
+The next example displays multiple code tabs, each with its own title.
+Line numbers are explicitly disabled for all panes at the `<code-tabs>` level.
+The second pane adds line numbers back for itself. 
 
-<code-tabs>
+<code-tabs linenums="false">
-  <code-pane title="app.component.html" path="docs-style-guide/src/app/app.component.html">
+  <code-pane 
+    title="app.component.html" 
+    path="docs-style-guide/src/app/app.component.html">
   </code-pane>
-  <code-pane title="app.component.ts" path="docs-style-guide/src/app/app.component.ts">
+  <code-pane 
+    title="app.component.ts" 
+    path="docs-style-guide/src/app/app.component.ts"
+    linenums="true">
   </code-pane>
-  <code-pane title="app.component.css (heroes)" path="docs-style-guide/src/app/app.component.css"  region="heroes">
+  <code-pane 
+    title="app.component.css (heroes)" 
+    path="docs-style-guide/src/app/app.component.css"  
+    region="heroes">
   </code-pane>
-  <code-pane title="tsconfig.json" path="docs-style-guide/src/tsconfig.json" linenums="false">
+  <code-pane 
+    title="package.json (scripts)" 
+    path="docs-style-guide/package.1.json">
   </code-pane>
 </code-tabs>
 
-The markup below creates multiple tab pane, each with its own title and syntax-colored content.
+Here's the markup for that example.
 
 ```html
-<code-tabs>
+<code-tabs linenums="false">
-  <code-pane title="app.component.html" path="docs-style-guide/src/app/app.component.html">
+  <code-pane 
+    title="app.component.html" 
+    path="docs-style-guide/src/app/app.component.html">
   </code-pane>
-  <code-pane title="app.component.ts" path="docs-style-guide/src/app/app.component.ts">
+  <code-pane 
+    title="app.component.ts" 
+    path="docs-style-guide/src/app/app.component.ts"
+    linenums="true">
   </code-pane>
-  <code-pane title="app.component.css (heroes)" path="docs-style-guide/src/app/app.component.css"  region="heroes">
+  <code-pane 
+    title="app.component.css (heroes)" 
+    path="docs-style-guide/src/app/app.component.css"  
+    region="heroes">
   </code-pane>
-  <code-pane title="tsconfig.json" path="docs-style-guide/src/tsconfig.json" linenums="false">
+  <code-pane 
+    title="package.json (scripts)" 
+    path="docs-style-guide/package.1.json">
   </code-pane>
 </code-tabs>
 ```

aio/content/guide/form-validation.md

@@ -25,24 +25,6 @@ and the [Reactive Forms](guide/reactive-forms) guides.
 
 </live-example>
 
-
-
-
-## Built-in validators
-
-Angular forms include a number of built-in validator functions, which are functions 
-that help you check common user input in forms. In addition to the built-in 
-validators covered here of `minlength`, `maxlength`, 
-and `required`, there are others such as `min`, `max`, `email` and `pattern` 
-for Template Driven as well as Reactive Forms. 
-For a full list of built-in validators, 
-see the [Validators](api/forms/Validators) API reference.
-
-
-
-
-
-
 ## Simple Template Driven Forms
 
 In the Template Driven approach, you arrange
@@ -473,8 +455,15 @@ Then it attaches the same `onValueChanged` handler (there's a one line differenc
 to the form's `valueChanges` event and calls it immediately
 to set error messages for the new control model.
 
+## Built-in validators
 
-{@a formbuilder}
+Angular forms include a number of built-in validator functions, which are functions 
+that help you check common user input in forms. In addition to the built-in 
+validators covered here of `minlength`, `maxlength`, 
+and `required`, there are others such as `email` and `pattern` 
+for Reactive Forms. 
+For a full list of built-in validators, 
+see the [Validators](api/forms/Validators) API reference.
 
 
 #### _FormBuilder_ declaration
@@ -490,19 +479,12 @@ Angular has stock validators that correspond to the standard HTML validation att
 The `forbiddenName` validator on the `"name"` control is a custom validator,
 discussed in a separate [section below](guide/form-validation#custom-validation).
 
-
 <div class="l-sub-section">
 
-
-
 Learn more about `FormBuilder` in the [Introduction to FormBuilder](guide/reactive-forms#formbuilder) section of Reactive Forms guide.
 
 </div>
 
-
-
-{@a committing-changes}
-
 #### Committing hero value changes
 
 In two-way data binding, the user's changes flow automatically from the controls back to the data model properties.
@@ -518,14 +500,12 @@ This sample updates the model twice:
 The `onSubmit()` method simply replaces the `hero` object with the combined values of the form:
 
 <code-example path="form-validation/src/app/reactive/hero-form-reactive.component.ts" region="on-submit" title="form-validation/src/app/reactive/hero-form-reactive.component.ts" linenums="false">
-
 </code-example>
 
 
 The `addHero()` method discards pending changes and creates a brand new `hero` model object.
 
 <code-example path="form-validation/src/app/reactive/hero-form-reactive.component.ts" region="add-hero" title="form-validation/src/app/reactive/hero-form-reactive.component.ts" linenums="false">
-
 </code-example>
 
 
@@ -555,20 +535,11 @@ Here's the complete reactive component file, compared to the two Template Driven
 
 <div class="l-sub-section">
 
-
-
 Run the [live example](guide/form-validation#live-example) to see how the reactive form behaves,
 and to compare all of the files in this sample.
 
-
 </div>
 
-
-
-
-{@a custom-validation}
-
-
 ## Custom validation
 This cookbook sample has a custom `forbiddenNameValidator()` function that's applied to both the
 Template Driven and the reactive form controls. It's in the `src/app/shared` folder
@@ -577,7 +548,6 @@ and declared in the `SharedModule`.
 Here's the `forbiddenNameValidator()` function:
 
 <code-example path="form-validation/src/app/shared/forbidden-name.directive.ts" region="custom-validator" title="shared/forbidden-name.directive.ts (forbiddenNameValidator)" linenums="false">
-
 </code-example>
 
 
@@ -596,15 +566,12 @@ The validation error object typically has a property whose name is the validatio
 and whose value is an arbitrary dictionary of values that you could insert into an error message (`{name}`).
 
 
-{@a custom-validation-directive}
-
 
 ### Custom validation directive
 In the Reactive Forms component, the `'name'` control's validator function list
 has a `forbiddenNameValidator` at the bottom.
 
 <code-example path="form-validation/src/app/reactive/hero-form-reactive.component.ts" region="name-validators" title="reactive/hero-form-reactive.component.ts (name validators)" linenums="false">
-
 </code-example>
 
 
@@ -613,7 +580,6 @@ In the Template Driven example, the `<input>` has the selector (`forbiddenName`)
 of a custom _attribute directive_, which rejects "bob".
 
 <code-example path="form-validation/src/app/template/hero-form-template2.component.html" region="name-input" title="template/hero-form-template2.component.html (name input)" linenums="false">
-
 </code-example>
 
 
@@ -624,7 +590,6 @@ Angular `forms` recognizes the directive's role in the validation process becaus
 with the `NG_VALIDATORS` provider, a provider with an extensible collection of validation directives.
 
 <code-example path="form-validation/src/app/shared/forbidden-name.directive.ts" region="directive-providers" title="shared/forbidden-name.directive.ts (providers)" linenums="false">
-
 </code-example>
 
 
@@ -632,7 +597,6 @@ with the `NG_VALIDATORS` provider, a provider with an extensible collection of v
 Here is the rest of the directive to help you get an idea of how it all comes together:
 
 <code-example path="form-validation/src/app/shared/forbidden-name.directive.ts" region="directive" title="shared/forbidden-name.directive.ts (directive)">
-
 </code-example>
 
 
@@ -641,8 +605,6 @@ Here is the rest of the directive to help you get an idea of how it all comes to
 
 <div class="l-sub-section">
 
-
-
 If you are familiar with Angular validations, you may have noticed
 that the custom validation directive is instantiated with `useExisting`
 rather than `useClass`. The registered validator must be _this instance_ of
@@ -655,29 +617,18 @@ To see this in action, run the example and then type “bob” in the name of He
 Notice that you get a validation error. Now change from `useExisting` to `useClass` and try again.
 This time, when you type “bob”, there's no "bob" error message.
 
-
 </div>
 
 
-
-
-
 <div class="l-sub-section">
 
-
-
 For more information on attaching behavior to elements,
 see [Attribute Directives](guide/attribute-directives).
 
-
 </div>
 
 
 
-
-{@a testing}
-
-
 ## Testing Considerations
 
 You can write _isolated unit tests_ of validation and control logic in Reactive Forms.

aio/content/guide/router.md

@@ -6,6 +6,10 @@ as users perform application tasks.
 This guide covers the router's primary features, illustrating them through the evolution
 of a small application that you can <live-example>run live in the browser</live-example>.
 
+<!-- style for all tables on this page -->
+<style>
+  td, th {vertical-align: top}
+</style>
 
 
 ## Overview
@@ -75,7 +79,7 @@ Import what you need from it as you would from any other Angular package.
 
 
 
-You'll learn about more options in the [details below](guide/router#browser-url-styles).
+You'll learn about more options in the [details below](#browser-url-styles).
 
 
 </div>
@@ -121,9 +125,9 @@ You'll learn more about route parameters later in this guide.
 The `data` property in the third route is a place to store arbitrary data associated with
 this specific route. The data property is accessible within each activated route. Use it to store
 items such as page titles, breadcrumb text, and other read-only, _static_ data.
-You'll use the [resolve guard](guide/router#resolve-guard) to retrieve _dynamic_ data later in the guide.
+You'll use the [resolve guard](#resolve-guard) to retrieve _dynamic_ data later in the guide.
 
-The `empty path` in the fourth route represents the default path for the application,
+The **empty path** in the fourth route represents the default path for the application,
 the place to go when the path in the URL is empty, as it typically is at the start.
 This default route redirects to the route for the `/heroes` URL and, therefore, will display the `HeroesListComponent`.
 
@@ -137,6 +141,7 @@ In the configuration above, routes with a static path are listed first, followed
 that matches the default route.
 The wildcard route comes last because it matches _every URL_ and should be selected _only_ if no other routes are matched first.
 
+If you need to see what events are happening during the navigation lifecycle, there is the **enableTracing** option as part of the router's default configuration. This outputs each router event that took place during each navigation lifecycle to the browser console. This should only be used for _debugging_ purposes. You set the `enableTracing: true` option in the object passed as the second argument to the `RouterModule.forRoot()` method.
 
 {@a basics-router-outlet}
 
@@ -199,6 +204,103 @@ application using the `Router` service and the `routerState` property.
 Each `ActivatedRoute` in the `RouterState` provides methods to traverse up and down the route tree
 to get information from parent, child and sibling routes.
 
+### Router events
+
+During each navigation, the `Router` emits navigation events through the `Router.events` property. These events range from when the navigation starts and ends to many points in between. The full list of navigation events is displayed in the table below.
+
+<table>
+  <tr>
+    <th>
+      Router Event
+    </th>
+
+    <th>
+      Description
+    </th>
+  </tr>
+
+  <tr>
+    <td>
+      <code>NavigationStart</code>
+    </td>
+    <td>
+
+      An [event](api/router/NavigationStart) triggered when navigation starts.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>RoutesRecognized</code>
+    </td>
+    <td>
+
+      An [event](api/router/RoutesRecognized) triggered when the Router parses the URL and the routes are recognized.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>RouteConfigLoadStart</code>
+    </td>
+    <td>
+
+      An [event](api/router/RouteConfigLoadStart) triggered before the `Router` 
+      [lazy loads](#asynchronous-routing) a route configuration.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>RouteConfigLoadStart</code>
+    </td>
+    <td>
+
+      An [event](api/router/RouteConfigLoadStart) triggered after a route has been lazy loaded.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>NavigationEnd</code>
+    </td>
+    <td>
+
+      An [event](api/router/NavigationEnd) triggered when navigation ends successfully.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>NavigationCancel</code>
+    </td>
+    <td>
+
+      An [event](api/router/NavigationCancel) triggered when navigation is canceled. 
+      This is due to a [Route Guard](#guards) returning false during navigation.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>NavigationError</code>
+    </td>
+    <td>
+
+      An [event](api/router/NavigationError) triggered when navigation fails due to an unexpected error.
+
+    </td>
+  </tr>
+</table>
+
+These events are logged to the console when the `enableTracing` option is enabled also. Since the events are provided as an `Observable`, you can `filter()` for events of interest and `subscribe()` to them to make decisions based on the sequence of events in the navigation process.
+
 
 {@a basics-summary}
 
@@ -211,12 +313,6 @@ It has `RouterLink`s that users can click to navigate via the router.
 
 Here are the key `Router` terms and their meanings:
 
-<style>
-  td, th {vertical-align: top}
-</style>
-
-
-
 <table>
 
   <tr>
@@ -369,7 +465,6 @@ Here are the key `Router` terms and their meanings:
 
     <td>
       An Angular component with a <code>RouterOutlet</code> that displays views based on router navigations.
-
     </td>
 
   </tr>
@@ -523,7 +618,7 @@ Modern HTML5 browsers were the first to support `pushState` which is why many pe
 
 
 HTML5 style navigation is the router default.
-In the [LocationStrategy and browser URL styles](guide/router#browser-url-styles) Appendix,
+In the [LocationStrategy and browser URL styles](#browser-url-styles) Appendix,
 learn why HTML5 style is preferred, how to adjust its behavior, and how to switch to the
 older hash (#) style, if necessary.
 
@@ -634,7 +729,7 @@ Once the application is bootstrapped, the `Router` performs the initial navigati
 
 Adding the configured `RouterModule` to the `AppModule` is sufficient for simple route configurations.
 As the application grows, you'll want to refactor the routing configuration into a separate file
-and create a **[Routing Module](guide/router#routing-module)**, a special type of `Service Module` dedicated to the purpose
+and create a **[Routing Module](#routing-module)**, a special type of `Service Module` dedicated to the purpose
 of routing in feature modules.
 
 
@@ -715,7 +810,7 @@ takes a single value bound to the `[fragment]` input binding.
 
 
 
-Learn about the how you can also use the _link parameters array_ in the [appendix below](guide/router#link-parameters-array).
+Learn about the how you can also use the _link parameters array_ in the [appendix below](#link-parameters-array).
 
 
 </div>
@@ -732,7 +827,7 @@ the `RouterLinkActive` directive that look like `routerLinkActive="..."`.
 
 The template expression to the right of the equals (=) contains a space-delimited string of CSS classes
 that the Router will add when this link is active (and remove when the link is inactive).
-You can also set the `RouterLinkActive` directive to a string of classes such as `[routerLinkActive]="active fluffy"`
+You can also set the `RouterLinkActive` directive to a string of classes such as `[routerLinkActive]="'active fluffy'"`
 or bind it to a component property that returns such a string.
 
 The `RouterLinkActive` directive toggles css classes for active `RouterLink`s based on the current `RouterState`.
@@ -769,14 +864,14 @@ Any other URL causes the router to throw an error and crash the app.
 Add a **wildcard** route to intercept invalid URLs and handle them gracefully.
 A _wildcard_ route has a path consisting of two asterisks. It matches _every_ URL.
 The router will select _this_ route if it can't match a route earlier in the configuration.
-A wildcard route can navigate to a custom "404 Not Found" component or [redirect](guide/router#redirect) to an existing route.
+A wildcard route can navigate to a custom "404 Not Found" component or [redirect](#redirect) to an existing route.
 
 
 <div class="l-sub-section">
 
 
 
-The router selects the route with a [_first match wins_](guide/router#example-config) strategy.
+The router selects the route with a [_first match wins_](#example-config) strategy.
 Wildcard routes are the least specific routes in the route configuration.
 Be sure it is the _last_ route in the configuration.
 
@@ -825,24 +920,19 @@ The browser address bar continues to point to the invalid URL.
 
 When the application launches, the initial URL in the browser bar is something like:
 
-
 <code-example>
   localhost:3000
-
 </code-example>
 
+That doesn't match any of the concrete configured routes which means 
+the router falls through to the wildcard route and displays the `PageNotFoundComponent`.
 
-
+The application needs a **default route** to a valid page.
-That doesn't match any of the configured routes which means that the application won't display any component when it's launched.
+The default page for this app is the list of heroes.
-The user must click one of the links to trigger a navigation and display a component.
+The app should navigate there as if the user clicked the "Heroes" link or pasted `localhost:3000/heroes` into the address bar.
-
-It would be nicer if the application had a **default route** that displayed the list of heroes immediately,
-just as it will when the user clicks the "Heroes" link or pastes `localhost:3000/heroes` into the address bar.
-
 
 {@a redirect}
 
-
 ### Redirecting routes
 
 The preferred solution is to add a `redirect` route that translates the initial relative URL (`''`)
@@ -853,7 +943,6 @@ It's just above the wildcard route in the following excerpt showing the complete
 
 
 <code-example path="router/src/app/app-routing.module.1.ts" linenums="false" title="src/app/app-routing.module.ts (appRoutes)" region="appRoutes">
-
 </code-example>
 
 
@@ -1085,8 +1174,8 @@ then replacing `RouterModule.forRoot` in the `imports` array with the `AppRoutin
 
 
 
-Later in this guide you will create [multiple routing modules](guide/router#hero-routing-module) and discover that
+Later in this guide you will create [multiple routing modules](#hero-routing-module) and discover that
-you must import those routing modules [in the correct order](guide/router#routing-module-order).
+you must import those routing modules [in the correct order](#routing-module-order).
 
 
 </div>
@@ -1377,7 +1466,7 @@ The order of route configuration matters.
 The router accepts the first route that matches a navigation request path.
 
 When all routes were in one `AppRoutingModule`,
-you put the default and [wildcard](guide/router#wildcard) routes last, after the `/heroes` route,
+you put the default and [wildcard](#wildcard) routes last, after the `/heroes` route,
 so that the router had a chance to match a URL to the `/heroes` route _before_
 hitting the wildcard route and navigating to "Page not found".
 
@@ -1398,7 +1487,7 @@ will intercept the attempt to navigate to a hero route.
 Reverse the routing modules and see for yourself that
 a click of the heroes link results in "Page not found".
 Learn about inspecting the runtime router configuration
-[below](guide/router#inspect-config "Inspect the router config").
+[below](#inspect-config "Inspect the router config").
 
 
 </div>
@@ -1551,37 +1640,143 @@ The route path and parameters are available through an injected router service c
 [ActivatedRoute](api/router/ActivatedRoute).
 It has a great deal of useful information including:
 
+<table>
+  <tr>
+    <th>
+      Property
+    </th>
+
+    <th>
+      Description
+    </th>
+  </tr>
+
+  <tr>
+    <td>
+      <code>url</code>
+    </td>
+    <td>
+
+    An `Observable` of the route path(s), represented as an array of strings for each part of the route path.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>data</code>
+    </td>
+    <td>
+
+    An `Observable` that contains the `data` object provided for the route. Also contains any resolved values from the [resolve guard](#resolve-guard).
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>paramMap</code>
+    </td>
+    <td>
+
+    An `Observable` that contains a [map](api/router/ParamMap) of the required and [optional parameters](#optional-route-parameters) specific to the route. The map supports retrieving single and multiple values from the same parameter.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>queryParamMap</code>
+    </td>
+    <td>
+
+    An `Observable` that contains a [map](api/router/ParamMap) of the [query parameters](#query-parameters) available to all routes. 
+    The map supports retrieving single and multiple values from the query parameter.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>fragment</code>
+    </td>
+    <td>
+
+    An `Observable` of the URL [fragment](#fragment) available to all routes.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>outlet</code>
+    </td>
+    <td>
+
+    The name of the `RouterOutlet` used to render the route. For an unnamed outlet, the outlet name is _primary_.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>routeConfig</code>
+    </td>
+    <td>
+
+    The route configuration used for the route that contains the origin path.
+
+    </td>
+  </tr>
+
+    <tr>
+    <td>
+      <code>parent</code>
+    </td>
+    <td>
+
+    The route's parent `ActivatedRoute` when this route is a [child route](#child-routing-component).
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>firstChild</code>
+    </td>
+    <td>
+
+    Contains the first `ActivatedRoute` in the list of this route's child routes.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>children</code>
+    </td>
+    <td>
+
+    Contains all the [child routes](#child-routing-component) activated under the current route.
+
+    </td>
+  </tr>
+</table>
 
 <div class="l-sub-section">
 
+Two older properties are still available. They are less capable than their replacements, discouraged, and may be deprecated in a future Angular version.
 
+**`params`** &mdash; An `Observable` that contains the required and [optional parameters](#optional-route-parameters) specific to the route. Use `paramMap` instead.
 
-**`url`**: An `Observable` of the route path(s), represented as an array of strings for each part of the route path.
+**`queryParams`** &mdash; An `Observable` that contains the [query parameters](#query-parameters) available to all routes. 
-
+Use `queryParamMap` instead.
-**`data`**: An `Observable` that contains the `data` object provided for the route. Also contains any resolved values from the [resolve guard](guide/router#resolve-guard).
-
-**`params`**: An `Observable` that contains the required and [optional parameters](guide/router#optional-route-parameters) specific to the route.
-
-**`queryParams`**: An `Observable` that contains the [query parameters](guide/router#query-parameters) available to all routes.
-
-**`fragment`**:  An `Observable` of the URL [fragment](guide/router#fragment) available to all routes.
-
-**`outlet`**: The name of the `RouterOutlet` used to render the route. For an unnamed outlet, the outlet name is _primary_.
-
-**`routeConfig`**: The route configuration used for the route that contains the origin path.
-
-**`parent`**: an `ActivatedRoute` that contains the information from the parent route when using [child routes](guide/router#child-routing-component).
-
-**`firstChild`**: contains the first `ActivatedRoute` in the list of child routes.
-
-**`children`**: contains all the [child routes](guide/router#child-routing-component) activated under the current route.
-
 
 </div>
 
+#### _Activated Route_ in action
 
-
+Import the `Router`, `ActivatedRoute`, and `ParamMap` tokens from the router package.
-Import the `Router`, `ActivatedRoute`, and `Params` tokens from the router package.
 
 
 <code-example path="router/src/app/heroes/hero-detail.component.1.ts" linenums="false" title="src/app/heroes/hero-detail.component.ts (activated route)" region="imports">
@@ -1610,53 +1805,96 @@ that the component requires and reference them as private variables.
 
 </code-example>
 
-
-
 Later, in the `ngOnInit` method, you use the `ActivatedRoute` service to retrieve the parameters for the route,
 pull the hero `id` from the parameters and retrieve the hero to display.
 
 
-<div class="l-sub-section">
-
-
-
-Put this data access logic in the `ngOnInit` method rather than inside the constructor to improve the component's testability.
-Angular calls the `ngOnInit` method shortly after creating an instance of the `HeroDetailComponent`
-so the hero will be retrieved in time to use it.
-
-Learn more about the `ngOnInit` method and other component lifecycle hooks in the [Lifecycle Hooks](guide/lifecycle-hooks) guide.
-
-
-</div>
-
-
-
 <code-example path="router/src/app/heroes/hero-detail.component.ts" linenums="false" title="src/app/heroes/hero-detail.component.ts (ngOnInit)" region="ngOnInit">
 
 </code-example>
 
+The `paramMap` processing is a bit tricky. When the map changes, you `get()` 
+the `id` parameter from the changed parameters.
 
+Then you tell the `HeroService` to fetch the hero with that `id` and return the result of the `HeroService` request. 
 
-Since the parameters are provided as an `Observable`, you use the `switchMap` operator to
+You might think to use the RxJS `map` operator.
-provide them for the `id` parameter by name and tell the `HeroService` to fetch the hero with that `id`.
+But the `HeroService` returns an `Observable<Hero>`.
+Your subscription wants the `Hero`, not an `Observable<Hero>`.
+So you flatten the `Observable` with the `switchMap` operator instead.
 
-The `switchMap` operator allows you to perform an action with the current value of the `Observable`,
+The `switchMap` operator also cancels previous in-flight requests. If the user re-navigates to this route
-and map it to a new `Observable`. As with many `rxjs` operators, `switchMap` handles
+with a new `id` while the `HeroService` is still retrieving the old `id`, `switchMap` discards that old request and returns the hero for the new `id`.
-an `Observable` as well as a `Promise` to retrieve the value they emit.
 
-The `switchMap` operator will also cancel any in-flight requests if the user re-navigates to the route
+Finally, you activate the observable with `subscribe` method and (re)set the component's `hero` property with the retrieved hero.
-while still retrieving a hero.
 
-Use the `subscribe` method to detect `id` changes and to (re)set the retrieved `Hero`.
+#### _ParamMap_ API
 
+The `ParamMap` API is inspired by the [URLSearchParams interface](https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParamsOPut). It provides methods
+to handle parameter access for both route parameters (`paramMap`) and query parameters (`queryParamMap`).
+
+<table>
+  <tr>
+    <th>
+      Member
+    </th>
+
+    <th>
+      Description
+    </th>
+  </tr>
+
+  <tr>
+    <td>
+      <code>has(name)</code>
+    </td>
+    <td>
+
+    Returns `true` if the parameter name is in the map of parameters.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>get(name)</code>
+    </td>
+    <td>
+
+    Returns the parameter name value (a `string`) if present, or `null` if the parameter name is not in the map. Returns the _first_ element if the parameter value is actually an array of values. 
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>getAll(name)</code>
+    </td>
+    <td>
+
+    Returns a `string array` of the parameter name value if found, or an empty `array` if the parameter name value is not in the map. Use `getAll` when a single parameter could have multiple values.
+
+    </td>
+  </tr>
+
+  <tr>
+    <td>
+      <code>keys</code>
+    </td>
+    <td>
+
+    Returns a `string array` of all parameter names in the map.
+
+    </td>
+  </tr>
+</table>
 
 {@a reuse}
 
+#### Observable <i>paramMap</i> and component reuse
 
-#### Observable <i>params</i> and component reuse
+In this example, you retrieve the route parameter map from an `Observable`.
-
+That implies that the route parameter map can change during the lifetime of this component.
-In this example, you retrieve the route params from an `Observable`.
-That implies that the route params can change during the lifetime of this component.
 
 They might. By default, the router re-uses a component instance when it re-navigates to the same component type
 without visiting a different component first. The route parameters could change each time.
@@ -1671,7 +1909,7 @@ Better to simply re-use the same component instance and update the parameter.
 
 Unfortunately, `ngOnInit` is only called once per component instantiation.
 You need a way to detect when the route parameters change from _within the same instance_.
-The observable `params` property handles that beautifully.
+The observable `paramMap` property handles that beautifully.
 
 
 <div class="l-sub-section">
@@ -1706,7 +1944,7 @@ Therefore, the router creates a new `HeroDetailComponent` instance every time.
 When you know for certain that a `HeroDetailComponent` instance will *never, never, ever*
 be re-used, you can simplify the code with the *snapshot*.
 
-The `route.snapshot` provides the initial value of the route parameters.
+The `route.snapshot` provides the initial value of the route parameter map.
 You can access the parameters directly without subscribing or adding observable operators.
 It's much simpler to write and read:
 
@@ -1721,10 +1959,10 @@ It's much simpler to write and read:
 
 
 
-**Remember:** you only get the _initial_ value of the parameters with this technique.
+**Remember:** you only get the _initial_ value of the parameter map with this technique.
-Stick with the observable `params` approach if there's even a chance that the router
+Stick with the observable `paramMap` approach if there's even a chance that the router
 could re-use the component.
-This sample stays with the observable `params` strategy just in case.
+This sample stays with the observable `paramMap` strategy just in case.
 
 
 </div>
@@ -1753,7 +1991,7 @@ It holds the _path to the `HeroListComponent`_:
 
 ### Route Parameters: Required or optional?
 
-Use [*route parameters*](guide/router#route-parameters) to specify a *required* parameter value *within* the route URL
+Use [*route parameters*](#route-parameters) to specify a *required* parameter value *within* the route URL
 as you do when navigating to the `HeroDetailComponent` in order to view the hero with *id* 15:
 
 
@@ -1803,7 +2041,7 @@ prefer an *optional parameter* when the value is optional, complex, and/or multi
 ### Heroes list: optionally selecting a hero
 
 When navigating to the `HeroDetailComponent` you specified the _required_ `id` of the hero-to-edit in the
-*route parameter* and made it the second item of the [_link parameters array_](guide/router#link-parameters-array).
+*route parameter* and made it the second item of the [_link parameters array_](#link-parameters-array).
 
 
 <code-example path="router/src/app/heroes/hero-list.component.1.ts" linenums="false" title="src/app/heroes/hero-list.component.ts (link-parameters-array)" region="link-parameters-array">
@@ -1916,7 +2154,7 @@ The `HeroListComponent` isn't expecting any parameters at all and wouldn't know
 You can change that.
 
 Previously, when navigating from the `HeroListComponent` to the `HeroDetailComponent`,
-you subscribed to the route params `Observable` and made it available to the `HeroDetailComponent`
+you subscribed to the route parameter map `Observable` and made it available to the `HeroDetailComponent`
 in the `ActivatedRoute` service.
 You injected that service in the constructor of the `HeroDetailComponent`.
 
@@ -1931,7 +2169,7 @@ First you extend the router import statement to include the `ActivatedRoute` ser
 
 
 
-Import the `switchMap` operator to perform an operation on the `Observable` of route parameters.
+Import the `switchMap` operator to perform an operation on the `Observable` of route parameter map.
 
 
 <code-example path="router/src/app/heroes/hero-list.component.ts" linenums="false" title="src/app/heroes/hero-list.component.ts (rxjs imports)" region="rxjs-imports">
@@ -1949,21 +2187,8 @@ Then you inject the `ActivatedRoute` in the `HeroListComponent` constructor.
 
 
 
-The `ActivatedRoute.params` property is an `Observable` of route parameters. The `params` emits new `id` values
+The `ActivatedRoute.paramMap` property is an `Observable` map of route parameters. The `paramMap` emits a new map of values that includes `id`
-when the user navigates to the component. In `ngOnInit` you subscribe to those values, set the `selectedId`,
+when the user navigates to the component. In `ngOnInit` you subscribe to those values, set the `selectedId`, and get the heroes.
-and get the heroes.
-
-
-<div class="l-sub-section">
-
-
-
-All route/query parameters are strings.
-The (+) in front of the `params['id']` expression is a JavaScript trick to convert the string to an integer.
-
-</div>
-
-
 
 Add an `isSelected` method that returns `true` when a hero's `id` matches the selected `id`.
 
@@ -2294,13 +2519,9 @@ If your app had many feature areas, the app component trees might look like this
 
 Add the following `crisis-center.component.ts` to the `crisis-center` folder:
 
-
+<code-example path="router/src/app/crisis-center/crisis-center.component.ts" linenums="false" title="src/app/crisis-center/crisis-center.component.ts">
-<code-example path="router/src/app/crisis-center/crisis-center.component.ts" linenums="false" title="src/app/crisis-center/crisis-center.component.ts (minus imports)" region="minus-imports">
-
 </code-example>
 
-
-
 The `CrisisCenterComponent` has the following in common with the `AppComponent`:
 
 * It is the *root* of the crisis center area,
@@ -2322,28 +2543,19 @@ instead you use the router to *navigate* to it.
 
 ### Child route configuration
 
-The `CrisisCenterComponent` is a *routing component* like the `AppComponent`.
+As a host page for the "Crisis Center" feature, add the following `crisis-center-home.component.ts` to the `crisis-center` folder.
-It has its own `RouterOutlet` and its own child routes.
-
-Add the following `crisis-center-home.component.ts` to the `crisis-center` folder.
-
-
-<code-example path="router/src/app/crisis-center/crisis-center-home.component.ts" linenums="false" title="src/app/crisis-center/crisis-center-home.component.ts (minus imports)" region="minus-imports">
 
+<code-example path="router/src/app/crisis-center/crisis-center-home.component.ts" linenums="false" title="src/app/crisis-center/crisis-center-home.component.ts" >
 </code-example>
 
-
-
 Create a `crisis-center-routing.module.ts` file as you did the `heroes-routing.module.ts` file.
 This time, you define **child routes** *within* the parent `crisis-center` route.
 
 
 <code-example path="router/src/app/crisis-center/crisis-center-routing.module.1.ts" linenums="false" title="src/app/crisis-center/crisis-center-routing.module.ts (Routes)" region="routes">
-
 </code-example>
 
 
-
 Notice that the parent `crisis-center` route has a `children` property
 with a single route containing the `CrisisListComponent`. The `CrisisListComponent` route
 also has a `children` array with two routes.
@@ -2359,7 +2571,7 @@ of the `CrisisCenterComponent`, not in the `RouterOutlet` of the `AppComponent`
 The `CrisisListComponent` contains the crisis list and a `RouterOutlet` to
 display the `Crisis Center Home` and `Crisis Detail` route components.
 
-The `Crisis Detail` route is a child of the `Crisis List`. Since the router [reuses components](guide/router#reuse)
+The `Crisis Detail` route is a child of the `Crisis List`. Since the router [reuses components](#reuse)
 by default, the `Crisis Detail` component will be re-used as you select different crises.
 In contrast, back in the `Hero Detail` route, the component was recreated each time you selected a different hero.
 
@@ -2609,7 +2821,7 @@ There are two noteworthy differences.
 Note that the `send()` method simulates latency by waiting a second before "sending" the message and closing the popup.
 
 The `closePopup()` method closes the popup view by navigating to the popup outlet with a `null`.
-That's a peculiarity covered [below](guide/router#clear-secondary-routes).
+That's a peculiarity covered [below](#clear-secondary-routes).
 
 As with other application components, you add the `ComposeMessageComponent` to the `declarations` of an `NgModule`.
 Do so in the `AppModule`.
@@ -2736,7 +2948,7 @@ To see how, look at the `closePopup()` method again:
 
 
 
-It navigates imperatively with the `Router.navigate()` method, passing in a [link parameters array](guide/router#link-parameters-array).
+It navigates imperatively with the `Router.navigate()` method, passing in a [link parameters array](#link-parameters-array).
 
 Like the array bound to the _Contact_ `RouterLink` in the `AppComponent`,
 this one includes an object with an `outlets` property.
@@ -2787,23 +2999,23 @@ These are all asynchronous operations.
 Accordingly, a routing guard can return an `Observable<boolean>` or a `Promise<boolean>` and the
 router will wait for the observable to resolve to `true` or `false`.
 
-The router supports multiple kinds of guards:
+The router supports multiple guard interfaces:
 
-1. [`CanActivate`](api/router/CanActivate) to mediate navigation *to* a route.
+* [`CanActivate`](api/router/CanActivate) to mediate navigation *to* a route.
 
-2. [`CanActivateChild()`](api/router/CanActivateChild) to mediate navigation *to* a child route.
+* [`CanActivateChild`](api/router/CanActivateChild) to mediate navigation *to* a child route.
 
-3. [`CanDeactivate`](api/router/CanDeactivate) to mediate navigation *away* from the current route.
+* [`CanDeactivate`](api/router/CanDeactivate) to mediate navigation *away* from the current route.
 
-4. [`Resolve`](api/router/Resolve) to perform route data retrieval *before* route activation.
+* [`Resolve`](api/router/Resolve) to perform route data retrieval *before* route activation.
 
-5. [`CanLoad`](api/router/CanLoad) to mediate navigation *to* a feature module loaded _asynchronously_.
+* [`CanLoad`](api/router/CanLoad) to mediate navigation *to* a feature module loaded _asynchronously_.
 
 
 You can have multiple guards at every level of a routing hierarchy.
-The router checks the `CanDeactivate()` and `CanActivateChild()` guards first, from the deepest child route to the top.
+The router checks the `CanDeactivate` and `CanActivateChild` guards first, from the deepest child route to the top.
-Then it checks the `CanActivate()` guards from the top down to the deepest child route. If the feature module
+Then it checks the `CanActivate` guards from the top down to the deepest child route. If the feature module
-is loaded asynchronously, the `CanLoad()` guard is checked before the module is loaded.
+is loaded asynchronously, the `CanLoad` guard is checked before the module is loaded.
 If _any_ guard returns false, pending guards that have not completed will be canceled,
 and the entire navigation is canceled.
 
@@ -2936,7 +3148,7 @@ You've defined a _component-less_ route.
 
 The goal is to group the `Crisis Center` management routes under the `admin` path.
 You don't need a component to do it.
-A _component-less_ route makes it easier to [guard child routes](guide/router#can-activate-child-guard).
+A _component-less_ route makes it easier to [guard child routes](#can-activate-child-guard).
 
 
 Next, import the `AdminModule` into `app.module.ts` and add it to the `imports` array
@@ -2968,7 +3180,7 @@ The new *admin* feature should be accessible only to authenticated users.
 
 You could hide the link until the user logs in. But that's tricky and difficult to maintain.
 
-Instead you'll write a `CanActivate()` guard to redirect anonymous users to the
+Instead you'll write a `canActivate()` guard method to redirect anonymous users to the
 login page when they try to enter the admin area.
 
 This is a general purpose guard&mdash;you can imagine other features
@@ -2986,7 +3198,7 @@ It simply logs to console and `returns` true immediately, allowing navigation to
 
 
 Next, open `admin-routing.module.ts `, import the `AuthGuard` class, and
-update the admin route with a `CanActivate()` guard property that references it:
+update the admin route with a `canActivate` guard property that references it:
 
 
 <code-example path="router/src/app/admin/admin-routing.module.2.ts" linenums="false" title="src/app/admin/admin-routing.module.ts (guarded admin route)" region="admin-route">
@@ -3083,7 +3295,7 @@ Import and add the `LoginRoutingModule` to the `AppModule` imports as well.
 
 Guards and the service providers they require _must_ be provided at the module-level. This allows
 the Router access to retrieve these services from the `Injector` during the navigation process.
-The same rule applies for feature modules loaded [asynchronously](guide/router#asynchronous-routing).
+The same rule applies for feature modules loaded [asynchronously](#asynchronous-routing).
 
 
 </div>
@@ -3105,9 +3317,9 @@ You should also protect child routes _within_ the feature module.
 Extend the `AuthGuard` to protect when navigating between the `admin` routes.
 Open `auth-guard.service.ts` and add the `CanActivateChild` interface to the imported tokens from the router package.
 
-Next, implement the `CanActivateChild` method which takes the same arguments as the `CanActivate` method:
+Next, implement the `canActivateChild()` method which takes the same arguments as the `canActivate()` method:
 an `ActivatedRouteSnapshot` and `RouterStateSnapshot`.
-The `CanActivateChild` method can return an `Observable<boolean>` or `Promise<boolean>` for
+The `canActivateChild()` method can return an `Observable<boolean>` or `Promise<boolean>` for
 async checks and a `boolean` for sync checks.
 This one returns a `boolean`:
 
@@ -3211,7 +3423,7 @@ to discard changes and navigate away (`true`) or to preserve the pending changes
 {@a CanDeactivate}
 
 
-Create a _guard_ that checks for the presence of a `canDeactivate` method in a component&mdash;any component.
+Create a _guard_ that checks for the presence of a `canDeactivate()` method in a component&mdash;any component.
 The `CrisisDetailComponent` will have this method.
 But the guard doesn't have to know that.
 The guard shouldn't know the details of any component's deactivation method.
@@ -3249,13 +3461,13 @@ Looking back at the `CrisisDetailComponent`, it implements the confirmation work
 
 
 
-Notice that the `canDeactivate` method *can* return synchronously;
+Notice that the `canDeactivate()` method *can* return synchronously;
 it returns `true` immediately if there is no crisis or there are no pending changes.
 But it can also return a `Promise` or an `Observable` and the router will wait for that
 to resolve to truthy (navigate) or falsy (stay put).
 
 
-Add the `Guard` to the crisis detail route in `crisis-center-routing.module.ts` using the `canDeactivate` array.
+Add the `Guard` to the crisis detail route in `crisis-center-routing.module.ts` using the `canDeactivate` array property.
 
 
 <code-example path="router/src/app/crisis-center/crisis-center-routing.module.3.ts" linenums="false" title="src/app/crisis-center/crisis-center-routing.module.ts (can deactivate guard)">
@@ -3431,7 +3643,7 @@ The relevant *Crisis Center* code for this milestone follows.
 
 ### Query parameters and fragments
 
-In the [route parameters](guide/router#optional-route-parameters) example, you only dealt with parameters specific to
+In the [route parameters](#optional-route-parameters) example, you only dealt with parameters specific to
 the route, but what if you wanted optional parameters available to all routes?
 This is where query parameters come into play.
 
@@ -3454,7 +3666,7 @@ Add the `NavigationExtras` object to the `router.navigate` method that navigates
 You can also preserve query parameters and fragments across navigations without having to provide them
 again when navigating. In the `LoginComponent`, you'll add an *object* as the
 second argument in the `router.navigate` function
-and provide the `preserveQueryParams` and `preserveFragment` to pass along the current query parameters
+and provide the `queryParamsHandling` and `preserveFragment` to pass along the current query parameters
 and fragment to the next route.
 
 
@@ -3462,6 +3674,15 @@ and fragment to the next route.
 
 </code-example>
 
+<div class="l-sub-section">
+
+
+The `queryParamsHandling` feature also provides a `merge` option, which will preserve and combine the current query parameters with any provided query parameters
+when navigating.
+
+
+</div>
+
 
 
 Since you'll be navigating to the *Admin Dashboard* route after logging in, you'll update it to handle the
@@ -3474,14 +3695,14 @@ query parameters and fragment.
 
 
 
-*Query Parameters* and *Fragments* are also available through the `ActivatedRoute` service.
+*Query parameters* and *fragments* are also available through the `ActivatedRoute` service.
 Just like *route parameters*, the query parameters and fragments are provided as an `Observable`.
 The updated *Crisis Admin* component feeds the `Observable` directly into the template using the `AsyncPipe`.
 
 
 Now, you can click on the *Admin* button, which takes you to the *Login*
-page with the provided `query params` and `fragment`. After you click the login button, notice that
+page with the provided `queryParamMap` and `fragment`. After you click the login button, notice that
-you have been redirected to the `Admin Dashboard` page with the `query params` and `fragment` still intact.
+you have been redirected to the `Admin Dashboard` page with the query parameters and fragment still intact in the address bar.
 
 You can use these persistent bits of information for things that need to be provided across pages like
 authentication tokens or session ids.
@@ -3492,7 +3713,7 @@ authentication tokens or session ids.
 
 
 The `query params` and `fragment` can also be preserved using a `RouterLink` with
-the `preserveQueryParams` and `preserveFragment` bindings respectively.
+the `queryParamsHandling` and `preserveFragment` bindings respectively.
 
 
 </div>
@@ -3596,7 +3817,7 @@ its `checkLogin()` method to support the `CanLoad` guard.
 Open `auth-guard.service.ts`.
 Import the `CanLoad` interface from `@angular/router`.
 Add it to the `AuthGuard` class's `implements` list.
-Then implement `canLoad` as follows:
+Then implement `canLoad()` as follows:
 
 
 <code-example path="router/src/app/auth-guard.service.ts" linenums="false" title="src/app/auth-guard.service.ts (CanLoad guard)" region="canLoad">
@@ -3609,7 +3830,7 @@ The router sets the `canLoad()` method's `route` parameter to the intended desti
 The `checkLogin()` method redirects to that URL once the user has logged in.
 
 Now import the `AuthGuard` into the `AppRoutingModule` and add the `AuthGuard` to the `canLoad`
-array for the `admin` route.
+array property for the `admin` route.
 The completed admin route looks like this:
 
 
@@ -3660,7 +3881,7 @@ The `Router` offers two preloading strategies out of the box:
 * Preloading of all lazy loaded feature areas.
 
 Out of the box, the router either never preloads, or preloads every lazy load module.
-The `Router` also supports [custom preloading strategies](guide/router#custom-preloading) for
+The `Router` also supports [custom preloading strategies](#custom-preloading) for
 fine control over which modules to preload and when.
 
 In this next section, you'll update the `CrisisCenterModule` to load lazily
@@ -3732,7 +3953,7 @@ Surprisingly, the `AdminModule` does _not_ preload. Something is blocking it.
 
 #### CanLoad blocks preload
 
-The `PreloadAllModules` strategy does not load feature areas protected by a [CanLoad](guide/router#can-load-guard) guard.
+The `PreloadAllModules` strategy does not load feature areas protected by a [CanLoad](#can-load-guard) guard.
 This is by design.
 
 You added a `CanLoad` guard to the route in the `AdminModule` a few steps back
@@ -3740,7 +3961,7 @@ to block loading of that module until the user is authorized.
 That `CanLoad` guard takes precedence over the preload strategy.
 
 If you want to preload a module _and_ guard against unauthorized access,
-drop the `canLoad` guard and rely on the [CanActivate](guide/router#can-activate-guard) guard alone.
+drop the `canLoad()` guard method and rely on the [canActivate()](#can-activate-guard) guard alone.
 
 
 {@a custom-preloading}
@@ -3827,7 +4048,7 @@ It's also logged to the browser's console.
 ## Inspect the router's configuration
 
 You put a lot of effort into configuring the router in several routing module files
-and were careful to list them [in the proper order](guide/router#routing-module-order).
+and were careful to list them [in the proper order](#routing-module-order).
 Are routes actually evaluated as you planned?
 How is the router really configured?
 
@@ -3841,11 +4062,9 @@ to see the finished route configuration.
 </code-example>
 
 
-
 {@a final-app}
 
 
-
 ## Wrap up and final app
 
 You've covered a lot of ground in this guide and the application is too big to reprint here.
@@ -4084,4 +4303,3 @@ in the `AppModule`.
 <code-example path="router/src/app/app.module.6.ts" linenums="false" title="src/app/app.module.ts (hash URL strategy)">
 
 </code-example>
-

aio/content/guide/setup-systemjs-anatomy.md

@@ -317,7 +317,7 @@ If you do, this page can help you understand their purpose.
 
 
       Optional extra SystemJS configuration.
-      A way to add SystemJS mappings, such as for appliation _barrels_,
+      A way to add SystemJS mappings, such as for application _barrels_,
       without changing the original `system.config.js`.
     </td>
 

aio/content/guide/setup.md

@@ -34,7 +34,7 @@ Perform the _clone-to-launch_ steps with these terminal commands.
 
 
 
-`npm start` fails in _Bash for Windows_ which does not support networking to servers as of January, 2017.
+`npm start` fails in _Bash for Windows_ in versions earlier than the Creator's Update (April 2017).
 
 
 </div>
@@ -62,7 +62,7 @@ and unzip it into your project folder. Then perform the remaining steps with the
 
 
 
-`npm start` fails in _Bash for Windows_ which does not support networking to servers as of January, 2017.
+`npm start` fails in _Bash for Windows_ in versions earlier than the Creator's Update (April 2017).
 
 
 </div>

aio/content/guide/testing.md

@@ -742,7 +742,7 @@ before calling `TestBed.createComponent` to instantiate the _component-under-tes
 
 
 
-Calling `compileComponents` closes the current `TestBed` instance is further configuration.
+Calling `compileComponents` closes the current `TestBed` instance to further configuration.
 You cannot call any more `TestBed` configuration methods, not `configureTestingModule`
 nor any of the `override...` methods. The `TestBed` throws an error if you try.
 

aio/content/guide/typescript-configuration.md

@@ -34,6 +34,8 @@ For details about `tsconfig.json`, see the official
 
 The [Setup](guide/setup) guide uses the following `tsconfig.json`:
 
+<code-example path="quickstart/src/tsconfig.1.json" title="tsconfig.json" linenums="false"></code-example>
+
 This file contains options and flags that are essential for Angular applications.
 
 

aio/content/guide/upgrade.md

@@ -1023,7 +1023,7 @@ a successful upgrade.
 Since you're going to be writing Angular code in TypeScript, it makes sense to
 bring in the TypeScript compiler even before you begin upgrading.
 
-You'll  also start to gradually phase out the Bower package manager in favor
+You'll also start to gradually phase out the Bower package manager in favor
 of NPM, installing all new dependencies using NPM, and eventually removing Bower from the project.
 
 Begin by installing TypeScript to the project.
@@ -1041,11 +1041,21 @@ Jasmine unit test framework.
 </code-example>
 
 You should also configure the TypeScript compiler with a `tsconfig.json` in the project directory
-as described in the [Setup](guide/setup) guide.
+as described in the [TypeScript Configuration](guide/typescript-configuration) guide.
 The `tsconfig.json` file tells the TypeScript compiler how to turn your TypeScript files
 into ES5 code bundled into CommonJS modules.
 
-Now launch the TypeScript compiler from the command line in watch mode.
+Finally, you should add some npm scripts in `package.json` to compile the TypeScript files to
+JavaScript (based on the `tsconfig.json` configuration file):
+
+<code-example format="">
+  "script": {
+    "tsc": "tsc",
+    "tsc:w": "tsc -w",
+    ...
+</code-example>
+
+Now launch the TypeScript compiler from the command line in watch mode:
 
 <code-example format="">
   npm run tsc:w
@@ -1192,6 +1202,10 @@ Move the `app/index.html` file to the project root directory. Then change the
 development server root path in `package.json` to also point to the project root
 instead of `app`:
 
+<code-example format="">
+  "start": "http-server ./ -a localhost -p 8000 -c-1",
+</code-example>
+
 Now you're able to serve everything from the project root to the web browser. But you do *not*
 want to have to change all the image and data paths used in the application code to match
 the development setup. For that reason, you'll add a `<base>` tag to `index.html`, which will

aio/content/marketing/resources.json

@@ -105,7 +105,7 @@
         "order": 3,
         "resources": {
           "-KLIzHDRfiB3d7W7vk-e": {
-            "desc": "Reactive Extensions for angular2",
+            "desc": "Reactive Extensions for Angular",
             "rev": true,
             "title": "ngrx",
             "url": "http://github.com/ngrx"
@@ -214,6 +214,13 @@
             "rev": true,
             "title": "Universal for ASP.NET",
             "url": "https://github.com/aspnet/nodeservices"
+          },
+          "f1": {
+            "desc": "This tool generates dedicated documentation for Angular applications.",
+            "logo": "",
+            "rev": true,
+            "title": "Compodoc",
+            "url": "https://github.com/compodoc/compodoc"
           }
         }
       },
@@ -246,7 +253,7 @@
             "url": "http://ng-lightning.github.io/ng-lightning/"
           },
           "7ab": {
-            "desc": "UI components for hybrid mobile apps with bindings for both Angular 1 & 2.",
+            "desc": "UI components for hybrid mobile apps with bindings for both Angular & AngularJS.",
             "rev": true,
             "title": "Onsen UI",
             "url": "https://onsen.io/v2/"
@@ -280,11 +287,11 @@
             "url": "https://vaadin.com/elements"
           },
           "a7b": {
-            "desc": "Native Angular2 directives for Bootstrap",
+            "desc": "Native Angular directives for Bootstrap",
             "logo": "",
             "rev": true,
-            "title": "ng2-bootstrap",
+            "title": "ngx-bootstrap",
-            "url": "http://valor-software.com/ng2-bootstrap/"
+            "url": "http://valor-software.com/ngx-bootstrap/#/"
           },
           "ab": {
             "desc": "Material Design components for Angular",
@@ -298,7 +305,13 @@
             "rev": true,
             "title": "ag-Grid",
             "url": "https://www.ag-grid.com/best-angular-2-data-grid/"
-          }
+          },
+          "jqwidgets": {
+            "desc": "Angular UI Components including Grids, Charts, Scheduling and more.",
+            "rev": true,
+            "title": "jQWidgets",
+            "url": "https://www.jqwidgets.com/angular/"
+          }          
         }
       }
     }
@@ -348,7 +361,7 @@
           "a5b": {
             "desc": "The in-depth, complete, and up-to-date book on Angular. Become an Angular expert today.",
             "rev": true,
-            "title": "ng-book 2",
+            "title": "ng-book",
             "url": "https://www.ng-book.com/2/"
           },
           "a6b": {
@@ -417,7 +430,7 @@
             "url": "https://www.eduonix.com/courses/Web-Development/angular-2-fundamentals-for-web-developers"
           },
           "-KMUuOWwciL_S_o0fzFO": {
-            "desc": "The ngmigrate project is brought to you by Todd Motto, a Developer Advocate at Telerik, spreading the good word of Kendo UI, NativeScript and Angular 1 & 2. You can follow him on Twitter for questions, or even requests about this guide.",
+            "desc": "The ngMigrate project is brought to you by Todd Motto, a Developer Advocate at Telerik, spreading the good word of Kendo UI, NativeScript and Angular & AngularJS. You can follow him on Twitter for questions, or even requests about this guide.",
             "rev": true,
             "title": "ngMigrate",
             "url": "http://ngmigrate.telerik.com/"
@@ -466,7 +479,7 @@
             "url": "https://frontendmasters.com/courses/angular-2/"
           },
           "ac6": {
-            "desc": "French language Angular course covering TypeScript, ES6, Depdendency Injection, Observables, and more.",
+            "desc": "French language Angular course covering TypeScript, ES6, Dependency Injection, Observables, and more.",
             "rev": true,
             "title": "Wishtack's Angular Course (francais)",
             "url": "http://courses.wishtack.com/angular-2/ecmascript-6"
@@ -519,7 +532,7 @@
             "url": "http://chariotsolutions.com/course/angular2-workshop-fundamentals-architecture/"
           },
           "-KLIBoN0p9be3kwC6-ga": {
-            "desc": "Angular Academy is a 2 day hands-on public course given in-person across Canada!",
+            "desc": "Angular Academy is a two day hands-on public course given in-person across Canada!",
             "rev": true,
             "title": "Angular Academy (Canada)",
             "url": "http://www.angularacademy.ca"
@@ -574,6 +587,13 @@
             "rev": true,
             "title": "Thoughtram",
             "url": "http://thoughtram.io/"
+          },
+          "jsru": {
+            "desc": "Complete Angular online course. Constantly updating. Real-time webinars with immediate feedback from the teacher.",
+            "logo": "https://learn.javascript.ru/img/sitetoolbar__logo_ru.svg",
+            "rev": true,
+            "title": "Learn Javascript (Russian)",
+            "url": "https://learn.javascript.ru/courses/angular"
           }
         }
       }

aio/content/tutorial/toh-pt4.md

@@ -420,7 +420,7 @@ The service does that work and eventually calls the function with the results or
 
 This is a simplified explanation. Read more about ES2015 Promises in the
 [Promises for asynchronous programming](http://exploringjs.com/es6/ch_promises.html) page of
-[Exploring ES6](http://http://exploringjs.com/es6.html).
+[Exploring ES6](http://exploringjs.com/es6.html).
 
 
 </div>