[REV] web: allow patching getters

This reverts commit d6b854306cae245343a7acb9edc4178d245a49fe.

The reverted commit introduced issues in patching class methods.
Introducing getters in the patch function is not actually compatible
with the original method patching.

Test for patch method is also introduced to guard patch from future
invalid modification that happened.

A more robust  way of patching classes is now being developed.

closes odoo/odoo#47557

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

2 files changed, 30 insertions, 111 deletions

diff --git a/addons/web/static/src/js/core/utils.js b/addons/web/static/src/js/core/utils.js
index 9bc1a287512..243f3f51cd0 100644
--- a/addons/web/static/src/js/core/utils.js
+++ b/addons/web/static/src/js/core/utils.js
@@ -520,10 +520,6 @@ var utils = {
      * Patch a class and return a function that remove the patch
      * when called.
      *
-     * In the patch object, super methods and getters are accessible thru:
-     *  - this._super() - for method
-     *  - this._super   - for getter
-     *
      * @param {Class} C Class to patch
      * @param {string} patchName
      * @param {Object} patch
@@ -535,8 +531,7 @@ var utils = {
             metadata = {
                 origMethods: {},
                 patches: {},
-                current: [],
-                originalDescriptors: {},
+                current: []
             };
             patchMap.set(C.prototype, metadata);
         }
@@ -549,41 +544,20 @@ var utils = {
         metadata.current.push(patchName);
 
         function applyPatch(proto, patch) {
-            Object.keys(patch).forEach(function(property) {
-                const protoDescriptor = Object.getOwnPropertyDescriptor(proto, property);
-                const patchDescriptor = Object.getOwnPropertyDescriptor(patch, property);
-                if (!protoDescriptor && patchDescriptor) {
-                    if (!(property in metadata.originalDescriptors)) {
-                        metadata.originalDescriptors[property] = undefined;
-                    }
-                    Object.defineProperty(proto, property, patchDescriptor);
-                    return;
-                }
-                if (protoDescriptor.value) {
-                    const method = patch[property];
-                    if (typeof method !== 'function') return;
-                    const original = proto[property];
-                    if (!(property in metadata.origMethods)) {
-                        metadata.origMethods[property] = original;
+            Object.keys(patch).forEach(function (methodName) {
+                const method = patch[methodName];
+                if (typeof method === "function") {
+                    const original = proto[methodName];
+                    if (!(methodName in metadata.origMethods)) {
+                        metadata.origMethods[methodName] = original;
                     }
-                    proto[property] = function(...args) {
+                    proto[methodName] = function (...args) {
                         const previousSuper = this._super;
                         this._super = original;
                         const res = method.call(this, ...args);
                         this._super = previousSuper;
                         return res;
                     };
-                } else if (protoDescriptor.get) {
-                    if (!(property in metadata.originalDescriptors)) {
-                        metadata.originalDescriptors[property] = protoDescriptor;
-                    }
-                    const patchDescriptor = Object.getOwnPropertyDescriptor(patch, property);
-                    Object.defineProperty(proto, property, {
-                        get() {
-                            this._super = protoDescriptor.get.apply(this);
-                            return patchDescriptor.get.apply(this);
-                        },
-                    });
                 }
             });
         }
@@ -761,16 +735,6 @@ var utils = {
             proto[k] = metadata.origMethods[k];
         }
 
-        // reset the descriptors
-        for (let k in metadata.originalDescriptors) {
-            const descriptor = metadata.originalDescriptors[k];
-            if (descriptor === undefined) {
-                delete proto[k];
-            } else {
-                Object.defineProperty(proto, k, descriptor);
-            }
-        }
-
         // apply other patches
         for (let name of metadata.current) {
             if (name !== patchName) {
diff --git a/addons/web/static/tests/core/util_tests.js b/addons/web/static/tests/core/util_tests.js
index f33a76d87c1..3403c9effaf 100644
--- a/addons/web/static/tests/core/util_tests.js
+++ b/addons/web/static/tests/core/util_tests.js
@@ -101,85 +101,40 @@ QUnit.module('core', {}, function () {
         assert.strictEqual(human_number(-1.012e+43, 2, 2), '-1.01e+43');
     });
 
-    QUnit.test('patch a class with getter', function (assert) {
-        assert.expect(13);
+    QUnit.test('patch a class', function(assert) {
+        assert.expect(4);
 
-        const { patch } = utils;
-
-        class TestClass {
-            get val() {
-                return 'aaa';
+        class Parent {
+            foo() {
+                return 'Parent foo';
             }
         }
 
-        const testInstance = new TestClass();
-        assert.strictEqual(testInstance.val, 'aaa');
-
-        // Extend `val`
-
-        const removePatch1 = patch(TestClass, 'patch1', {
-            get val() {
-                return this._super + 'bbb';
-            },
-        });
-        assert.strictEqual(testInstance.val, 'aaabbb');
-
-        const removePatch2 = patch(TestClass, 'patch2', {
-            get val() {
-                return this._super + 'ccc';
-            },
-        });
-        assert.strictEqual(testInstance.val, 'aaabbbccc');
-
-        removePatch1();
-        assert.strictEqual(testInstance.val, 'aaaccc');
-
-        removePatch2();
-        assert.strictEqual(testInstance.val, 'aaa');
-
-        // Add getters
+        class Child extends Parent {
+            bar() {
+                return 'Child bar';
+            }
+        }
 
-        const removeMorePatch1 = patch(TestClass, 'more-patch-1', {
-            get realVal() {
-                return `${this.wrapper.start}${this.val}${this.wrapper.end}`
+        const removePatch = utils.patch(Child, 'patch', {
+            foo() {
+                return this._super() + ' patch foo';
             },
-            get wrapper() {
-                return {
-                    start: '(',
-                    end: ')',
-                }
-            }
-        })
-        assert.strictEqual(testInstance.realVal, '(aaa)');
-
-        const removeMorePatch2 = patch(TestClass, 'more-patch-2', {
-            get wrapper() {
-                return {
-                    start: `[${this._super.start}`,
-                    end: `${this._super.end}]`,
-                }
+            bar() {
+                return this._super() + ' patch bar';
             }
         })
-        assert.strictEqual(testInstance.realVal, '[(aaa)]');
 
-        const removeMorePatch3 = patch(TestClass, 'more-patch-3', {
-            get realVal() {
-                return `${this._super} - augmented`;
-            }
-        })
-        assert.strictEqual(testInstance.realVal, '[(aaa)] - augmented');
+        const child = new Child();
 
-        removeMorePatch2();
-        assert.strictEqual(testInstance.realVal, '(aaa) - augmented');
+        assert.strictEqual(child.foo(), 'Parent foo patch foo')
+        assert.strictEqual(child.bar(), 'Child bar patch bar')
 
-        removeMorePatch3();
-        assert.strictEqual(testInstance.realVal, '(aaa)');
+        removePatch();
 
-        removeMorePatch1();
-        assert.strictEqual(testInstance.realVal, undefined);
-        assert.strictEqual(testInstance.wrapper, undefined);
-        assert.strictEqual(testInstance.val, 'aaa');
-    });
+        assert.strictEqual(child.foo(), 'Parent foo');
+        assert.strictEqual(child.bar(), 'Child bar');
+    })
 
 });