Add a concept of a singleflight to avoid repeated calls to stop/ending

This makes it easier to keep track of which pieces the client will have already dispatched or been executed, reducing the amount of class members needed.

Critically, this makes it so the 'stop' button (which is currently a send button) actually works even after the automatic stop has happened.

UI is still pending for stopping recording early. This is not covered by this change.

src/utils/Singleflight.ts

@@ -0,0 +1,101 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import {EnhancedMap} from "./maps";
+
+// Inspired by https://pkg.go.dev/golang.org/x/sync/singleflight
+
+const keyMap = new EnhancedMap<Object, EnhancedMap<string, unknown>>();
+
+/**
+ * Access class to get a singleflight context. Singleflights execute a
+ * function exactly once, unless instructed to forget about a result.
+ */
+export class Singleflight {
+    private constructor() {
+    }
+
+    /**
+     * A void marker to help with returning a value in a singleflight context.
+     * If your code doesn't return anything, return this instead.
+     */
+    public static Void = Symbol("void");
+
+    /**
+     * Acquire a singleflight context.
+     * @param {Object} instance An instance to associate the context with. Can be any object.
+     * @param {string} key A string key relevant to that instance to namespace under.
+     * @returns {SingleflightContext} Returns the context to execute the function.
+     */
+    public static for(instance: Object, key: string): SingleflightContext {
+        if (!instance || !key) throw new Error("An instance and key must be supplied");
+        return new SingleflightContext(instance, key);
+    }
+
+    /**
+     * Forgets all results for a given instance.
+     * @param {Object} instance The instance to forget about.
+     */
+    public static forgetAllFor(instance: Object) {
+        keyMap.delete(instance);
+    }
+
+    /**
+     * Forgets all cached results for all instances. Intended for use by tests.
+     */
+    public static forgetAll() {
+        for (const k of keyMap.keys()) {
+            keyMap.remove(k);
+        }
+    }
+}
+
+class SingleflightContext {
+    public constructor(private instance: Object, private key: string) {
+    }
+
+    /**
+     * Forget this particular instance and key combination, discarding the result.
+     */
+    public forget() {
+        const map = keyMap.get(this.instance);
+        if (!map) return;
+        map.remove(this.key);
+        if (!map.size) keyMap.remove(this.instance);
+    }
+
+    /**
+     * Execute a function. If a result is already known, that will be returned instead
+     * of executing the provided function. However, if no result is known then the function
+     * will be called, with its return value cached. The function must return a value
+     * other than `undefined` - take a look at Singleflight.Void if you don't have a return
+     * to make.
+     * @param {Function} fn The function to execute.
+     * @returns The recorded value.
+     */
+    public do<T>(fn: () => T): T {
+        const map = keyMap.getOrCreate(this.instance, new EnhancedMap<string, unknown>());
+
+        // We have to manually getOrCreate() because we need to execute the fn
+        let val = <T>map.get(this.key);
+        if (val === undefined) {
+            val = fn();
+            map.set(this.key, val);
+        }
+
+        return val;
+    }
+}

src/voice/VoiceRecording.ts

@@ -22,6 +22,7 @@ import {SimpleObservable} from "matrix-widget-api";
 import {clamp} from "../utils/numbers";
 import EventEmitter from "events";
 import {IDestroyable} from "../utils/IDestroyable";
+import {Singleflight} from "../utils/Singleflight";
 
 const CHANNELS = 1; // stereo isn't important
 const SAMPLE_RATE = 48000; // 48khz is what WebRTC uses. 12khz is where we lose quality.
