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.

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

Using my own storage provider instead of node-persist #864

Open
hippotastic opened this issue Dec 27, 2020 · 5 comments
Open

Using my own storage provider instead of node-persist #864

hippotastic opened this issue Dec 27, 2020 · 5 comments

Comments

@hippotastic
Copy link

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?

@Supereg
Copy link
Member

Supereg commented Dec 27, 2020

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 Supereg added enhancement long running Issues which should be excluded from the stale bot and removed question labels Dec 27, 2020
@Supereg Supereg added the vision label Feb 14, 2021
@Supereg
Copy link
Member

Supereg commented Feb 17, 2021

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 Supereg removed the long running Issues which should be excluded from the stale bot label Feb 17, 2021
@github-actions
Copy link

github-actions bot commented Nov 5, 2021

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 github-actions bot added the stale label Nov 5, 2021
@Supereg Supereg removed the stale label Nov 5, 2021
@hippotastic
Copy link
Author

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. :)

@Supereg
Copy link
Member

Supereg commented Nov 5, 2021

Yes. This issue was accidentally considered stale. Forgot to create an exempt for the vision label. Fixed in 8e612a3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants