HAP-NodeJS icon indicating copy to clipboard operation
HAP-NodeJS copied to clipboard

Using my own storage provider instead of node-persist

Open hippotastic opened this issue 3 years ago • 5 comments

Describe Your Problem:

First of all, I'm not sure if I selected the correct issue type for my request. Sorry if this causes confusion. This is neither a bug, nor a real support request, it's more of a request to improve the architecture of HAP-NodeJS to make using it as a library more flexible.

I'd like to integrate HAP-NodeJS into my own automation system, and stumbled upon the following code in both Core.ts and BridgedCore.ts:

import storage from 'node-persist';

// [...]

// Initialize our storage system
storage.initSync();

I also noticed that calls to this singleton-like storage system are made deep inside HAP-NodeJS from various models like IdentifierCache.ts, AccessoryInfo.ts, ControllerStorage.ts and others. I also noticed that a persist folder gets created and some files inside that folder are created and managed by this storage system.

My automation system has its own database with support for multiple configuration branches, rollbacks to previous configurations etc., and I'm not particularly happy with the fact that HAP-NodeJS uses a singleton storage system under the hood without me being able to plug in a different storage system.

I'd love to see HAP-NodeJS use dependency injection instead, e.g. by having users of the library create a storage instance that implements a certain interface and then simply passing this instance as a reference to HAP-NodeJS when creating a Bridge instance. Using dependency injection would make it trivial to replace the storage system, or integrate it with my own database.

In case no own storage instance is passed to HAP-NodeJS, it could internally create one that uses node-persist to maintain backwards compatibility.

Would you consider making such an architectural change to HAP-NodeJS?

hippotastic avatar Dec 27 '20 18:12 hippotastic

The node-persist library is a legacy library we carry since along time. There are already ambitions to replace that with a new (probably custom built) solution. I see we could introduce a modular storage provider system like you proposed it. Though I can't guarantee the timeline of it.

Reference: https://github.com/homebridge/HAP-NodeJS/projects/4#card-37486119

EDIT: this is kind of related to #629. Though we kind of decided that we want to move completely away from node-persist. Ideally we would incorporate a storage system built around Promises, moving away from blocking operations.

To give a better scope of what is planned. The new Storage API should be integrated into HAP-NodeJS in a way that we can use it for homebridge as well. There is also plans for the homebridge project to create a "Plugin Storage API", so we have a unified way of storing homebridge plugin related data to disk (or whatever storage provider is plugged in).

Supereg avatar Dec 27 '20 18:12 Supereg

This issue maybe considered for the upcoming v1.0.0 release. Though can't promise anything. First step for now is to finally get rid of the legacy node-persist dependency. So not promising anything here. I need to see if we can already include a full blown rework of the storage system.

Supereg avatar Feb 17 '21 14:02 Supereg

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Nov 05 '21 18:11 github-actions[bot]

This is not stale in my opinion, the issue persists. I still cannot control how persistence is handled, and v1.0.0 hasn't been released yet. I would still be very grateful if this could be handled somehow. :)

hippotastic avatar Nov 05 '21 18:11 hippotastic

Yes. This issue was accidentally considered stale. Forgot to create an exempt for the vision label. Fixed in https://github.com/homebridge/HAP-NodeJS/commit/8e612a364b70c4acb200223f01b9c8b6ff4e4eed.

Supereg avatar Nov 05 '21 19:11 Supereg