refactor: Type check and restructure tests (#119)

This is more what I was getting at with https://github.com/storacha/freeway/pull/116/files#r1787752932. This avoids some of the stashing and mutation by putting more inside the `new Promise()` construction itself. I've also pulled `wranglerEnv` out into its own `const`, because it's actually an _input_, not an output. Then I've made `workerInfo` a `let` which is assigned when Wrangler boots, and is `undefined` before that, signaling that Wrangler is not yet available. The previous code was failing because there were `undefined` fields that aren't typed to allow `undefined`. Which wasn't an issue until… I also turned type checking on in `test/`, because apparently it wasn't already. 😛 And lastly, I've ignored `.wrangler`, because Wrangler writes to that when it runs. --------- Co-authored-by: Felipe Forbeck <[email protected]>
storacha · Oct 21, 2024 · d097a91 · d097a91
1 parent 9444729
commit d097a91
Show file tree

Hide file tree

Showing 13 changed files with 503 additions and 281 deletions.
diff --git a/.gitignore b/.gitignore
@@ -2,4 +2,5 @@ node_modules
 dist
 .mf
 .env
-.dev.vars
+.dev.vars
+.wrangler
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -44,6 +44,10 @@
   },
   "devDependencies": {
     "@cloudflare/workers-types": "^4.20231218.0",
+    "@types/chai": "^5.0.0",
+    "@types/mocha": "^10.0.9",
+    "@types/node-fetch": "^2.6.11",
+    "@types/sinon": "^17.0.3",
     "@ucanto/principal": "^8.1.0",
     "@web3-storage/content-claims": "^5.0.0",
     "@web3-storage/public-bucket": "^1.1.0",
@@ -58,7 +62,7 @@
     "sinon": "^19.0.2",
     "standard": "^17.1.0",
     "tree-kill": "^1.2.2",
-    "typescript": "^5.3.3",
+    "typescript": "^5.6.3",
     "wrangler": "^3.78.8"
   },
   "standard": {

diff --git a/src/bindings.d.ts b/src/bindings.d.ts
@@ -1,29 +1,10 @@
-import type { R2Bucket, KVNamespace, RateLimit } from '@cloudflare/workers-types'
 import { CID } from '@web3-storage/gateway-lib/handlers'
-import { RATE_LIMIT_EXCEEDED } from './constants.js'
+import { Environment as RateLimiterEnvironment } from './handlers/rate-limiter.types.ts'
+import { Environment as CarBlockEnvironment } from './handlers/car-block.types.ts'
 
-export { }
-
-export interface Environment {
+export interface Environment extends CarBlockEnvironment, RateLimiterEnvironment {
   VERSION: string
-  DEBUG: string
-  CARPARK: R2Bucket
   CONTENT_CLAIMS_SERVICE_URL?: string
-  ACCOUNTING_SERVICE_URL: string
-  RATE_LIMITER: RateLimit
-  AUTH_TOKEN_METADATA: KVNamespace
-  FF_RATE_LIMITER_ENABLED: string
-}
-
-export type RateLimitExceeded = typeof RATE_LIMIT_EXCEEDED[keyof typeof RATE_LIMIT_EXCEEDED]
-
-export interface RateLimitService {
-  check: (cid: CID, req: Request) => Promise<RateLimitExceeded>
-}
-
-export interface TokenMetadata {
-  locationClaim?: unknown // TODO: figure out the right type to use for this - we probably need it for the private data case to verify auth
-  invalid?: boolean
 }
 
 export interface AccountingService {

diff --git a/src/handlers/car-block.js b/src/handlers/car-block.js
@@ -7,14 +7,17 @@ import { base58btc } from 'multiformats/bases/base58'
 import { CAR_CODE } from '../constants.js'
 
 /**
- * @typedef {import('@web3-storage/gateway-lib').IpfsUrlContext} CarBlockHandlerContext
- * @typedef {{ offset: number, length?: number } | { offset?: number, length: number } | { suffix: number }} Range
+ * @import { Context, IpfsUrlContext as CarBlockHandlerContext, Handler } from '@web3-storage/gateway-lib'
+ * @import { R2Bucket, KVNamespace, RateLimit } from '@cloudflare/workers-types'
+ * @import { Environment } from './car-block.types.js'
  */
 
+/** @typedef {{ offset: number, length?: number } | { offset?: number, length: number } | { suffix: number }} Range */
+
 /**
  * Handler that serves CAR files directly from R2.
  *
- * @type {import('@web3-storage/gateway-lib').Handler<CarBlockHandlerContext, import('../bindings.js').Environment>}
+ * @type {Handler<CarBlockHandlerContext, Environment>}
  */
 export async function handleCarBlock (request, env, ctx) {
   const { searchParams, dataCid } = ctx

diff --git a/src/handlers/car-block.types.ts b/src/handlers/car-block.types.ts
@@ -0,0 +1,6 @@
+import { Environment as MiddlewareEnvironment } from '@web3-storage/gateway-lib'
+import { R2Bucket } from '@cloudflare/workers-types'
+
+export interface Environment extends MiddlewareEnvironment {
+  CARPARK: R2Bucket
+}
diff --git a/src/handlers/rate-limiter.js b/src/handlers/rate-limiter.js
@@ -3,10 +3,14 @@ import { RATE_LIMIT_EXCEEDED } from '../constants.js'
 import { Accounting } from '../services/accounting.js'
 
 /**
- * @typedef {import('../bindings.js').Environment} Environment
- * @typedef {import('@web3-storage/gateway-lib').IpfsUrlContext} IpfsUrlContext
- * @typedef {import('../bindings.js').RateLimitService} RateLimitService
- * @typedef {import('../bindings.js').RateLimitExceeded} RateLimitExceeded
+ * @import { Context, IpfsUrlContext, Middleware } from '@web3-storage/gateway-lib'
+ * @import { R2Bucket, KVNamespace, RateLimit } from '@cloudflare/workers-types'
+ * @import {
+ *   Environment,
+ *   TokenMetadata,
+ *   RateLimitService,
+ *   RateLimitExceeded
+ * } from './rate-limiter.types.js'
  */
 
 /**
@@ -15,7 +19,7 @@ import { Accounting } from '../services/accounting.js'
  * it can be enabled or disabled using the FF_RATE_LIMITER_ENABLED flag.
  * Every successful request is recorded in the accounting service.
  *
- * @type {import('@web3-storage/gateway-lib').Middleware<IpfsUrlContext, IpfsUrlContext, Environment>}
+ * @type {Middleware<IpfsUrlContext, IpfsUrlContext, Environment>}
  */
 export function withRateLimit (handler) {
   return async (req, env, ctx) => {
@@ -29,7 +33,9 @@ export function withRateLimit (handler) {
     if (isRateLimitExceeded === RATE_LIMIT_EXCEEDED.YES) {
       throw new HttpError('Too Many Requests', { status: 429 })
     } else {
-      const accounting = Accounting.create({ serviceURL: env.ACCOUNTING_SERVICE_URL })
+      const accounting = Accounting.create({
+        serviceURL: env.ACCOUNTING_SERVICE_URL
+      })
       // NOTE: non-blocking call to the accounting service
       ctx.waitUntil(accounting.record(dataCid, req))
       return handler(req, env, ctx)
@@ -89,7 +95,7 @@ async function getAuthorizationTokenFromRequest (request) {
 }
 
 /**
- * @param {import('@cloudflare/workers-types').RateLimit} rateLimitAPI
+ * @param {RateLimit} rateLimitAPI
  * @param {import('multiformats/cid').CID} cid
  * @returns {Promise<import('../constants.js').RATE_LIMIT_EXCEEDED>}
  * @throws {Error} if no rate limit API is found
@@ -108,10 +114,10 @@ async function isRateLimited (rateLimitAPI, cid) {
 }
 
 /**
- * @param {import("../bindings.js").Environment} env
+ * @param {Environment} env
  * @param {string} authToken
- * @param {import('@web3-storage/gateway-lib').Context} ctx
- * @returns {Promise<import('../bindings.js').TokenMetadata | null>}
+ * @param {Context} ctx
+ * @returns {Promise<TokenMetadata | null>}
  */
 async function getTokenMetadata (env, authToken, ctx) {
   const cachedValue = await env.AUTH_TOKEN_METADATA.get(authToken)
@@ -121,7 +127,9 @@ async function getTokenMetadata (env, authToken, ctx) {
     return decode(cachedValue)
   }
 
-  const accounting = Accounting.create({ serviceURL: env.ACCOUNTING_SERVICE_URL })
+  const accounting = Accounting.create({
+    serviceURL: env.ACCOUNTING_SERVICE_URL
+  })
   const tokenMetadata = await accounting.getTokenMetadata(authToken)
   if (tokenMetadata) {
     // NOTE: non-blocking call to the auth token metadata cache
@@ -134,15 +142,15 @@ async function getTokenMetadata (env, authToken, ctx) {
 
 /**
  * @param {string} s
- * @returns {import('../bindings.js').TokenMetadata}
+ * @returns {TokenMetadata}
  */
 function decode (s) {
   // TODO should this be dag-json?
   return JSON.parse(s)
 }
 
 /**
- * @param {import('../bindings.js').TokenMetadata} m
+ * @param {TokenMetadata} m
  * @returns {string}
  */
 function encode (m) {