Change emulator arg to take SourceFile only

src/emulator/storage/apis/firebase.ts

@@ -68,7 +68,7 @@ export function createFirebaseEndpoints(emulator: StorageEmulator): Router {
   firebaseStorageAPI.use(/.*\/b\/(.+?)\/.*/, (req, res, next) => {
     const bucketId = req.params[0];
     storageLayer.createBucket(bucketId);
-    if (!emulator.getRules(bucketId)) {
+    if (!emulator.rulesManager.getRuleset(bucketId)) {
       EmulatorLogger.forEmulator(Emulators.STORAGE).log(
         "WARN",
         "Permission denied because no Storage ruleset is currently loaded, check your rules for syntax errors."

src/emulator/storage/index.ts

@@ -6,25 +6,23 @@ import { createApp } from "./server";
 import { StorageLayer } from "./files";
 import { EmulatorLogger } from "../emulatorLogger";
 import { createStorageRulesManager, StorageRulesManager } from "./rules/manager";
-import { StorageRulesetInstance, StorageRulesRuntime, StorageRulesIssues } from "./rules/runtime";
+import { StorageRulesRuntime } from "./rules/runtime";
 import { SourceFile } from "./rules/types";
 import express = require("express");
 import { getAdminCredentialValidator, getRulesValidator } from "./rules/utils";
 import { Persistence } from "./persistence";
 import { UploadService } from "./upload";
 
-export type RulesType = SourceFile | string;
-
 export type RulesConfig = {
   resource: string;
-  rules: RulesType;
+  rules: SourceFile;
 };
 
 export interface StorageEmulatorArgs {
   projectId: string;
   port?: number;
   host?: string;
-  rules: string | RulesConfig[];
+  rules: SourceFile | RulesConfig[];
   auto_download?: boolean;
 }
 
@@ -45,7 +43,7 @@ export class StorageEmulator implements EmulatorInstance {
     this._persistence = new Persistence(this.getPersistenceTmpDir());
     this._storageLayer = new StorageLayer(
       args.projectId,
-      getRulesValidator((resource: string) => this.getRules(resource)),
+      getRulesValidator((resource: string) => this._rulesManager.getRuleset(resource)),
       getAdminCredentialValidator(),
       this._persistence
     );
@@ -60,6 +58,10 @@ export class StorageEmulator implements EmulatorInstance {
     return this._uploadService;
   }
 
+  get rulesManager(): StorageRulesManager {
+    return this._rulesManager;
+  }
+
   get logger(): EmulatorLogger {
     return this._logger;
   }
@@ -83,14 +85,6 @@ export class StorageEmulator implements EmulatorInstance {
     // No-op
   }
 
-  getRules(resource: string): StorageRulesetInstance | undefined {
-    return this._rulesManager.getRuleset(resource);
-  }
-
-  async setRules(rules: RulesType, resource: string): Promise<StorageRulesIssues> {
-    return this._rulesManager.setSourceFile(rules, resource);
-  }
-
   async stop(): Promise<void> {
     await this._persistence.deleteAll();
     await this._rulesManager.close();

src/emulator/storage/rules/config.ts

@@ -1,18 +1,24 @@
 import { RulesConfig } from "..";
 import { FirebaseError } from "../../../error";
+import { readFile } from "../../../fsutils";
 import { Options } from "../../../options";
+import { SourceFile } from "./types";
 
-function getAbsoluteRulesPath(rules: string, options: Options): string {
-  return options.config.path(rules);
+function getSourceFile(rules: string, options: Options): SourceFile {
+  const path = options.config.path(rules);
+  return { name: path, content: readFile(path) };
 }
 
 /**
  * Parses rules file for each target specified in the storage config under {@link options}.
  * @returns The rules file path if the storage config does not specify a target and an array
  *     of project resources and their corresponding rules files otherwise.
- * @throws {FirebaseError} if storage config or rules file is missing from firebase.json.
+ * @throws {FirebaseError} if storage config is missing or rules file is missing or invalid.
  */
-export function getStorageRulesConfig(projectId: string, options: Options): string | RulesConfig[] {
+export function getStorageRulesConfig(
+  projectId: string,
+  options: Options
+): SourceFile | RulesConfig[] {
   const storageConfig = options.config.data.storage;
   if (!storageConfig) {
     throw new FirebaseError(
@@ -28,7 +34,7 @@ export function getStorageRulesConfig(projectId: string, options: Options): stri
       );
     }
 
-    return getAbsoluteRulesPath(storageConfig.rules, options);
+    return getSourceFile(storageConfig.rules, options);
   }
 
   // Multiple targets
@@ -40,7 +46,7 @@ export function getStorageRulesConfig(projectId: string, options: Options): stri
     }
     rc.requireTarget(projectId, "storage", targetConfig.target);
     rc.target(projectId, "storage", targetConfig.target).forEach((resource: string) => {
-      results.push({ resource, rules: getAbsoluteRulesPath(targetConfig.rules, options) });
+      results.push({ resource, rules: getSourceFile(targetConfig.rules, options) });
     });
   }
   return results;

src/emulator/storage/rules/manager.ts

@@ -3,8 +3,7 @@ import { EmulatorLogger } from "../../emulatorLogger";
 import { Emulators } from "../../types";
 import { SourceFile } from "./types";
 import { StorageRulesIssues, StorageRulesRuntime, StorageRulesetInstance } from "./runtime";
-import { readFile } from "../../../fsutils";
-import { RulesConfig, RulesType } from "..";
+import { RulesConfig } from "..";
 
 /**
  * Keeps track of the rules source file and maintains a generated ruleset for one or more storage
@@ -19,9 +18,8 @@ export interface StorageRulesManager {
 
   /**
    * Updates the source file and, correspondingly, the file watcher and ruleset for the resource.
-   * @throws {FirebaseError} if file path is invalid.
    */
-  setSourceFile: (rules: RulesType, resource: string) => Promise<StorageRulesIssues>;
+  setSourceFile: (rules: SourceFile, resource: string) => Promise<StorageRulesIssues>;
 
   /** Deletes source file, ruleset, and removes listeners from all files for all resources. */
   close: () => Promise<void>;
@@ -32,7 +30,7 @@ export interface StorageRulesManager {
  * or a {@link StorageRulesManagerRegistry} for multiple resources.
  */
 export function createStorageRulesManager(
-  rules: RulesType | RulesConfig[],
+  rules: SourceFile | RulesConfig[],
   runtime: StorageRulesRuntime
 ): StorageRulesManager {
   return Array.isArray(rules)
@@ -50,7 +48,7 @@ class StorageRulesManagerImplementation implements StorageRulesManager {
   private _watcher = new chokidar.FSWatcher();
   private _logger = EmulatorLogger.forEmulator(Emulators.STORAGE);
 
-  constructor(private _initRules: RulesType, private _runtime: StorageRulesRuntime) {}
+  constructor(private _initRules: SourceFile, private _runtime: StorageRulesRuntime) {}
 
   async start(): Promise<StorageRulesIssues> {
     return this.setSourceFile(this._initRules);
@@ -60,20 +58,11 @@ class StorageRulesManagerImplementation implements StorageRulesManager {
     return this._ruleset;
   }
 
-  async setSourceFile(rules: RulesType): Promise<StorageRulesIssues> {
+  async setSourceFile(rules: SourceFile): Promise<StorageRulesIssues> {
     const prevRulesFile = this._sourceFile?.name;
-    let rulesFile: string;
-    if (typeof rules === "string") {
-      this._sourceFile = { name: rules, content: readFile(rules) };
-      rulesFile = rules;
-    } else {
-      // Allow invalid file path here for testing
-      this._sourceFile = rules;
-      rulesFile = rules.name;
-    }
-
+    this._sourceFile = rules;
     const issues = await this.loadRuleset();
-    this.updateWatcher(rulesFile, prevRulesFile);
+    this.updateWatcher(rules.name, prevRulesFile);
     return issues;
   }
 
@@ -135,7 +124,7 @@ class StorageRulesManagerImplementation implements StorageRulesManager {
  * Maintains a mapping from storage resource to {@link StorageRulesManagerImplementation} and
  * directs calls to the appropriate instance.
  */
-class StorageRulesManagerRegistry {
+class StorageRulesManagerRegistry implements StorageRulesManager {
   private _rulesManagers: Map<string, StorageRulesManagerImplementation>;
 
   constructor(_initRules: RulesConfig[], private _runtime: StorageRulesRuntime) {
@@ -157,7 +146,7 @@ class StorageRulesManagerRegistry {
     return this._rulesManagers.get(resource)?.getRuleset();
   }
 
-  async setSourceFile(rules: RulesType, resource: string): Promise<StorageRulesIssues> {
+  async setSourceFile(rules: SourceFile, resource: string): Promise<StorageRulesIssues> {
     const rulesManager =
       this._rulesManagers.get(resource) || this.createRulesManager(resource, rules);
     return rulesManager.setSourceFile(rules);
@@ -171,7 +160,7 @@ class StorageRulesManagerRegistry {
 
   private createRulesManager(
     resource: string,
-    rules: RulesType
+    rules: SourceFile
   ): StorageRulesManagerImplementation {
     const rulesManager = new StorageRulesManagerImplementation(rules, this._runtime);
     this._rulesManagers.set(resource, rulesManager);

src/emulator/storage/server.ts

@@ -92,7 +92,10 @@ export function createApp(
 
     const name = file.name;
     const content = file.content;
-    const issues = await emulator.setRules({ name, content }, req.params.bucketId);
+    const issues = await emulator.rulesManager.setSourceFile(
+      { name, content },
+      req.params.bucketId
+    );
 
     if (issues.errors.length > 0) {
       res.status(400).json({

src/test/emulators/storage/rules/config.spec.ts

@@ -9,6 +9,8 @@ import { getStorageRulesConfig } from "../../../../emulator/storage/rules/config
 import { StorageRulesFiles } from "../../fixtures";
 import { Persistence } from "../../../../emulator/storage/persistence";
 import { FirebaseError } from "../../../../error";
+import { RulesConfig } from "../../../../emulator/storage";
+import { SourceFile } from "../../../../emulator/storage/rules/types";
 
 const PROJECT_ID = "test-project";
 
@@ -19,18 +21,25 @@ describe("Storage Rules Config", () => {
 
   it("should parse rules config for single target", () => {
     const rulesFile = "storage.rules";
-    persistence.appendBytes(rulesFile, Buffer.from(StorageRulesFiles.readWriteIfTrue.content));
+    const rulesContent = Buffer.from(StorageRulesFiles.readWriteIfTrue.content);
+    persistence.appendBytes(rulesFile, rulesContent);
 
     const config = getOptions({
       data: { storage: { rules: rulesFile } },
       path: resolvePath,
     });
-    const result = getStorageRulesConfig(PROJECT_ID, config);
+    const result = getStorageRulesConfig(PROJECT_ID, config) as SourceFile;
 
-    expect(result).to.equal(`${tmpDir}/storage.rules`);
+    expect(result.name).to.equal(`${tmpDir}/storage.rules`);
+    expect(result.content).to.contain("allow read, write: if true");
   });
 
   it("should parse rules file for multiple targets", () => {
+    const mainRulesContent = Buffer.from(StorageRulesFiles.readWriteIfTrue.content);
+    const otherRulesContent = Buffer.from(StorageRulesFiles.readWriteIfAuth.content);
+    persistence.appendBytes("storage_main.rules", mainRulesContent);
+    persistence.appendBytes("storage_other.rules", otherRulesContent);
+
     const config = getOptions({
       data: {
         storage: [
@@ -40,15 +49,20 @@ describe("Storage Rules Config", () => {
       },
       path: resolvePath,
     });
-    config.rc.applyTarget(PROJECT_ID, "storage", "main", ["bucket_1", "bucket_2"]);
-    config.rc.applyTarget(PROJECT_ID, "storage", "other", ["bucket_3"]);
+    config.rc.applyTarget(PROJECT_ID, "storage", "main", ["bucket_0", "bucket_1"]);
+    config.rc.applyTarget(PROJECT_ID, "storage", "other", ["bucket_2"]);
 
-    const result = getStorageRulesConfig(PROJECT_ID, config);
+    const result = getStorageRulesConfig(PROJECT_ID, config) as RulesConfig[];
 
     expect(result.length).to.equal(3);
-    expect(result[0]).to.eql({ resource: "bucket_1", rules: `${tmpDir}/storage_main.rules` });
-    expect(result[1]).to.eql({ resource: "bucket_2", rules: `${tmpDir}/storage_main.rules` });
-    expect(result[2]).to.eql({ resource: "bucket_3", rules: `${tmpDir}/storage_other.rules` });
+    for (let i = 0; i < result.length; i++) {
+      expect(result[i].resource).to.eql(`bucket_${i}`);
+    }
+    expect(result[0].rules.name).to.equal(`${tmpDir}/storage_main.rules`);
+    expect(result[0].rules.content).to.contain("allow read, write: if true");
+    expect(result[1].rules.name).to.equal(`${tmpDir}/storage_main.rules`);
+    expect(result[2].rules.name).to.equal(`${tmpDir}/storage_other.rules`);
+    expect(result[2].rules.content).to.contain("allow read, write: if request.auth!=null");
   });
 
   it("should throw FirebaseError when storage config is missing", () => {
@@ -66,6 +80,15 @@ describe("Storage Rules Config", () => {
       "Cannot start the Storage emulator without rules file specified in firebase.json: run 'firebase init' and set up your Storage configuration"
     );
   });
+
+  it("should throw FirebaseError when rules file is invalid", () => {
+    const invalidFileName = "foo";
+    const config = getOptions({ data: { storage: { rules: invalidFileName } }, path: resolvePath });
+    expect(() => getStorageRulesConfig(PROJECT_ID, config)).to.throw(
+      FirebaseError,
+      `File not found: ${resolvePath(invalidFileName)}`
+    );
+  });
 });
 
 function getOptions(config: any): Options {