Repository: sjbarag/brs
Pull request #526 (2020-09-02 18:34:05)

feat(rsg): Implement unobserveField
`roSGNode#unobserveField` unsubscribes _all_ nodes subscribed to that field, regardless of subtype, when they subscribed, etc.  It's pretty dangerous, but available in the standard library.


Patch:
diff --git a/src/brsTypes/components/RoSGNode.ts b/src/brsTypes/components/RoSGNode.ts
index 0236a8d..cfeb9e6 100644
--- a/src/brsTypes/components/RoSGNode.ts
+++ b/src/brsTypes/components/RoSGNode.ts
@@ -212,6 +212,10 @@ export class Field {
         this.observers.set(subscriber, [...maybeCallbacks, brsCallback]);
     }
 
+    removeAllObservers() {
+        this.observers.clear();
+    }
+
     private executeCallbacks(callback: BrsCallback) {
         let { interpreter, callable, hostNode, environment, eventParams } = callback;
 
@@ -280,6 +284,7 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable {
                 this.getfield,
                 this.hasfield,
                 this.observefield,
+                this.unobservefield,
                 this.removefield,
                 this.setfield,
                 this.setfields,
@@ -786,6 +791,32 @@ export class RoSGNode extends BrsComponent implements BrsValue, BrsIterable {
         },
     });
 
