Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Makes sure to return promise from setObject logic. … #2967

Merged
merged 7 commits into from
Nov 27, 2024
Merged

Makes sure to return promise from setObject logic. … #2967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
98bf7b4
Makes sure to return promise from setObject logic. Affects setObject …
Apollon77 Nov 15, 2024
cf9620a
make linter happy
Apollon77 Nov 15, 2024
470d749
make setObject async save
Apollon77 Nov 15, 2024
b5653b0
Merge branch 'master' into fix-setopbject-return
GermanBluefox Nov 15, 2024
f57945b
Merge branch 'master' into fix-setopbject-return
foxriver76 Nov 17, 2024
57b76ef
[chore]: deprecate setObjectAsync/setForeignObjectAsync (#2974)
foxriver76 Nov 27, 2024
e8f288d
Merge branch 'master' into fix-setopbject-return
Apollon77 Nov 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading