Skip to content

Commit

Permalink
Makes sure to return promise from setObject logic. … (#2967)
Browse files Browse the repository at this point in the history 
* 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]>
  • Loading branch information
@Apollon77 @GermanBluefox @foxriver76
3 people authored Nov 27, 2024
1 parent 541f7f0 commit 2c4baa4
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 33 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@
Placeholder for the next version (at the beginning of the line):
## __WORK IN PROGRESS__
-->

## __WORK IN PROGRESS__
* (@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)

Expand Down
90 changes: 67 additions & 23 deletions packages/adapter/src/lib/adapter/adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,9 +319,17 @@ export interface AdapterClass {
commandsPermissions: CommandsPermissions,
options?: unknown,
): Promise<ioBroker.PermissionSet>;
/** 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<T extends string>(
id: T,
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
Expand Down Expand Up @@ -2765,14 +2773,17 @@ export class AdapterClass extends EventEmitter {
this._intervals.delete(interval as NodeJS.Timeout);
}

setObject(id: string, obj: ioBroker.SettableObject, callback?: ioBroker.SetObjectCallback): Promise<void>;
setObject(
id: string,
obj: ioBroker.SettableObject,
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
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<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
/**
* Creates or overwrites an object in objectDB.
*
Expand Down Expand Up @@ -2803,7 +2814,12 @@ export class AdapterClass extends EventEmitter {
* }
* ```
*/
setObject(id: unknown, obj: unknown, options: unknown, callback?: unknown): Promise<void> | void {
setObject(
id: unknown,
obj: unknown,
options?: unknown,
callback?: unknown,
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> {
if (typeof options === 'function') {
callback = options;
options = null;
Expand All @@ -2819,7 +2835,9 @@ export class AdapterClass extends EventEmitter {
return this._setObject({ id, obj: obj as ioBroker.SettableObject, options, callback });
}

private async _setObject(options: InternalSetObjectOptions): Promise<void> {
private async _setObject(
options: InternalSetObjectOptions,
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> {
if (!this._defaultObjs) {
this._defaultObjs = (await import('./defaultObjs.js')).createDefaults();
}
Expand All @@ -2844,6 +2862,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;
}
}
Expand Down Expand Up @@ -2921,11 +2940,11 @@ 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);
} else {
this._logger.error(`${this.namespaceLog} setObject ${options.id} mandatory property type missing!`);
return tools.maybeCallbackWithError(options.callback, 'mandatory property type missing!');
return this._setObjectWithDefaultValue(options.id, options.obj, options.options, options.callback);
}

this._logger.error(`${this.namespaceLog} setObject ${options.id} mandatory property type missing!`);
return tools.maybeCallbackWithError(options.callback, 'mandatory property type missing!');
}

/**
Expand Down Expand Up @@ -3332,14 +3351,23 @@ export class AdapterClass extends EventEmitter {
setForeignObject<T extends string>(
id: T,
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
callback?: ioBroker.SetObjectCallback,
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
setForeignObject<T extends string>(
id: T,
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
callback: ioBroker.SetObjectCallback,
): void;
setForeignObject<T extends string>(
id: T,
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
options: unknown,
): Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
setForeignObject<T extends string>(
id: T,
obj: ioBroker.SettableObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
options: unknown,
callback?: ioBroker.SetObjectCallback,
): void;
): void | Promise<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback>>;

/**
* Same as {@link AdapterClass.setObject}, but for any object.
Expand All @@ -3357,7 +3385,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<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> | void {
if (typeof options === 'function') {
callback = options;
options = null;
Expand Down Expand Up @@ -3386,7 +3419,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<ioBroker.NonNullCallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> | void {
const { options, callback, obj } = _options;
let { id } = _options;

Expand Down Expand Up @@ -3426,21 +3461,30 @@ export class AdapterClass extends EventEmitter {
}
}

this._setObjectWithDefaultValue(id, obj, options, callback);
return this._setObjectWithDefaultValue(id, obj, options, callback);
}

// external signatures
extendForeignObject<T extends string>(
id: T,
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
callback?: ioBroker.SetObjectCallback,
): Promise<ioBroker.CallbackReturnTypeOf<ioBroker.SetObjectCallback>>;
extendForeignObject<T extends string>(
id: T,
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
callback: ioBroker.SetObjectCallback,
): void;
extendForeignObject<T extends string>(
id: T,
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
options: ioBroker.ExtendObjectOptions,
callback?: ioBroker.SetObjectCallback,
callback: ioBroker.SetObjectCallback,
): void;
extendForeignObject<T extends string>(
id: T,
objPart: ioBroker.PartialObject<ioBroker.ObjectIdToObjectType<T, 'write'>>,
options: ioBroker.ExtendObjectOptions,
): Promise<ioBroker.CallbackReturnTypeOf<ioBroker.SetObjectCallback>>;

/**
* Same as {@link AdapterClass.extendObject}, but for any object.
Expand All @@ -3461,7 +3505,7 @@ export class AdapterClass extends EventEmitter {
extendForeignObject(
id: unknown,
obj: unknown,
options: unknown,
options?: unknown,
callback?: unknown,
): Promise<ioBroker.CallbackReturnTypeOf<ioBroker.SetObjectCallback> | void> | void {
if (typeof options === 'function') {
Expand Down Expand Up @@ -9316,7 +9360,7 @@ export class AdapterClass extends EventEmitter {
return tools.maybeCallbackWithError(callback, e);
}
}
this.#states.delState(id, callback);
await this.#states.delState(id, callback);
}

// external signature
Expand Down Expand Up @@ -9784,7 +9828,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));
}
}

Expand Down Expand Up @@ -10013,7 +10057,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));
}
}
}
Expand Down
68 changes: 58 additions & 10 deletions packages/controller/test/lib/testObjectsFunctions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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' },
},
Expand All @@ -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',
Expand All @@ -1417,35 +1464,36 @@ 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',
},
});

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' },
},
});

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);
Expand Down

0 comments on commit 2c4baa4

Please sign in to comment.