Added unit tests to check usage of IRedisClientConnectionManager for Historian and Gitrest

server/gitrest/packages/gitrest-base/src/test/redis.spec.ts

@@ -0,0 +1,111 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { TestRedisClientConnectionManagerWithInvalidation } from "./testRedisClientConnectionManagerWithInvalidation";
+import { Redis, HashMapRedis } from "../utils/redisFs/redis";
+import { RedisOptions } from "ioredis-mock";
+import assert from "assert";
+
+type GitRedisFileSystem = "redisfs" | "hashmap-redisfs";
+
+function createRedisFs(
+	fileSystem: string,
+	redisClientConnectionManager: TestRedisClientConnectionManagerWithInvalidation,
+): Redis | HashMapRedis {
+	if (fileSystem === "hashmap-redisfs") {
+		const key = "test";
+		return new HashMapRedis(key, redisClientConnectionManager);
+	} else {
+		return new Redis(redisClientConnectionManager);
+	}
+}
+
+const testFileSystems: GitRedisFileSystem[] = ["redisfs", "hashmap-redisfs"];
+testFileSystems.forEach((fileSystem) => {
+	describe(`RedisFs ${fileSystem} file system`, () => {
+		let redis: Redis | HashMapRedis;
+		const redisOptions: RedisOptions = {
+			host: "localhost",
+			port: 6379,
+			connectTimeout: 10000,
+			maxRetriesPerRequest: 20,
+			enableAutoPipelining: false,
+			enableOfflineQueue: true,
+		};
+		let redisClientConnectionManager: TestRedisClientConnectionManagerWithInvalidation;
+		beforeEach(() => {
+			redisClientConnectionManager = new TestRedisClientConnectionManagerWithInvalidation(
+				redisOptions,
+			);
+			redis = createRedisFs(fileSystem, redisClientConnectionManager);
+		});
+		afterEach(async () => {
+			await redis.delAll("");
+			redisClientConnectionManager.invalidateRedisClient();
+		});
+		it("single key CRD operations should succeed", async () => {
+			await assert.doesNotReject(async () => await redis.set("foo", "bar"));
+			assert.strictEqual(await redis.get<string>("foo"), "bar");
+			assert.notStrictEqual(await redis.peek("foo"), -1);
+			assert.strictEqual(await redis.peek("foo"), 3);
+			assert.strictEqual(await redis.del("foo"), true);
+		});
+		it("multi key CRD operations should succeed", async () => {
+			const keyValuePairs = [
+				{ key: "foo", value: "bar" },
+				{ key: "foo1", value: "bar1" },
+			];
+			await assert.doesNotReject(async () => await redis.setMany(keyValuePairs));
+			assert.strictEqual(await redis.get<string>("foo1"), "bar1");
+			assert.deepStrictEqual(await redis.keysByPrefix("foo"), ["foo", "foo1"]);
+			assert.strictEqual(await redis.delAll("foo"), true);
+		});
+		it("single key CRD operations should succeed with client invalidation", async () => {
+			redisClientConnectionManager.invalidateRedisClient();
+			await assert.doesNotReject(async () => await redis.set("foo", "bar"));
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.strictEqual(await redis.get<string>("foo"), "bar");
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.notStrictEqual(await redis.peek("foo"), -1);
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.strictEqual(await redis.peek("foo"), 3);
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.strictEqual(await redis.del("foo"), true);
+		});
+		it("multi key CRD operations should succeed with client invalidation", async () => {
+			redisClientConnectionManager.invalidateRedisClient();
+			const keyValuePairs = [
+				{ key: "foo", value: "bar" },
+				{ key: "foo1", value: "bar1" },
+			];
+			await assert.doesNotReject(async () => await redis.setMany(keyValuePairs));
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.strictEqual(await redis.get<string>("foo1"), "bar1");
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.deepStrictEqual(await redis.keysByPrefix("foo"), ["foo", "foo1"]);
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.strictEqual(await redis.delAll("foo"), true);
+		});
+		it("single key CRD operations should not succeed without client recreation", async () => {
+			redisClientConnectionManager.invalidateRedisClient(false);
+			assert.rejects(async () => await redis.set("foo", "bar"));
+			assert.rejects(async () => await redis.get<string>("foo"));
+			assert.rejects(async () => await redis.peek("foo"));
+			assert.rejects(async () => redis.peek("foo"));
+			assert.rejects(async () => await redis.del("foo"));
+		});
+		it("multi key CRD operations should not succeed without client recreation", async () => {
+			redisClientConnectionManager.invalidateRedisClient(false);
+			const keyValuePairs = [
+				{ key: "foo", value: "bar" },
+				{ key: "foo1", value: "bar1" },
+			];
+			assert.rejects(async () => await redis.setMany(keyValuePairs));
+			assert.rejects(async () => await redis.get<string>("foo1"));
+			assert.rejects(async () => await redis.keysByPrefix("foo"));
+			assert.rejects(async () => await redis.delAll("foo"));
+		});
+	});
+});

server/gitrest/packages/gitrest-base/src/test/testRedisClientConnectionManagerWithInvalidation.ts

@@ -0,0 +1,35 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+import RedisMock from "ioredis-mock";
+import * as Redis from "ioredis";
+import { IRedisClientConnectionManager } from "@fluidframework/server-services-utils";
+
+// TODO: Implement this in Routerlicious
+export class TestRedisClientConnectionManagerWithInvalidation
+	implements IRedisClientConnectionManager
+{
+	private readonly options: Redis.RedisOptions;
+	private mockRedisClient: Redis.Redis;
+
+	constructor(options?) {
+		this.options = options;
+		this.createRedisClient();
+	}
+
+	private createRedisClient() {
+		this.mockRedisClient = this.options ? new RedisMock(this.options) : new RedisMock();
+	}
+
+	public invalidateRedisClient(recreateClient: boolean = true) {
+		this.mockRedisClient.disconnect();
+		if (recreateClient) {
+			this.createRedisClient();
+		}
+	}
+
+	public getRedisClient(): Redis.Redis {
+		return this.mockRedisClient;
+	}
+}

server/gitrest/packages/gitrest-base/src/utils/redisFs/redis.ts

@@ -135,7 +135,9 @@ export class Redis implements IRedis {
 		if (keys.length === 0) {
 			return false;
 		}
-		const result = await this.redisClientConnectionManager.getRedisClient().unlink(...keys);
+		const result = await this.redisClientConnectionManager
+			.getRedisClient()
+			.unlink(...keys.map((key) => this.getKey(key)));
 		return result === keys.length;
 	}
 
@@ -152,7 +154,8 @@ export class Redis implements IRedis {
 				keyPrefix,
 			},
 		);
