From 98bf7b427d418169f2349d646cab515d5128eada Mon Sep 17 00:00:00 2001 From: Ingo Fischer Date: Fri, 15 Nov 2024 15:37:44 +0100 Subject: [PATCH 1/4] Makes sure to return promise from setObject logic. Affects setObject and extendObject at least --- CHANGELOG.md | 4 ++++ packages/adapter/src/lib/adapter/adapter.ts | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41af8b5e9e..4569853ad5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ Placeholder for the next version (at the beginning of the line): ## __WORK IN PROGRESS__ --> + +## __WORK IN PROGRESS__ +* (Apollon77) Fixes async usage of setObject and extendObject methods + ## 7.0.3 (2024-11-13) - Lucy * (@foxriver76) Introduce "Vendor Packages Workflow" (only relevant for vendors - see README.md) diff --git a/packages/adapter/src/lib/adapter/adapter.ts b/packages/adapter/src/lib/adapter/adapter.ts index c1dfc560f8..874bf858e4 100644 --- a/packages/adapter/src/lib/adapter/adapter.ts +++ b/packages/adapter/src/lib/adapter/adapter.ts @@ -2921,7 +2921,7 @@ export class AdapterClass extends EventEmitter { options.obj.user = options.obj.user || (options.options ? options.options.user : '') || SYSTEM_ADMIN_USER; options.obj.ts = options.obj.ts || Date.now(); - this._setObjectWithDefaultValue(options.id, options.obj, options.options, options.callback); + return this._setObjectWithDefaultValue(options.id, options.obj, options.options, options.callback); } else { this._logger.error(`${this.namespaceLog} setObject ${options.id} mandatory property type missing!`); return tools.maybeCallbackWithError(options.callback, 'mandatory property type missing!'); @@ -3426,7 +3426,7 @@ export class AdapterClass extends EventEmitter { } } - this._setObjectWithDefaultValue(id, obj, options, callback); + return this._setObjectWithDefaultValue(id, obj, options, callback); } // external signatures From cf9620ad7271aaa140b85fa2249871a42b134535 Mon Sep 17 00:00:00 2001 From: Ingo Fischer Date: Fri, 15 Nov 2024 15:50:32 +0100 Subject: [PATCH 2/4] make linter happy --- packages/adapter/src/lib/adapter/adapter.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/adapter/src/lib/adapter/adapter.ts b/packages/adapter/src/lib/adapter/adapter.ts index 874bf858e4..753e1c26a1 100644 --- a/packages/adapter/src/lib/adapter/adapter.ts +++ b/packages/adapter/src/lib/adapter/adapter.ts @@ -2922,10 +2922,10 @@ export class AdapterClass extends EventEmitter { options.obj.ts = options.obj.ts || Date.now(); return this._setObjectWithDefaultValue(options.id, options.obj, options.options, options.callback); - } else { - this._logger.error(`${this.namespaceLog} setObject ${options.id} mandatory property type missing!`); - return tools.maybeCallbackWithError(options.callback, 'mandatory property type missing!'); } + + this._logger.error(`${this.namespaceLog} setObject ${options.id} mandatory property type missing!`); + return tools.maybeCallbackWithError(options.callback, 'mandatory property type missing!'); } /** From 470d7490e328537953f860e9bbaf57e8aa8425a3 Mon Sep 17 00:00:00 2001 From: Ingo Fischer Date: Fri, 15 Nov 2024 16:52:05 +0100 Subject: [PATCH 3/4] make setObject async save --- CHANGELOG.md | 3 +- packages/adapter/src/lib/adapter/adapter.ts | 43 ++++++++++++++++----- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4569853ad5..4269e72d30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ --> ## __WORK IN PROGRESS__ -* (Apollon77) Fixes async usage of setObject and extendObject methods +* (Apollon77) Fixes async usage of extendObject +* (Apollon77) Makes setObject async save ## 7.0.3 (2024-11-13) - Lucy * (@foxriver76) Introduce "Vendor Packages Workflow" (only relevant for vendors - see README.md) diff --git a/packages/adapter/src/lib/adapter/adapter.ts b/packages/adapter/src/lib/adapter/adapter.ts index 753e1c26a1..2ccf9cd8e3 100644 --- a/packages/adapter/src/lib/adapter/adapter.ts +++ b/packages/adapter/src/lib/adapter/adapter.ts @@ -2765,14 +2765,22 @@ export class AdapterClass extends EventEmitter { this._intervals.delete(interval as NodeJS.Timeout); } - setObject(id: string, obj: ioBroker.SettableObject, callback?: ioBroker.SetObjectCallback): Promise; + setObject( + id: string, + obj: ioBroker.SettableObject, + callback?: ioBroker.SetObjectCallback, + ): Promise | void>; setObject( id: string, obj: ioBroker.SettableObject, options: unknown, callback?: ioBroker.SetObjectCallback, - ): Promise; - setObject(id: string, obj: ioBroker.SettableObject, callback?: ioBroker.SetObjectCallback): Promise; + ): Promise | void>; + setObject( + id: string, + obj: ioBroker.SettableObject, + callback?: ioBroker.SetObjectCallback, + ): Promise | void>; /** * Creates or overwrites an object in objectDB. * @@ -2803,7 +2811,12 @@ export class AdapterClass extends EventEmitter { * } * ``` */ - setObject(id: unknown, obj: unknown, options: unknown, callback?: unknown): Promise | void { + setObject( + id: unknown, + obj: unknown, + options: unknown, + callback?: unknown, + ): Promise | void> { if (typeof options === 'function') { callback = options; options = null; @@ -2819,7 +2832,9 @@ export class AdapterClass extends EventEmitter { return this._setObject({ id, obj: obj as ioBroker.SettableObject, options, callback }); } - private async _setObject(options: InternalSetObjectOptions): Promise { + private async _setObject( + options: InternalSetObjectOptions, + ): Promise | void> { if (!this._defaultObjs) { this._defaultObjs = (await import('./defaultObjs.js')).createDefaults(); } @@ -2844,6 +2859,7 @@ export class AdapterClass extends EventEmitter { this._utils.validateId(options.id, false, null); } catch (e) { this._logger.error(tools.appendStackTrace(`${this.namespaceLog} ${e.message}`)); + // Error is logged and silently ignored to not break older adapters return; } } @@ -3357,7 +3373,12 @@ export class AdapterClass extends EventEmitter { * } * ``` */ - setForeignObject(id: unknown, obj: unknown, options: unknown, callback?: unknown): MaybePromise { + setForeignObject( + id: unknown, + obj: unknown, + options: unknown, + callback?: unknown, + ): Promise | void> | void { if (typeof options === 'function') { callback = options; options = null; @@ -3386,7 +3407,9 @@ export class AdapterClass extends EventEmitter { return this._setForeignObject({ id, obj: obj as ioBroker.SettableObject, options, callback }); } - private _setForeignObject(_options: InternalSetObjectOptions): MaybePromise { + private _setForeignObject( + _options: InternalSetObjectOptions, + ): Promise | void> | void { const { options, callback, obj } = _options; let { id } = _options; @@ -9316,7 +9339,7 @@ export class AdapterClass extends EventEmitter { return tools.maybeCallbackWithError(callback, e); } } - this.#states.delState(id, callback); + await this.#states.delState(id, callback); } // external signature @@ -9784,7 +9807,7 @@ export class AdapterClass extends EventEmitter { subs[pattern][this.namespace]++; this.outputCount++; - this.#states.setState(`system.adapter.${autoSubEntry}.subscribes`, JSON.stringify(subs)); + await this.#states.setState(`system.adapter.${autoSubEntry}.subscribes`, JSON.stringify(subs)); } } @@ -10013,7 +10036,7 @@ export class AdapterClass extends EventEmitter { delete subs[pattern]; } this.outputCount++; - this.#states.setState(`system.adapter.${autoSub}.subscribes`, JSON.stringify(subs)); + await this.#states.setState(`system.adapter.${autoSub}.subscribes`, JSON.stringify(subs)); } } } From 57b76efdf33e83b631d291a4eaf3c712fbe20f5f Mon Sep 17 00:00:00 2001 From: Max Hauser Date: Wed, 27 Nov 2024 19:15:55 +0100 Subject: [PATCH 4/4] [chore]: deprecate setObjectAsync/setForeignObjectAsync (#2974) * deprecate setObjectAsync/setForeignObjectAsync * added tests and corrected types * fix the test --- CHANGELOG.md | 5 +- packages/adapter/src/lib/adapter/adapter.ts | 57 +++++++++++----- .../test/lib/testObjectsFunctions.ts | 68 ++++++++++++++++--- 3 files changed, 100 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4269e72d30..963508d32b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,9 @@ --> ## __WORK IN PROGRESS__ -* (Apollon77) Fixes async usage of extendObject -* (Apollon77) Makes setObject async save +* (@Apollon77) Fixes async usage of extendObject +* (@Apollon77) Makes setObject async save +* (@foxriver76) deprecated `set(Foreign)ObjectAsync` as the non async methods are now working correctly with promises ## 7.0.3 (2024-11-13) - Lucy * (@foxriver76) Introduce "Vendor Packages Workflow" (only relevant for vendors - see README.md) diff --git a/packages/adapter/src/lib/adapter/adapter.ts b/packages/adapter/src/lib/adapter/adapter.ts index 3a6b789827..1cf9d0c4ee 100644 --- a/packages/adapter/src/lib/adapter/adapter.ts +++ b/packages/adapter/src/lib/adapter/adapter.ts @@ -319,9 +319,17 @@ export interface AdapterClass { commandsPermissions: CommandsPermissions, options?: unknown, ): Promise; - /** Creates or overwrites an object in the object db */ + /** + * Creates or overwrites an object in the object db + * + * @deprecated use `adapter.setObject` without a callback instead + */ setObjectAsync(id: string, obj: ioBroker.SettableObject, options?: unknown): ioBroker.SetObjectPromise; - /** Creates or overwrites an object (which might not belong to this adapter) in the object db */ + /** + * Creates or overwrites an object (which might not belong to this adapter) in the object db + * + * @deprecated use `adapter.setForeignObject` without a callback instead + */ setForeignObjectAsync( id: T, obj: ioBroker.SettableObject>, @@ -2768,19 +2776,14 @@ export class AdapterClass extends EventEmitter { setObject( id: string, obj: ioBroker.SettableObject, - callback?: ioBroker.SetObjectCallback, - ): Promise | void>; + ): Promise>; + setObject(id: string, obj: ioBroker.SettableObject, callback: ioBroker.SetObjectCallback): void; + setObject(id: string, obj: ioBroker.SettableObject, options: unknown, callback: ioBroker.SetObjectCallback): void; setObject( id: string, obj: ioBroker.SettableObject, options: unknown, - callback?: ioBroker.SetObjectCallback, - ): Promise | void>; - setObject( - id: string, - obj: ioBroker.SettableObject, - callback?: ioBroker.SetObjectCallback, - ): Promise | void>; + ): Promise>; /** * Creates or overwrites an object in objectDB. * @@ -2814,7 +2817,7 @@ export class AdapterClass extends EventEmitter { setObject( id: unknown, obj: unknown, - options: unknown, + options?: unknown, callback?: unknown, ): Promise | void> { if (typeof options === 'function') { @@ -3348,14 +3351,23 @@ export class AdapterClass extends EventEmitter { setForeignObject( id: T, obj: ioBroker.SettableObject>, - callback?: ioBroker.SetObjectCallback, + ): Promise>; + setForeignObject( + id: T, + obj: ioBroker.SettableObject>, + callback: ioBroker.SetObjectCallback, ): void; + setForeignObject( + id: T, + obj: ioBroker.SettableObject>, + options: unknown, + ): Promise>; setForeignObject( id: T, obj: ioBroker.SettableObject>, options: unknown, callback?: ioBroker.SetObjectCallback, - ): void; + ): void | Promise>; /** * Same as {@link AdapterClass.setObject}, but for any object. @@ -3376,7 +3388,7 @@ export class AdapterClass extends EventEmitter { setForeignObject( id: unknown, obj: unknown, - options: unknown, + options?: unknown, callback?: unknown, ): Promise | void> | void { if (typeof options === 'function') { @@ -3456,14 +3468,23 @@ export class AdapterClass extends EventEmitter { extendForeignObject( id: T, objPart: ioBroker.PartialObject>, - callback?: ioBroker.SetObjectCallback, + ): Promise>; + extendForeignObject( + id: T, + objPart: ioBroker.PartialObject>, + callback: ioBroker.SetObjectCallback, ): void; extendForeignObject( id: T, objPart: ioBroker.PartialObject>, options: ioBroker.ExtendObjectOptions, - callback?: ioBroker.SetObjectCallback, + callback: ioBroker.SetObjectCallback, ): void; + extendForeignObject( + id: T, + objPart: ioBroker.PartialObject>, + options: ioBroker.ExtendObjectOptions, + ): Promise>; /** * Same as {@link AdapterClass.extendObject}, but for any object. @@ -3484,7 +3505,7 @@ export class AdapterClass extends EventEmitter { extendForeignObject( id: unknown, obj: unknown, - options: unknown, + options?: unknown, callback?: unknown, ): Promise | void> | void { if (typeof options === 'function') { diff --git a/packages/controller/test/lib/testObjectsFunctions.ts b/packages/controller/test/lib/testObjectsFunctions.ts index c8590dbafb..f329095e03 100644 --- a/packages/controller/test/lib/testObjectsFunctions.ts +++ b/packages/controller/test/lib/testObjectsFunctions.ts @@ -84,7 +84,53 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont }); }); - //extendObject + it(`${testName}setObject without callback is async`, async () => { + const id = `${gid}AsyncNoCb`; + + const res = await context.adapter.setObject(id, { + type: 'state', + common: { + name: 'test1', + type: 'number', + role: 'level', + read: true, + write: true, + }, + native: { + attr1: '1', + attr2: '2', + attr3: '3', + }, + }); + + expect(res).to.be.ok; + expect(res.id).to.be.equal(`${context.adapter.namespace}.${id}`); + }); + + it(`${testName}setForeignObject without callback is async`, async () => { + const id = `${context.adapterShortName}.0.${gid}ForeignAsyncNoCb`; + + const res = await context.adapter.setForeignObject(id, { + type: 'state', + common: { + name: 'test1', + type: 'number', + role: 'level', + read: true, + write: true, + }, + native: { + attr1: '1', + attr2: '2', + attr3: '3', + }, + }); + + expect(res).to.be.ok; + expect(res.id).to.be.equal(id); + }); + + // extendObject it(`${testName}Check if objects will be extended`, function (done) { context.adapter.extendObject( gid, @@ -1363,7 +1409,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont // should use def as default state value on extendObject when obj non-existing it(`${testName}Check extendObject state with def`, async function () { this.timeout(3_000); - let obj = await context.adapter.extendObjectAsync('testDefaultValExtend', { + let obj = await context.adapter.extendObject('testDefaultValExtend', { type: 'state', common: { type: 'string', @@ -1392,7 +1438,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont // delete state but not object await context.adapter.delStateAsync('testDefaultValExtend'); // extend it again - def should be created again, because state has been removed - now we set a def object - obj = await context.adapter.extendObjectAsync('testDefaultValExtend', { + obj = await context.adapter.extendObject('testDefaultValExtend', { common: { def: { hello: 'world' }, }, @@ -1408,7 +1454,8 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont // should use def as default state value on extendForeignObject when obj non-existing it(`${testName}Check extendForeignObject state with def`, async () => { - let obj = await context.adapter.extendForeignObjectAsync('foreign.0.testDefaultValExtend', { + const id = 'foreign.0.testDefaultValExtend'; + let obj = await context.adapter.extendForeignObject(id, { type: 'state', common: { type: 'string', @@ -1417,13 +1464,14 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont }); expect(obj).to.be.ok; + expect(obj?.id).to.be.equal(id); - let state = await context.adapter.getForeignStateAsync('foreign.0.testDefaultValExtend'); + let state = await context.adapter.getForeignStateAsync(id); expect(state!.val).to.equal('Run Forrest, Run!'); expect(state!.ack).to.equal(true); // when state already exists def should not override - obj = await context.adapter.extendForeignObjectAsync('foreign.0.testDefaultValExtend', { + obj = await context.adapter.extendForeignObjectAsync(id, { common: { def: 'Please, do not set me up', }, @@ -1431,13 +1479,13 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont expect(obj).to.be.ok; - state = await context.adapter.getForeignStateAsync('foreign.0.testDefaultValExtend'); + state = await context.adapter.getForeignStateAsync(id); expect(state!.val).to.equal('Run Forrest, Run!'); // delete state but not object - await context.adapter.delForeignStateAsync('foreign.0.testDefaultValExtend'); + await context.adapter.delForeignStateAsync(id); // extend it again - def should be created again, because state has been removed - now we set a def object - obj = await context.adapter.extendForeignObjectAsync('foreign.0.testDefaultValExtend', { + obj = await context.adapter.extendForeignObjectAsync(id, { common: { def: { hello: 'world' }, }, @@ -1445,7 +1493,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont expect(obj).to.be.ok; - state = await context.adapter.getForeignStateAsync('foreign.0.testDefaultValExtend'); + state = await context.adapter.getForeignStateAsync(id); // @ts-expect-error TODO do we want this auto parsing? expect(state!.val!.hello).to.equal('world'); expect(state!.ack).to.equal(true);