@@ -52,8 +53,6 @@ export class VoiceRecording extends EventEmitter implements IDestroyable {
     private buffer = new Uint8Array(0);
     private mxc: string;
     private recording = false;
-    private stopping = false;
-    private haveWarned = false; // whether or not EndingSoon has been fired
     private observable: SimpleObservable<IRecordingUpdate>;
 
     public constructor(private client: MatrixClient) {
@@ -172,9 +171,11 @@ export class VoiceRecording extends EventEmitter implements IDestroyable {
         if (secondsLeft <= 0) {
             // noinspection JSIgnoredPromiseFromCall - we aren't concerned with it overlapping
             this.stop();
-        } else if (secondsLeft <= TARGET_WARN_TIME_LEFT && !this.haveWarned) {
-            this.emit(RecordingState.EndingSoon, {secondsLeft});
-            this.haveWarned = true;
+        } else if (secondsLeft <= TARGET_WARN_TIME_LEFT) {
+            Singleflight.for(this, "ending_soon").do(() => {
+                this.emit(RecordingState.EndingSoon, {secondsLeft});
+                return Singleflight.Void;
+            });
         }
     };
 
@@ -197,37 +198,37 @@ export class VoiceRecording extends EventEmitter implements IDestroyable {
     }
 
     public async stop(): Promise<Uint8Array> {
-        if (!this.recording) {
-            throw new Error("No recording to stop");
-        }
+        return Singleflight.for(this, "stop").do(async () => {
+            if (!this.recording) {
+                throw new Error("No recording to stop");
+            }
 
-        if (this.stopping) return;
-        this.stopping = true;
+            // Disconnect the source early to start shutting down resources
+            this.recorderSource.disconnect();
+            await this.recorder.stop();
 
-        // Disconnect the source early to start shutting down resources
-        this.recorderSource.disconnect();
-        await this.recorder.stop();
+            // close the context after the recorder so the recorder doesn't try to
+            // connect anything to the context (this would generate a warning)
+            await this.recorderContext.close();
 
-        // close the context after the recorder so the recorder doesn't try to
-        // connect anything to the context (this would generate a warning)
-        await this.recorderContext.close();
+            // Now stop all the media tracks so we can release them back to the user/OS
+            this.recorderStream.getTracks().forEach(t => t.stop());
 
-        // Now stop all the media tracks so we can release them back to the user/OS
-        this.recorderStream.getTracks().forEach(t => t.stop());
+            // Finally do our post-processing and clean up
+            this.recording = false;
+            this.recorderProcessor.removeEventListener("audioprocess", this.processAudioUpdate);
+            await this.recorder.close();
+            this.emit(RecordingState.Ended);
 
-        // Finally do our post-processing and clean up
-        this.recording = false;
-        this.recorderProcessor.removeEventListener("audioprocess", this.processAudioUpdate);
-        await this.recorder.close();
-        this.emit(RecordingState.Ended);
-
-        return this.buffer;
+            return this.buffer;
+        });
     }
 
     public destroy() {
         // noinspection JSIgnoredPromiseFromCall - not concerned about stop() being called async here
         this.stop();
         this.removeAllListeners();
+        Singleflight.forgetAllFor(this);
     }
 
     public async upload(): Promise<string> {

test/Singleflight-test.ts

@@ -0,0 +1,115 @@
+/*
+Copyright 2021 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import {Singleflight} from "../src/utils/Singleflight";
+
+describe('Singleflight', () => {
+    afterEach(() => {
+        Singleflight.forgetAll();
+    });
+
+    it('should throw for bad context variables', () => {
+        const permutations: [Object, string][] = [
+            [null, null],
+            [{}, null],
+            [null, "test"],
+        ];
+        for (const p of permutations) {
+            try {
+                Singleflight.for(p[0], p[1]);
+                // noinspection ExceptionCaughtLocallyJS
+                throw new Error("failed to fail: " + JSON.stringify(p));
+            } catch (e) {
+                expect(e.message).toBe("An instance and key must be supplied");
+            }
+        }
+    });
+
+    it('should execute the function once', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+    });
+
+    it('should execute the function once, even with new contexts', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        let sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        sf = Singleflight.for(instance, key); // RESET FOR TEST
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+    });
+
+    it('should execute the function twice if the result was forgotten', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        sf.forget();
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(2);
+    });
+
+    it('should execute the function twice if the instance was forgotten', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        Singleflight.forgetAllFor(instance);
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(2);
+    });
+
+    it('should execute the function twice if everything was forgotten', () => {
+        const instance = {};
+        const key = "test";
+        const val = {}; // unique object for reference check
+        const fn = jest.fn().mockReturnValue(val);
+        const sf = Singleflight.for(instance, key);
+        const r1 = sf.do(fn);
+        expect(r1).toBe(val);
+        expect(fn.mock.calls.length).toBe(1);
+        Singleflight.forgetAll();
+        const r2 = sf.do(fn);
+        expect(r2).toBe(val);
+        expect(fn.mock.calls.length).toBe(2);
+    });
+});
+