+    /**
+     * Removes all observers of a given field, regardless of whether or not the host node is the subscriber.
+     */
+    private unobservefield = new Callable("unobservefield", {
+        signature: {
+            args: [new StdlibArgument("fieldname", ValueKind.String)],
+            returns: ValueKind.Boolean,
+        },
+        impl: (interpreter: Interpreter, fieldname: BrsString, functionname: BrsString) => {
+            if (!interpreter.environment.hostNode) {
+                let location = `${interpreter.location.file}:(${interpreter.location.start.line})`;
+                interpreter.stderr.write(
+                    `BRIGHTSCRIPT: ERROR: roSGNode.unObserveField: no active host node: ${location}\n`
+                );
+                return BrsBoolean.False;
+            }
+
+            let field = this.fields.get(fieldname.value.toLowerCase());
+            if (field instanceof Field) {
+                field.removeAllObservers();
+            }
+            // returns true, even if the field doesn't exist
+            return BrsBoolean.True;
+        },
+    });
+
     /** Removes the given field from the node */
     /** TODO: node built-in fields shouldn't be removable (i.e. id, change, focusable,) */
     private removefield = new Callable("removefield", {


Test patch:
diff --git a/test/brsTypes/components/RoSGNode.test.js b/test/brsTypes/components/RoSGNode.test.js
index 4f88498..54f1b6e 100644
--- a/test/brsTypes/components/RoSGNode.test.js
+++ b/test/brsTypes/components/RoSGNode.test.js
@@ -12,8 +12,10 @@ const {
     BrsInvalid,
     ValueKind,
     Uninitialized,
+    Callable,
 } = brs.types;
 const { Interpreter } = require("../../../lib/interpreter");
+const { Scope } = require("../../../lib/interpreter/Environment");
 
 describe("RoSGNode", () => {
     describe("stringification", () => {
@@ -519,13 +521,78 @@ describe("RoSGNode", () => {
 
         describe("observefield", () => {
             it("adds an observer", () => {
+                let cbImpl = jest.fn();
                 let node = new RoSGNode([
                     { name: new BrsString("foo"), value: new Int32(-99) },
                     { name: new BrsString("bar"), value: new BrsString("hello") },
                 ]);
 
+                interpreter.environment.hostNode = node;
+                interpreter.environment.define(
+                    Scope.Module,
+                    "callback",
+                    new Callable("callback", {
+                        signature: {
+                            args: [],
+                            returns: ValueKind.Void,
+                        },
+                        impl: cbImpl,
+                    })
+                );
+
                 let observeField = node.getMethod("observefield");
                 expect(observeField).toBeTruthy();
+
+                observeField.call(interpreter, new BrsString("foo"), new BrsString("callback"));
+
+                node.set(new BrsString("foo"), new Int32(1));
+                expect(cbImpl).toHaveBeenCalledTimes(1);
+            });
+        });
+
+        describe("unobserveField", () => {
+            it("removes all observers", () => {
+                let cbImpl = jest.fn();
+                let target = new RoSGNode([
+                    { name: new BrsString("foo"), value: new Int32(-99) },
+                    { name: new BrsString("bar"), value: new BrsString("hello") },
+                ]);
+
+                let subscriberA = new RoSGNode([]);
+                let subscriberB = new RoSGNode([]);
+
+                [subscriberA, subscriberB].forEach((subscriber) => {
+                    interpreter.environment.hostNode = subscriber;
+                    interpreter.environment.define(
+                        Scope.Module,
+                        "callback",
+                        new Callable("callback", {
+                            signature: {
+                                args: [],
+                                returns: ValueKind.Void,
+                            },
+                            impl: cbImpl,
+                        })
+                    );
+
+                    // observe the field from each subscriber
+                    let observeField = target.getMethod("observefield");
+                    expect(observeField).toBeTruthy();
+                    observeField.call(interpreter, new BrsString("foo"), new BrsString("callback"));
+                });
+
+                // update the field to trigger both subscribers
+                target.set(new BrsString("foo"), new Int32(1));
+                expect(cbImpl).toHaveBeenCalledTimes(2);
+
+                // now remove the observer
+                let unobserveField = target.getMethod("unobserveField");
+                expect(unobserveField).toBeTruthy();
+                unobserveField.call(interpreter, new BrsString("foo"));
+
+                // and make sure neither subscriber is called
+                target.set(new BrsString("foo"), new Int32(1));
+                expect(cbImpl).toHaveBeenCalledTimes(2);
             });
         });
 
diff --git a/test/e2e/Observers.test.js b/test/e2e/Observers.test.js
new file mode 100644
index 0000000..a36d7df
--- /dev/null
+++ b/test/e2e/Observers.test.js
@@ -0,0 +1,41 @@
+const { execute } = require("../../lib/");
+const { createMockStreams, resourceFile, allArgs } = require("./E2ETests");
+const path = require("path");
+
+describe("end to end brightscript functions", () => {
+    let outputStreams;
+
+    beforeAll(() => {
+        outputStreams = createMockStreams();
+        outputStreams.root = path.join(__dirname, "resources", "observers");
+    });
+
+    afterEach(() => {
+        jest.resetAllMocks();
+    });
+
+    afterAll(() => {
+        jest.restoreAllMocks();
+    });
+
+    test("observers/unscoped-main.brs", async () => {
+        await execute([resourceFile("observers", "unscoped-main.brs")], outputStreams);
+        expect(allArgs(outputStreams.stdout.write).join("")).toBe(
+            `[Observer::init]
+[Observer::init]
+[Observer2::init]
+[Observer#a :: onTargetChanged] observing
+[Observer#b :: onTargetChanged] observing
+[Observer2#c :: onTargetChanged] observing
+[Observer#a :: onTriggerChanged] trigger = 1
+[Observer#b :: onTriggerChanged] trigger = 1
+[Observer2#c :: onTriggerChanged] trigger = 1
+[Observer#a :: onTriggerChanged] trigger = 2
+[Observer#b :: onTriggerChanged] trigger = 2
+[Observer2#c :: onTriggerChanged] trigger = 2
+[Observer#a :: onTriggerChanged] trigger = unobserve-a
+[Observer#a :: onTriggerChanged] unobserveField result = true
+`
+        );
+    });
+});
diff --git a/test/e2e/resources/observers/components/Observer.brs b/test/e2e/resources/observers/components/Observer.brs
new file mode 100644
index 0000000..d91cd50
--- /dev/null
+++ b/test/e2e/resources/observers/components/Observer.brs
@@ -0,0 +1,25 @@
+sub init()
+    m.subtype = m.top.subtype()
+    print "[" m.subtype "::init]"
+    m.target = invalid
+end sub
+
+sub onTargetChanged(event as object)
+    target = event.getData()
+    if m.target <> invalid and target <> invalid and m.target.isSameNode(target) then return
+    if target <> invalid then
+        m.target = target
+        print "[" + m.subtype + "#" + m.top.id + " :: onTargetChanged] observing"
+        target.observeField("trigger", "onTriggerChanged")
+    end if
+end sub
+
+sub onTriggerChanged(event as object)
+    trigger = event.getData()
+    print "[" + m.subtype + "#" + m.top.id + " :: onTriggerChanged] trigger = " + trigger
+
+    if trigger = "unobserve-" + m.top.id then
+        result = m.top.target.unobserveField("trigger")
+        print "[" + m.subtype + "#" + m.top.id + " :: onTriggerChanged] unobserveField result = " + result.toStr()
+    end if
+end sub
diff --git a/test/e2e/resources/observers/components/Observer.xml b/test/e2e/resources/observers/components/Observer.xml
new file mode 100644
index 0000000..86f957e
--- /dev/null
+++ b/test/e2e/resources/observers/components/Observer.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<component name="Observer">
+    <interface>
+        <field id="target" type="node" onChange="onTargetChanged" />
+    </interface>
+
+    <script type="text/brightscript" uri="./Observer.brs"/>
+</component>
diff --git a/test/e2e/resources/observers/components/Observer2.xml b/test/e2e/resources/observers/components/Observer2.xml
new file mode 100644
index 0000000..30fb7c4
--- /dev/null
+++ b/test/e2e/resources/observers/components/Observer2.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<component name="Observer2">
+    <interface>
+        <field id="target" type="node" onChange="onTargetChanged" />
+    </interface>
+
+    <script type="text/brightscript" uri="./Observer.brs"/>
+</component>
diff --git a/test/e2e/resources/observers/components/UnscopedObserversTestBed.brs b/test/e2e/resources/observers/components/UnscopedObserversTestBed.brs
new file mode 100644
index 0000000..49ec141
--- /dev/null
+++ b/test/e2e/resources/observers/components/UnscopedObserversTestBed.brs
@@ -0,0 +1,22 @@
+sub init()
+    target = createObject("roSGNode", "Node")
+    target.update({ trigger: "none" }, true)
+
+    a = createObject("roSGNode", "Observer")
+    b = createObject("roSGNode", "Observer")
+    c = createObject("roSGNode", "Observer2")
+
+    a.id = "a"
+    a.target = target
+
+    b.id = "b"
+    b.target = target
+
+    c.id = "c"
+    c.target = target
+
+    target.trigger = "1"
+    target.trigger = "2"
+    target.trigger = "unobserve-a"
+    target.trigger = "3"
+end sub
diff --git a/test/e2e/resources/observers/components/UnscopedObserversTestBed.xml b/test/e2e/resources/observers/components/UnscopedObserversTestBed.xml
new file mode 100644
index 0000000..9df2bba
--- /dev/null
+++ b/test/e2e/resources/observers/components/UnscopedObserversTestBed.xml
@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<component name="UnscopedObserversTestBed">
+    <script type="text/brightscript" uri="./UnscopedObserversTestBed.brs"/>
+</component>
diff --git a/test/e2e/resources/observers/unscoped-main.brs b/test/e2e/resources/observers/unscoped-main.brs
new file mode 100644
index 0000000..6e35440
--- /dev/null
+++ b/test/e2e/resources/observers/unscoped-main.brs
@@ -0,0 +1,5 @@
+sub main()
+    ' observeField can only be called from the context of a node,
+    ' so these tests happen in `UnscopedObserversTestBed::init()`
+    createObject("roSGNode", "UnscopedObserversTestBed")
+end sub