-		return result;
+		// Remove the prefix from the keys before returning them.
+		return result.map((key) => key.replace(`${this.prefix}:`, ""));
 	}
 
 	/**
@@ -229,7 +232,7 @@ export class HashMapRedis implements IRedis {
 		// Filter out root directory fields, since they are not necessary fields in the HashMap.
 		// Then, map each field/value pair to array of arguments for the HSET command (f1, v1, f2, v2...).
 		const fieldValueArgs = fieldValuePairs
-			.filter(({ key: field, value }) => this.isFieldRootDirectory(field))
+			.filter(({ key: field, value }) => !this.isFieldRootDirectory(field))
 			.flatMap(({ key: field, value }) => [this.getMapField(field), JSON.stringify(value)]);
 		if (fieldValueArgs.length === 0) {
 			// Don't do anything if there are no fields to set.
@@ -239,7 +242,7 @@ export class HashMapRedis implements IRedis {
 		// However, if it's a duplicate field, it will return 0, so we can't rely on the return value to determine success.
 		await this.redisClientConnectionManager
 			.getRedisClient()
-			.hset(this.getMapKey(), fieldValueArgs);
+			.hset(this.getMapKey(), ...fieldValueArgs);
 	}
 
 	public async peek(field: string): Promise<number> {
@@ -300,7 +303,7 @@ export class HashMapRedis implements IRedis {
 				keyPrefix,
 			},
 		);
-		return result.filter((field) => `${this.getMapKey()}/${field}`.startsWith(keyPrefix));
+		return result.filter((field) => field.startsWith(keyPrefix));
 	}
 
 	/**

server/historian/packages/historian-base/package.json

@@ -61,13 +61,15 @@
 		"@fluidframework/server-services-shared": "5.0.0-248224",
 		"@fluidframework/server-services-telemetry": "5.0.0-248224",
 		"@fluidframework/server-services-utils": "5.0.0-248224",
+		"@types/ioredis-mock": "^8.2.5",
 		"axios": "^1.6.2",
 		"body-parser": "^1.17.2",
 		"compression": "^1.7.3",
 		"cors": "^2.8.5",
 		"debug": "^4.3.4",
 		"express": "^4.17.3",
 		"ioredis": "^5.2.3",
+		"ioredis-mock": "^8.9.0",
 		"json-stringify-safe": "5.0.1",
 		"jsonwebtoken": "^9.0.0",
 		"nconf": "^0.11.4",

server/historian/packages/historian-base/src/test/redis.spec.ts

@@ -0,0 +1,92 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { TestRedisClientConnectionManagerWithInvalidation } from "./testRedisClientConnectionManagerWithInvalidation";
+import { RedisCache, RedisTenantCache } from "../services";
+import { RedisOptions } from "ioredis-mock";
+import assert from "assert";
+
+type GitRedisFileSystem = "redisCache" | "redisTenantCache";
+
+function createRedisFs(
+	fileSystem: string,
+	redisClientConnectionManager: TestRedisClientConnectionManagerWithInvalidation,
+): RedisCache | RedisTenantCache {
+	if (fileSystem === "redisCache") {
+		return new RedisCache(redisClientConnectionManager);
+	} else {
+		return new RedisTenantCache(redisClientConnectionManager);
+	}
+}
+
+const testCache: GitRedisFileSystem[] = ["redisCache", "redisTenantCache"];
+testCache.forEach((fileSystem) => {
+	describe(`Redis cache: ${fileSystem}`, () => {
+		let redis: RedisCache | RedisTenantCache;
+		const redisOptions: RedisOptions = {
+			host: "localhost",
+			port: 6379,
+			connectTimeout: 10000,
+			maxRetriesPerRequest: 20,
+			enableAutoPipelining: false,
+			enableOfflineQueue: true,
+		};
+		const redisClientConnectionManager = new TestRedisClientConnectionManagerWithInvalidation(
+			redisOptions,
+		);
+		beforeEach(() => {
+			redis = createRedisFs(fileSystem, redisClientConnectionManager);
+		});
+		afterEach(async () => {
+			redisClientConnectionManager.invalidateRedisClient();
+		});
+		it("single key CRD operations should succeed", async () => {
+			await assert.doesNotReject(async () => {
+				await redis.set("foo", "bar");
+			});
+			assert.strictEqual(await redis.get<string>("foo"), "bar");
+			if (redis instanceof RedisTenantCache) {
+				assert.strictEqual(await redis.exists("foo"), true);
+				assert.strictEqual(await redis.exists("foo1"), false);
+			}
+			assert.strictEqual(await redis.delete("foo"), true);
+		});
+		it("single key CRD operations should succeed with client invalidation", async () => {
+			redisClientConnectionManager.invalidateRedisClient();
+			await assert.doesNotReject(async () => {
+				await redis.set("foo", "bar");
+			});
+			redisClientConnectionManager.invalidateRedisClient();
+			assert.strictEqual(await redis.get<string>("foo"), "bar");
+			redisClientConnectionManager.invalidateRedisClient();
+			if (redis instanceof RedisTenantCache) {
+				assert.strictEqual(await redis.exists("foo"), true);
+				redisClientConnectionManager.invalidateRedisClient();
+				assert.strictEqual(await redis.exists("foo1"), false);
+			}
+			assert.strictEqual(await redis.delete("foo"), true);
+		});
+		it("single key CRD operations should not succeed without client recreation", async () => {
+			redisClientConnectionManager.invalidateRedisClient(false);
+			assert.rejects(async () => {
+				await redis.set("foo", "bar");
+			});
+			assert.rejects(async () => {
+				await redis.get<string>("foo");
+			});
+			if (redis instanceof RedisTenantCache) {
+				assert.rejects(async () => {
+					await (redis as RedisTenantCache).exists("foo");
+				});
+				assert.rejects(async () => {
+					await (redis as RedisTenantCache).exists("foo1");
+				});
+			}
+			assert.rejects(async () => {
+				await redis.delete("foo");
+			});
+		});
+	});
+});

server/historian/packages/historian-base/src/test/testRedisClientConnectionManagerWithInvalidation.ts

@@ -0,0 +1,35 @@
+/*!
+ * Copyright (c) Microsoft Corporation and contributors. All rights reserved.
+ * Licensed under the MIT License.
+ */
+import RedisMock from "ioredis-mock";
+import * as Redis from "ioredis";
+import { IRedisClientConnectionManager } from "@fluidframework/server-services-utils";
+
+// TODO: Implement this in Routerlicious
+export class TestRedisClientConnectionManagerWithInvalidation
+	implements IRedisClientConnectionManager
+{
+	private readonly options: Redis.RedisOptions;
+	private mockRedisClient: Redis.Redis;
+
+	constructor(options?) {
+		this.options = options;
+		this.createRedisClient();
+	}
+
+	private createRedisClient() {
+		this.mockRedisClient = this.options ? new RedisMock(this.options) : new RedisMock();
+	}
+
+	public invalidateRedisClient(recreateClient: boolean = true) {
+		this.mockRedisClient.disconnect();
+		if (recreateClient) {
+			this.createRedisClient();
+		}
+	}
+
+	public getRedisClient(): Redis.Redis {
+		return this.mockRedisClient;
+	}
+}