Skip to content

Commit 2c4baa4

Browse files
Apollon77GermanBluefoxfoxriver76
authored
Makes sure to return promise from setObject logic. … (#2967)
* Makes sure to return promise from setObject logic. Affects setObject and extendObject at least * make linter happy * make setObject async save * [chore]: deprecate setObjectAsync/setForeignObjectAsync (#2974) * deprecate setObjectAsync/setForeignObjectAsync * added tests and corrected types * fix the test --------- Co-authored-by: Bluefox <[email protected]> Co-authored-by: Max Hauser <[email protected]>
1 parent 541f7f0 commit 2c4baa4

File tree

3 files changed

+131
-33
lines changed

3 files changed

+131
-33
lines changed

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
Placeholder for the next version (at the beginning of the line):
44
## __WORK IN PROGRESS__
55
-->
6+
7+
## __WORK IN PROGRESS__
8+
* (@Apollon77) Fixes async usage of extendObject
9+
* (@Apollon77) Makes setObject async save
10+
* (@foxriver76) deprecated `set(Foreign)ObjectAsync` as the non async methods are now working correctly with promises
11+
612
## 7.0.3 (2024-11-13) - Lucy
713
* (@foxriver76) Introduce "Vendor Packages Workflow" (only relevant for vendors - see README.md)
814

‎packages/adapter/src/lib/adapter/adapter.ts‎

Lines changed: 67 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -319,9 +319,17 @@ export interface AdapterClass {
319319
commandsPermissions: CommandsPermissions,
320320
options?: unknown,
321321
): Promise<ioBroker.PermissionSet>;
322-
/** Creates or overwrites an object in the object db */
322+
/**
323+
* Creates or overwrites an object in the object db
324+
*
325+
* @deprecated use `adapter.setObject` without a callback instead
326+
*/
323327
setObjectAsync(id: string, obj: ioBroker.SettableObject, options?: unknown): ioBroker.SetObjectPromise;
324-
/** Creates or overwrites an object (which might not belong to this adapter) in the object db */
328+
/**
329+
* Creates or overwrites an object (which might not belong to this adapter) in the object db
330+
*
331+
* @deprecated use `adapter.setForeignObject` without a callback instead
332+
*/
325333
setForeignObjectAsync<T extends string>(
326334
id: T,
327335
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
@@ -2765,14 +2773,17 @@ export class AdapterClass extends EventEmitter {
27652773
this._intervals.delete(interval as NodeJS.Timeout);
27662774
}
27672775

2768-
setObject(id: string, obj: ioBroker.SettableObject, callback?: ioBroker.SetObjectCallback): Promise<void>;
2776+
setObject(
2777+
id: string,
2778+
obj: ioBroker.SettableObject,
2779+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
2780+
setObject(id: string, obj: ioBroker.SettableObject, callback: ioBroker.SetObjectCallback): void;
2781+
setObject(id: string, obj: ioBroker.SettableObject, options: unknown, callback: ioBroker.SetObjectCallback): void;
27692782
setObject(
27702783
id: string,
27712784
obj: ioBroker.SettableObject,
27722785
options: unknown,
2773-
callback?: ioBroker.SetObjectCallback,
2774-
): Promise<void>;
2775-
setObject(id: string, obj: ioBroker.SettableObject, callback?: ioBroker.SetObjectCallback): Promise<void>;
2786+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
27762787
/**
27772788
* Creates or overwrites an object in objectDB.
27782789
*
@@ -2803,7 +2814,12 @@ export class AdapterClass extends EventEmitter {
28032814
* }
28042815
* ```
28052816
*/
2806-
setObject(id: unknown, obj: unknown, options: unknown, callback?: unknown): Promise<void> | void {
2817+
setObject(
2818+
id: unknown,
2819+
obj: unknown,
2820+
options?: unknown,
2821+
callback?: unknown,
2822+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> {
28072823
if (typeof options === 'function') {
28082824
callback = options;
28092825
options = null;
@@ -2819,7 +2835,9 @@ export class AdapterClass extends EventEmitter {
28192835
return this._setObject({ id, obj: obj as ioBroker.SettableObject, options, callback });
28202836
}
28212837

2822-
private async _setObject(options: InternalSetObjectOptions): Promise<void> {
2838+
private async _setObject(
2839+
options: InternalSetObjectOptions,
2840+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> {
28232841
if (!this._defaultObjs) {
28242842
this._defaultObjs = (await import('./defaultObjs.js')).createDefaults();
28252843
}
@@ -2844,6 +2862,7 @@ export class AdapterClass extends EventEmitter {
28442862
this._utils.validateId(options.id, false, null);
28452863
} catch (e) {
28462864
this._logger.error(tools.appendStackTrace(`${this.namespaceLog} ${e.message}`));
2865+
// Error is logged and silently ignored to not break older adapters
28472866
return;
28482867
}
28492868
}
@@ -2921,11 +2940,11 @@ export class AdapterClass extends EventEmitter {
29212940
options.obj.user = options.obj.user || (options.options ? options.options.user : '') || SYSTEM_ADMIN_USER;
29222941
options.obj.ts = options.obj.ts || Date.now();
29232942

2924-
this._setObjectWithDefaultValue(options.id, options.obj, options.options, options.callback);
2925-
} else {
2926-
this._logger.error(`${this.namespaceLog} setObject ${options.id} mandatory property type missing!`);
2927-
return tools.maybeCallbackWithError(options.callback, 'mandatory property type missing!');
2943+
return this._setObjectWithDefaultValue(options.id, options.obj, options.options, options.callback);
29282944
}
2945+
2946+
this._logger.error(`${this.namespaceLog} setObject ${options.id} mandatory property type missing!`);
2947+
return tools.maybeCallbackWithError(options.callback, 'mandatory property type missing!');
29292948
}
29302949

29312950
/**
@@ -3332,14 +3351,23 @@ export class AdapterClass extends EventEmitter {
33323351
setForeignObject<T extends string>(
33333352
id: T,
33343353
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
3335-
callback?: ioBroker.SetObjectCallback,
3354+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
3355+
setForeignObject<T extends string>(
3356+
id: T,
3357+
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
3358+
callback: ioBroker.SetObjectCallback,
33363359
): void;
3360+
setForeignObject<T extends string>(
3361+
id: T,
3362+
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
3363+
options: unknown,
3364+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
33373365
setForeignObject<T extends string>(
33383366
id: T,
33393367
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
33403368
options: unknown,
33413369
callback?: ioBroker.SetObjectCallback,
3342-
): void;
3370+
): void | Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
33433371

33443372
/**
33453373
* Same as {@link AdapterClass.setObject}, but for any object.
@@ -3357,7 +3385,12 @@ export class AdapterClass extends EventEmitter {
33573385
* }
33583386
* ```
33593387
*/
3360-
setForeignObject(id: unknown, obj: unknown, options: unknown, callback?: unknown): MaybePromise {
3388+
setForeignObject(
3389+
id: unknown,
3390+
obj: unknown,
3391+
options?: unknown,
3392+
callback?: unknown,
3393+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> | void {
33613394
if (typeof options === 'function') {
33623395
callback = options;
33633396
options = null;
@@ -3386,7 +3419,9 @@ export class AdapterClass extends EventEmitter {
33863419
return this._setForeignObject({ id, obj: obj as ioBroker.SettableObject, options, callback });
33873420
}
33883421

3389-
private _setForeignObject(_options: InternalSetObjectOptions): MaybePromise {
3422+
private _setForeignObject(
3423+
_options: InternalSetObjectOptions,
3424+
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> | void {
33903425
const { options, callback, obj } = _options;
33913426
let { id } = _options;
33923427

@@ -3426,21 +3461,30 @@ export class AdapterClass extends EventEmitter {
34263461
}
34273462
}
34283463

3429-
this._setObjectWithDefaultValue(id, obj, options, callback);
3464+
return this._setObjectWithDefaultValue(id, obj, options, callback);
34303465
}
34313466

34323467
// external signatures
34333468
extendForeignObject<T extends string>(
34343469
id: T,
34353470
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
3436-
callback?: ioBroker.SetObjectCallback,
3471+
): Promise<ioBroker.CallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
3472+
extendForeignObject<T extends string>(
3473+
id: T,
3474+
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
3475+
callback: ioBroker.SetObjectCallback,
34373476
): void;
34383477
extendForeignObject<T extends string>(
34393478
id: T,
34403479
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
34413480
options: ioBroker.ExtendObjectOptions,
3442-
callback?: ioBroker.SetObjectCallback,
3481+
callback: ioBroker.SetObjectCallback,
34433482
): void;
3483+
extendForeignObject<T extends string>(
3484+
id: T,
3485+
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
3486+
options: ioBroker.ExtendObjectOptions,
3487+
): Promise<ioBroker.CallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
34443488

34453489
/**
34463490
* Same as {@link AdapterClass.extendObject}, but for any object.
@@ -3461,7 +3505,7 @@ export class AdapterClass extends EventEmitter {
34613505
extendForeignObject(
34623506
id: unknown,
34633507
obj: unknown,
3464-
options: unknown,
3508+
options?: unknown,
34653509
callback?: unknown,
34663510
): Promise<ioBroker.CallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> | void {
34673511
if (typeof options === 'function') {
@@ -9316,7 +9360,7 @@ export class AdapterClass extends EventEmitter {
93169360
return tools.maybeCallbackWithError(callback, e);
93179361
}
93189362
}
9319-
this.#states.delState(id, callback);
9363+
await this.#states.delState(id, callback);
93209364
}
93219365

93229366
// external signature
@@ -9784,7 +9828,7 @@ export class AdapterClass extends EventEmitter {
97849828

97859829
subs[pattern][this.namespace]++;
97869830
this.outputCount++;
9787-
this.#states.setState(`system.adapter.${autoSubEntry}.subscribes`, JSON.stringify(subs));
9831+
await this.#states.setState(`system.adapter.${autoSubEntry}.subscribes`, JSON.stringify(subs));
97889832
}
97899833
}
97909834

@@ -10013,7 +10057,7 @@ export class AdapterClass extends EventEmitter {
1001310057
delete subs[pattern];
1001410058
}
1001510059
this.outputCount++;
10016-
this.#states.setState(`system.adapter.${autoSub}.subscribes`, JSON.stringify(subs));
10060+
await this.#states.setState(`system.adapter.${autoSub}.subscribes`, JSON.stringify(subs));
1001710061
}
1001810062
}
1001910063
}

‎packages/controller/test/lib/testObjectsFunctions.ts‎

Lines changed: 58 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,53 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
8484
});
8585
});
8686

87-
//extendObject
87+
it(`${testName}setObject without callback is async`, async () => {
88+
const id = `${gid}AsyncNoCb`;
89+
90+
const res = await context.adapter.setObject(id, {
91+
type: 'state',
92+
common: {
93+
name: 'test1',
94+
type: 'number',
95+
role: 'level',
96+
read: true,
97+
write: true,
98+
},
99+
native: {
100+
attr1: '1',
101+
attr2: '2',
102+
attr3: '3',
103+
},
104+
});
105+
106+
expect(res).to.be.ok;
107+
expect(res.id).to.be.equal(`${context.adapter.namespace}.${id}`);
108+
});
109+
110+
it(`${testName}setForeignObject without callback is async`, async () => {
111+
const id = `${context.adapterShortName}.0.${gid}ForeignAsyncNoCb`;
112+
113+
const res = await context.adapter.setForeignObject(id, {
114+
type: 'state',
115+
common: {
116+
name: 'test1',
117+
type: 'number',
118+
role: 'level',
119+
read: true,
120+
write: true,
121+
},
122+
native: {
123+
attr1: '1',
124+
attr2: '2',
125+
attr3: '3',
126+
},
127+
});
128+
129+
expect(res).to.be.ok;
130+
expect(res.id).to.be.equal(id);
131+
});
132+
133+
// extendObject
88134
it(`${testName}Check if objects will be extended`, function (done) {
89135
context.adapter.extendObject(
90136
gid,
@@ -1363,7 +1409,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
13631409
// should use def as default state value on extendObject when obj non-existing
13641410
it(`${testName}Check extendObject state with def`, async function () {
13651411
this.timeout(3_000);
1366-
let obj = await context.adapter.extendObjectAsync('testDefaultValExtend', {
1412+
let obj = await context.adapter.extendObject('testDefaultValExtend', {
13671413
type: 'state',
13681414
common: {
13691415
type: 'string',
@@ -1392,7 +1438,7 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
13921438
// delete state but not object
13931439
await context.adapter.delStateAsync('testDefaultValExtend');
13941440
// extend it again - def should be created again, because state has been removed - now we set a def object
1395-
obj = await context.adapter.extendObjectAsync('testDefaultValExtend', {
1441+
obj = await context.adapter.extendObject('testDefaultValExtend', {
13961442
common: {
13971443
def: { hello: 'world' },
13981444
},
@@ -1408,7 +1454,8 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
14081454

14091455
// should use def as default state value on extendForeignObject when obj non-existing
14101456
it(`${testName}Check extendForeignObject state with def`, async () => {
1411-
let obj = await context.adapter.extendForeignObjectAsync('foreign.0.testDefaultValExtend', {
1457+
const id = 'foreign.0.testDefaultValExtend';
1458+
let obj = await context.adapter.extendForeignObject(id, {
14121459
type: 'state',
14131460
common: {
14141461
type: 'string',
@@ -1417,35 +1464,36 @@ export function register(it: Mocha.TestFunction, expect: Chai.ExpectStatic, cont
14171464
});
14181465

14191466
expect(obj).to.be.ok;
1467+
expect(obj?.id).to.be.equal(id);
14201468

1421-
let state = await context.adapter.getForeignStateAsync('foreign.0.testDefaultValExtend');
1469+
let state = await context.adapter.getForeignStateAsync(id);
14221470
expect(state!.val).to.equal('Run Forrest, Run!');
14231471
expect(state!.ack).to.equal(true);
14241472

14251473
// when state already exists def should not override
1426-
obj = await context.adapter.extendForeignObjectAsync('foreign.0.testDefaultValExtend', {
1474+
obj = await context.adapter.extendForeignObjectAsync(id, {
14271475
common: {
14281476
def: 'Please, do not set me up',
14291477
},
14301478
});
14311479

14321480
expect(obj).to.be.ok;
14331481

1434-
state = await context.adapter.getForeignStateAsync('foreign.0.testDefaultValExtend');
1482+
state = await context.adapter.getForeignStateAsync(id);
14351483
expect(state!.val).to.equal('Run Forrest, Run!');
14361484

14371485
// delete state but not object
1438-
await context.adapter.delForeignStateAsync('foreign.0.testDefaultValExtend');
1486+
await context.adapter.delForeignStateAsync(id);
14391487
// extend it again - def should be created again, because state has been removed - now we set a def object
1440-
obj = await context.adapter.extendForeignObjectAsync('foreign.0.testDefaultValExtend', {
1488+
obj = await context.adapter.extendForeignObjectAsync(id, {
14411489
common: {
14421490
def: { hello: 'world' },
14431491
},
14441492
});
14451493

14461494
expect(obj).to.be.ok;
14471495

1448-
state = await context.adapter.getForeignStateAsync('foreign.0.testDefaultValExtend');
1496+
state = await context.adapter.getForeignStateAsync(id);
14491497
// @ts-expect-error TODO do we want this auto parsing?
14501498
expect(state!.val!.hello).to.equal('world');
14511499
expect(state!.ack).to.equal(true);

0 commit comments

Comments
 (0)