Add missing path parameter to HiveInterface methods

[wbusey0]

Adds String? path parameters to HiveInterface methods

[themisir]

Why do you think this is needed? If you want to delete box by name you can use File(path).delete() instead. And same applies for box exists method. HiveInterface doesn't uses path parameter because it tries to be compatible with both io and web platforms. Web platforms doesn't uses filesystem paths so Hive interface doesn't uses it as well.

If there's special need we've missing please let me know..

[wbusey0]

Hi @themisir ,

Thanks for the quick response!

We are using Hive in a way where (some) boxes are saved in a different subpath (let's say on a per-user basis). So we heavily use the path parameter with the openBox methods.

I saw the path parameter is already there in the HiveImpl class but we can't use it because not in the interface.
BackendManagerInterface and subclasses already have the path parameter for both methods.
So because of that it seemed weird that only HiveInterface is missing the path parameter which results in HiveImpl always passing homePath instead.

Furthermore openBox & openLazyBox are both in HiveImpl and both have an optional path parameter.

The reason I think it is better to go through the Hive interface instead of using File is to hide implementation details. For example Hive could change so that more than one file is stored and all of them would need to be deleted. That would require us checking the Hive implementation to be sure that our implementations for boxExists and deleteBox are still up to date.

Also I check the implementations and it already seems some more logic is present in these methods:

• deleteBox deleted hivec & lock files as well
• boxExists uses toLowerCase() on the name and also checks for the above mentioned files

But I didn't think about the web platform to be fair :-)

(also see issue #742).

[wbusey0]

@themisir Could you tell me what you think about our usecases for this?

[themisir]

@themisir Could you tell me what you think about our usecases for this?

Oh thanks for pinging me, I totally forget about that issue. My feelings about the feature is a bit complex cause this is nice to have feature for advanced use cases but might confuse new users (especially the ones that uses Hive for web only).

[themisir]

Sorry for the late response but I don't have enough bandwidth for adding new features to hive. I'm not exactly sure how could new features would affect users or could cause issues + might lead to new feature requests (eg: path will not work on web, so there might be someone requesting that to work). So I'm unsure about merging this feature. What do you think about that?

[wbusey0]

My main argument for doing this would be consistency. It's kind of weird that you can open a box with path but not delete it. Having the path parameter there but not in the other methods is in my opinion more confusing.
Of course the inconsistency could also be resolved by removing the path parameter for openBox method, but obviously I'm not vouching for that ;) .

I don't think we can solve our case by calling the Hive.initFlutter() multiple times as we have multiple parts of the app needing boxes in other subdirectories.

Other ways to solve it would indeed be to delete the relevant Hive files manually by checking the deleteBoxFromDisk implementation; which is what we'll be doing. I thought about casting the Hive object to HiveImpl but that's not possible as it is not part of the public API.

[themisir]

My main argument for doing this would be consistency. It's kind of weird that you can open a box with path but not delete it. Having the path parameter there but not in the other methods is in my opinion more confusing.
Of course the inconsistency could also be resolved by removing the path parameter for openBox method, but obviously I'm not vouching for that ;) .

Haven't noticed that openBox supports custom path parameter. So it makes sense to support path parameter for other methods too.

[wbusey0]

Thank you!
I think the the web implementation needs those parameters as well, there were some checks failing on this PR?

[themisir]

I think the the web implementation needs those parameters as well

Hive web uses different implementation (using indexed db) which doesn't relies on file system / so it doesn't uses path parameter.

there were some checks failing on this PR?

I'm currently working on fixing pipeline here #806