From b51e1c986787c1bb2ecd93dd12c909013178f1f1 Mon Sep 17 00:00:00 2001
From: Wyatt Johnson <wyattjoh@gmail.com>
Date: Tue, 26 Jun 2018 14:46:58 -0600
Subject: [PATCH] fix: added support for strictNullChecks

---
 src/core/server/app/middleware/tenant.ts      | 29 ++++++++++++
 src/core/server/app/router.ts                 |  8 ++--
 .../server/graph/common/scalars/cursor.ts     |  2 +-
 src/core/server/graph/tenant/context.ts       |  4 +-
 .../server/graph/tenant/loaders/assets.ts     |  2 +-
 src/core/server/graph/tenant/loaders/users.ts |  2 +-
 src/core/server/graph/tenant/middleware.ts    | 17 +++----
 .../server/graph/tenant/mutators/comment.ts   |  5 ++-
 .../server/graph/tenant/resolvers/query.ts    |  6 +--
 src/core/server/models/asset.ts               | 14 +++---
 src/core/server/models/comment.ts             |  6 +--
 src/core/server/models/connection.ts          |  2 +-
 src/core/server/models/tenant.ts              | 21 +++++----
 src/core/server/models/user.ts                | 12 ++---
 src/core/server/services/tenant/cache.ts      |  4 +-
 src/types/{passport.d.ts => express.d.ts}     |  2 +
 tsconfig.json                                 | 45 ++++++++++---------
 17 files changed, 104 insertions(+), 77 deletions(-)
 create mode 100644 src/core/server/app/middleware/tenant.ts
 rename src/types/{passport.d.ts => express.d.ts} (62%)

diff --git a/src/core/server/app/middleware/tenant.ts b/src/core/server/app/middleware/tenant.ts
new file mode 100644
index 000000000..26c13e73a
--- /dev/null
+++ b/src/core/server/app/middleware/tenant.ts
@@ -0,0 +1,29 @@
+import { NextFunction, Request, Response } from "express";
+import { Db } from "mongodb";
+import { retrieveTenantByDomain } from "talk-server/models/tenant";
+
+export interface MiddlewareOptions {
+  db: Db;
+}
+
+export default (options: MiddlewareOptions) => async (
+  req: Request,
+  res: Response,
+  next: NextFunction
+) => {
+  try {
+    // TODO: replace with shared synced cache instead of direct db access.
+    const tenant = await retrieveTenantByDomain(options.db, req.hostname);
+    if (!tenant) {
+      // TODO: send a http.StatusNotFound?
+      return next(new Error("not found"));
+    }
+
+    // Attach the tenant to the request.
+    req.tenant = tenant;
+
+    next();
+  } catch (err) {
+    next(err);
+  }
+};
diff --git a/src/core/server/app/router.ts b/src/core/server/app/router.ts
index e50e42b15..fd481ee5c 100644
--- a/src/core/server/app/router.ts
+++ b/src/core/server/app/router.ts
@@ -1,6 +1,6 @@
 import express from "express";
-import passport from "passport";
 
+import tenantMiddleware from "talk-server/app/middleware/tenant";
 import managementGraphMiddleware from "talk-server/graph/management/middleware";
 import tenantGraphMiddleware from "talk-server/graph/tenant/middleware";
 
@@ -27,6 +27,9 @@ async function createManagementRouter(opts: AppOptions) {
 async function createTenantRouter(opts: AppOptions) {
   const router = express.Router();
 
+  // Tenant identification middleware.
+  router.use(tenantMiddleware({ db: opts.mongo }));
+
   // Tenant API
   router.use(
     "/graphql",
@@ -41,9 +44,6 @@ async function createAPIRouter(opts: AppOptions) {
   // Create a router.
   const router = express.Router();
 
-  // Setup Passport.
-  router.use(passport.initialize());
-
   // Configure the tenant routes.
   router.use("/tenant", await createTenantRouter(opts));
 
diff --git a/src/core/server/graph/common/scalars/cursor.ts b/src/core/server/graph/common/scalars/cursor.ts
index c48cd0b65..23649e10b 100644
--- a/src/core/server/graph/common/scalars/cursor.ts
+++ b/src/core/server/graph/common/scalars/cursor.ts
@@ -3,7 +3,7 @@ import { Kind } from "graphql/language";
 import { DateTime } from "luxon";
 import { Cursor } from "talk-server/models/connection";
 
-function parseIntegerCursor(value: string): number {
+function parseIntegerCursor(value: string): number | null {
   try {
     const cursor = parseInt(value, 10);
 
diff --git a/src/core/server/graph/tenant/context.ts b/src/core/server/graph/tenant/context.ts
index 18c6cee88..c49b3d49a 100644
--- a/src/core/server/graph/tenant/context.ts
+++ b/src/core/server/graph/tenant/context.ts
@@ -6,7 +6,7 @@ import mutators from "./mutators";
 
 export interface TenantContextOptions {
   db: Db;
-  tenant?: Tenant;
+  tenant: Tenant;
   user?: User;
 }
 
@@ -14,7 +14,7 @@ export default class TenantContext {
   public loaders: ReturnType<typeof loaders>;
   public mutators: ReturnType<typeof mutators>;
   public db: Db;
-  public tenant?: Tenant;
+  public tenant: Tenant;
   public user?: User;
 
   constructor({ user, tenant, db }: TenantContextOptions) {
diff --git a/src/core/server/graph/tenant/loaders/assets.ts b/src/core/server/graph/tenant/loaders/assets.ts
index a468db9e9..55c50a0dc 100644
--- a/src/core/server/graph/tenant/loaders/assets.ts
+++ b/src/core/server/graph/tenant/loaders/assets.ts
@@ -3,7 +3,7 @@ import TenantContext from "talk-server/graph/tenant/context";
 import { Asset, retrieveManyAssets } from "talk-server/models/asset";
 
 export default (ctx: TenantContext) => ({
-  asset: new DataLoader<string, Asset>(ids =>
+  asset: new DataLoader<string, Asset | null>(ids =>
     retrieveManyAssets(ctx.db, ctx.tenant.id, ids)
   ),
 });
diff --git a/src/core/server/graph/tenant/loaders/users.ts b/src/core/server/graph/tenant/loaders/users.ts
index a7b9f5aa7..d875bc6b4 100644
--- a/src/core/server/graph/tenant/loaders/users.ts
+++ b/src/core/server/graph/tenant/loaders/users.ts
@@ -3,7 +3,7 @@ import Context from "talk-server/graph/tenant/context";
 import { retrieveMany, User } from "talk-server/models/user";
 
 export default (ctx: Context) => ({
-  user: new DataLoader<string, User>(ids =>
+  user: new DataLoader<string, User | null>(ids =>
     retrieveMany(ctx.db, ctx.tenant.id, ids)
   ),
 });
diff --git a/src/core/server/graph/tenant/middleware.ts b/src/core/server/graph/tenant/middleware.ts
index a98dfa563..776b8eca7 100644
--- a/src/core/server/graph/tenant/middleware.ts
+++ b/src/core/server/graph/tenant/middleware.ts
@@ -1,28 +1,21 @@
+import { Request } from "express";
 import { GraphQLSchema } from "graphql";
 import { Db } from "mongodb";
 
 import { Config } from "talk-server/config";
 import { graphqlMiddleware } from "talk-server/graph/common/middleware";
-import { createPubSub } from "talk-server/graph/common/subscriptions/pubsub";
-import { retrieveTenantByDomain } from "talk-server/models/tenant";
 
 import TenantContext from "./context";
 
 export default async (schema: GraphQLSchema, config: Config, db: Db) => {
-  // Configure the PubSub broker.
-  const pubsub = await createPubSub(config);
-
-  return graphqlMiddleware(config, async req => {
-    // TODO: replace with shared synced cache instead of direct db access.
-    const tenant = await retrieveTenantByDomain(db, req.hostname);
-
-    // Load the user from the request.
-    const user = req.user;
+  return graphqlMiddleware(config, async (req: Request) => {
+    // Load the tenant and user from the request.
+    const { tenant, user } = req;
 
     // Return the graph options.
     return {
       schema,
-      context: new TenantContext({ db, tenant, user }),
+      context: new TenantContext({ db, tenant: tenant!, user }),
     };
   });
 };
diff --git a/src/core/server/graph/tenant/mutators/comment.ts b/src/core/server/graph/tenant/mutators/comment.ts
index f51deecb0..1046930b9 100644
--- a/src/core/server/graph/tenant/mutators/comment.ts
+++ b/src/core/server/graph/tenant/mutators/comment.ts
@@ -5,8 +5,9 @@ import { create } from "talk-server/services/comments";
 
 export default (ctx: TenantContext) => ({
   create: (input: CreateCommentInput): Promise<Comment> => {
-    return create(ctx.db, ctx.tenant.id, {
-      author_id: ctx.user.id,
+    // FIXME: remove tenant + user !
+    return create(ctx.db, ctx.tenant!.id, {
+      author_id: ctx.user!.id,
       asset_id: input.assetID,
       body: input.body,
       parent_id: input.parentID,
diff --git a/src/core/server/graph/tenant/resolvers/query.ts b/src/core/server/graph/tenant/resolvers/query.ts
index f4ea10b4b..c79e1af1c 100644
--- a/src/core/server/graph/tenant/resolvers/query.ts
+++ b/src/core/server/graph/tenant/resolvers/query.ts
@@ -4,10 +4,8 @@ import { Asset } from "talk-server/models/asset";
 export default {
   asset: async (
     source: void,
-    { id, url }: { id?: string; url: string },
+    { id, url }: { id: string; url: string },
     ctx: TenantContext
-  ): Promise<Asset> => {
-    return ctx.loaders.Assets.asset.load(id);
-  },
+  ) => ctx.loaders.Assets.asset.load(id),
   settings: async (parent: any, args: any, ctx: TenantContext) => ctx.tenant,
 };
diff --git a/src/core/server/models/asset.ts b/src/core/server/models/asset.ts
index 34e1df8ca..6fc5c55c9 100644
--- a/src/core/server/models/asset.ts
+++ b/src/core/server/models/asset.ts
@@ -33,7 +33,7 @@ export async function createAsset(
   db: Db,
   tenantID: string,
   input: CreateAssetInput
-): Promise<Asset> {
+): Promise<Readonly<Asset> | null> {
   const now = new Date();
 
   // Construct the filter.
@@ -66,14 +66,14 @@ export async function createAsset(
       returnOriginal: false,
     });
 
-  return result.value;
+  return result.value || null;
 }
 
 export async function retrieveAsset(
   db: Db,
   tenantID: string,
   id: string
-): Promise<Asset> {
+): Promise<Asset | null> {
   return await db
     .collection<Asset>("assets")
     .findOne({ id, tenant_id: tenantID });
@@ -83,14 +83,14 @@ export async function retrieveManyAssets(
   db: Db,
   tenantID: string,
   ids: string[]
-): Promise<Asset[]> {
+): Promise<Array<Asset | null>> {
   const cursor = await db
     .collection<Asset>("assets")
     .find({ id: { $in: ids }, tenant_id: tenantID });
 
   const assets = await cursor.toArray();
 
-  return ids.map(id => assets.find(asset => asset.id === id));
+  return ids.map(id => assets.find(asset => asset.id === id) || null);
 }
 
 export type UpdateAssetInput = Omit<
@@ -103,7 +103,7 @@ export async function updateAsset(
   tenantID: string,
   id: string,
   update: UpdateAssetInput
-): Promise<Readonly<Asset>> {
+): Promise<Readonly<Asset> | null> {
   const result = await db.collection<Asset>("assets").findOneAndUpdate(
     { id, tenant_id: tenantID },
     // Only update fields that have been updated.
@@ -113,5 +113,5 @@ export async function updateAsset(
     { returnOriginal: false }
   );
 
-  return result.value;
+  return result.value || null;
 }
diff --git a/src/core/server/models/comment.ts b/src/core/server/models/comment.ts
index 951fcee97..058f3799a 100644
--- a/src/core/server/models/comment.ts
+++ b/src/core/server/models/comment.ts
@@ -108,7 +108,7 @@ export async function retrieve(
   db: Db,
   tenantID: string,
   id: string
-): Promise<Readonly<Comment>> {
+): Promise<Readonly<Comment> | null> {
   return collection(db).findOne({ id, tenant_id: tenantID });
 }
 
@@ -116,7 +116,7 @@ export async function retrieveMany(
   db: Db,
   tenantID: string,
   ids: string[]
-): Promise<Array<Readonly<Comment>>> {
+): Promise<Array<Readonly<Comment> | null>> {
   const cursor = await collection(db).find({
     id: {
       $in: ids,
@@ -126,7 +126,7 @@ export async function retrieveMany(
 
   const comments = await cursor.toArray();
 
-  return ids.map(id => comments.find(comment => comment.id === id));
+  return ids.map(id => comments.find(comment => comment.id === id) || null);
 }
 
 export enum CommentSort {
diff --git a/src/core/server/models/connection.ts b/src/core/server/models/connection.ts
index eb687c255..9ed8cac2b 100644
--- a/src/core/server/models/connection.ts
+++ b/src/core/server/models/connection.ts
@@ -1,4 +1,4 @@
-export type Cursor = Date | number | string;
+export type Cursor = Date | number | string | null;
 
 export interface Edge<T> {
   node: T;
diff --git a/src/core/server/models/tenant.ts b/src/core/server/models/tenant.ts
index 11a2fac04..44a7d316b 100644
--- a/src/core/server/models/tenant.ts
+++ b/src/core/server/models/tenant.ts
@@ -116,18 +116,21 @@ export async function createTenant(
 export async function retrieveTenantByDomain(
   db: Db,
   domain: string
-): Promise<Readonly<Tenant>> {
+): Promise<Readonly<Tenant> | null> {
   return collection(db).findOne({ domain });
 }
 
-export async function retrieve(db: Db, id: string): Promise<Readonly<Tenant>> {
+export async function retrieve(
+  db: Db,
+  id: string
+): Promise<Readonly<Tenant> | null> {
   return collection(db).findOne({ id });
 }
 
 export async function retrieveManyTenants(
   db: Db,
   ids: string[]
-): Promise<Array<Readonly<Tenant>>> {
+): Promise<Array<Readonly<Tenant> | null>> {
   const cursor = await collection(db).find({
     id: {
       $in: ids,
@@ -136,13 +139,13 @@ export async function retrieveManyTenants(
 
   const tenants = await cursor.toArray();
 
-  return ids.map(id => tenants.find(tenant => tenant.id === id));
+  return ids.map(id => tenants.find(tenant => tenant.id === id) || null);
 }
 
 export async function retrieveManyTenantsByDomain(
   db: Db,
   domains: string[]
-): Promise<Array<Readonly<Tenant>>> {
+): Promise<Array<Readonly<Tenant> | null>> {
   const cursor = await collection(db).find({
     domain: {
       $in: domains,
@@ -151,8 +154,8 @@ export async function retrieveManyTenantsByDomain(
 
   const tenants = await cursor.toArray();
 
-  return domains.map(domain =>
-    tenants.find(tenant => tenant.domain === domain)
+  return domains.map(
+    domain => tenants.find(tenant => tenant.domain === domain) || null
   );
 }
 
@@ -168,7 +171,7 @@ export async function updateTenant(
   db: Db,
   id: string,
   update: Partial<CreateTenantInput>
-): Promise<Readonly<Tenant>> {
+): Promise<Readonly<Tenant> | null> {
   // Get the tenant from the database.
   const result = await collection(db).findOneAndUpdate(
     { id },
@@ -179,5 +182,5 @@ export async function updateTenant(
     { returnOriginal: false }
   );
 
-  return result.value;
+  return result.value || null;
 }
diff --git a/src/core/server/models/user.ts b/src/core/server/models/user.ts
index fba955e24..14182c666 100644
--- a/src/core/server/models/user.ts
+++ b/src/core/server/models/user.ts
@@ -67,7 +67,7 @@ export interface UserStatusItem<T> {
 export interface UserStatus {
   username: UserStatusItem<UserUsernameStatus>;
   banned: UserStatusItem<boolean>;
-  suspension: UserStatusItem<Date>;
+  suspension: UserStatusItem<Date | null>;
 }
 
 export interface User extends TenantResource {
@@ -144,7 +144,7 @@ export async function retrieve(
   db: Db,
   tenantID: string,
   id: string
-): Promise<Readonly<User>> {
+): Promise<Readonly<User> | null> {
   return collection(db).findOne({ id, tenant_id: tenantID });
 }
 
@@ -152,7 +152,7 @@ export async function retrieveMany(
   db: Db,
   tenantID: string,
   ids: string[]
-): Promise<Array<Readonly<User>>> {
+): Promise<Array<Readonly<User> | null>> {
   const cursor = await collection(db).find({
     id: {
       $in: ids,
@@ -162,7 +162,7 @@ export async function retrieveMany(
 
   const users = await cursor.toArray();
 
-  return ids.map(id => users.find(comment => comment.id === id));
+  return ids.map(id => users.find(comment => comment.id === id) || null);
 }
 
 export async function updateRole(
@@ -170,12 +170,12 @@ export async function updateRole(
   tenantID: string,
   id: string,
   role: UserRole
-): Promise<Readonly<User>> {
+): Promise<Readonly<User> | null> {
   const result = await collection(db).findOneAndUpdate(
     { id, tenant_id: tenantID },
     { $set: { role } },
     { returnOriginal: false }
   );
 
-  return result.value;
+  return result.value || null;
 }
diff --git a/src/core/server/services/tenant/cache.ts b/src/core/server/services/tenant/cache.ts
index eb86d384a..bd3599461 100644
--- a/src/core/server/services/tenant/cache.ts
+++ b/src/core/server/services/tenant/cache.ts
@@ -14,7 +14,7 @@ const CacheUpdateChannel = "tenant";
 // rather than grabbing it from the database every single call.
 export default class Cache {
   // private tenants: Map<string, Promise<Readonly<Tenant>>>;
-  private tenants: DataLoader<string, Readonly<Tenant>>;
+  private tenants: DataLoader<string, Readonly<Tenant> | null>;
   private db: Db;
 
   constructor(db: Db, subscriber: Redis) {
@@ -72,7 +72,7 @@ export default class Cache {
   /**
    * retrieve returns a promise that will resolve to the tenant for Talk.
    */
-  public async retrieve(id: string): Promise<Readonly<Tenant>> {
+  public async retrieve(id: string): Promise<Readonly<Tenant> | null> {
     return this.tenants.load(id);
   }
 
diff --git a/src/types/passport.d.ts b/src/types/express.d.ts
similarity index 62%
rename from src/types/passport.d.ts
rename to src/types/express.d.ts
index 673c8f61f..b870dde73 100644
--- a/src/types/passport.d.ts
+++ b/src/types/express.d.ts
@@ -1,7 +1,9 @@
+import { Tenant } from "talk-server/models/tenant";
 import { User } from "talk-server/models/user";
 
 declare module "express" {
   interface Request {
     user?: User;
+    tenant?: Tenant;
   }
 }
diff --git a/tsconfig.json b/tsconfig.json
index 59ad4132d..152c7ed7a 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -1,24 +1,25 @@
 {
-    "compilerOptions": {
-        "target": "es5",
-        "module": "commonjs",
-        "esModuleInterop": true,
-        "noImplicitAny": true,
-        "allowJs": true,
-        "moduleResolution": "node",
-        "sourceMap": true,
-        "outDir": "dist",
-        "baseUrl": ".",
-        "pretty": false,
-        "removeComments": true,
-        // See https://github.com/prismagraphql/graphql-request/issues/26 for why we
-        // have to include "dom" here.
-        "lib": ["es6", "esnext.asynciterable", "dom"],
-        "paths": {
-            "talk-server/*": ["./src/core/server/*"],
-            "talk-common/*": ["./src/core/common/*"]
-        }
-    },
-    "include": ["src/**/*"],
-    "exclude": ["node_modules"]
+  "compilerOptions": {
+    "target": "es5",
+    "module": "commonjs",
+    "esModuleInterop": true,
+    "noImplicitAny": true,
+    "allowJs": true,
+    "moduleResolution": "node",
+    "sourceMap": true,
+    "outDir": "dist",
+    "baseUrl": ".",
+    "pretty": false,
+    "removeComments": true,
+    "strictNullChecks": true,
+    // See https://github.com/prismagraphql/graphql-request/issues/26 for why we
+    // have to include "dom" here.
+    "lib": ["es6", "esnext.asynciterable", "dom"],
+    "paths": {
+      "talk-server/*": ["./src/core/server/*"],
+      "talk-common/*": ["./src/core/common/*"]
+    }
+  },
+  "include": ["src/**/*"],
+  "exclude": ["node_modules"]
 }