fix: HydratedBloc storage doesn't change without closing previous storage

[mahdi739]

Description
When I try to change the HydratedBloc.storage variable, It won't change, unless I close the previous storage.

Steps To Reproduce

1. Create a hydrated bloc (cubit)
2. Build and set a HydratedStorage with a path
3. Change the state of bloc
4. Build and set a different HydratedStorage with a new path
5. Renew the Bloc(cubit) object;
6. It supposed to have the initial state but it load the state from previous storage which means the HydratedStorage is not changed.

Simple code to reproduce it:

import 'dart:io';
import 'package:hydrated_bloc/hydrated_bloc.dart';

class CounterCubit extends HydratedCubit<int> {
  CounterCubit() : super(0);

  void increment() => emit(state + 1);

  @override
  int fromJson(Map<String, dynamic> json) => json['value'] as int;

  @override
  Map<String, int> toJson(int state) => {'value': state};
}

Future<void> main(List<String> arguments) async {
  HydratedBloc.storage = await HydratedStorage.build(storageDirectory: Directory("source_1")..createSync());
  var counterCubit = CounterCubit();
  counterCubit.increment();
  print(counterCubit.state); // 1 ✔
  HydratedBloc.storage = await HydratedStorage.build(storageDirectory: Directory("source_2")..createSync());
  counterCubit = CounterCubit();
  counterCubit.increment();
  print(counterCubit.state); // 2 ❌
}

We have to close the previous storage, however If we don't, I think the method should do it automatically or throw an exception.
When nothing changes and no exceptions are thrown, debugging can be difficult.

[codesculpture]

I guess, it safe to throw error if we try to build storage when already a storage lives (i.e not closed) ?. @felangel

[felangel]

I guess, it safe to throw error if we try to build storage when already a storage lives (i.e not closed) ?. @felangel

That seems reasonable. The more user-friendly option in my opinion would be to automatically close the previous storage and register the new storage since that's likely what users would want in this case. Wdyt?

[codesculpture]

Well thats what shld be done, am wondering does the user are aware that if they build new storage and existing one would deleted without doing "clear". Since as of now, user may know the only way to delete the storage instance is by doing "clear". Well i agree to clear automatically, but wondering that doesnt bother existing users who are using already. Does this be considered be breaking change.

[felangel]

Well thats what shld be done, am wondering does the user are aware that if they build new storage and existing one would deleted without doing "clear". Since as of now, user may know the only way to delete the storage instance is by doing "clear". Well i agree to clear automatically, but wondering that doesnt bother existing users who are using already. Does this be considered be breaking change.

Technically it would be but this seems more like a bug since I can't imagine many people would expect/rely on the current behavior. I would personally treat this as a bug fix since the current behavior is incorrect.

[codesculpture]

Well agreed to clear the existing one, will write a PR asap and will let u know. Thanks for ur time felix

[felangel]

Well agreed to clear the existing one, will write a PR asap and will let u know. Thanks for ur time felix

Thanks for your time and help as always! 🙏

[codesculpture]

Its been a while we talked about this, the fix am trying to land
That would introduce this, at a single time there must only one storage can live with the same path.
If anyone need to create a storage in the same the path, they must close the existing one but if they
fail to, there is two storage points to same path, which could wait forever due to another storage is using already would throw error.

But as of our current impl, if user already building the multiple storage at same path its not breaking due to the fact we are just handing the same storage instance even if they try to create multiple times.

So after this fix, if one might using in this way, the app could break. But this is totally wrong behavior and the right thing is to throw if they using same storages at same path at a same time. What do u think @felangel :/ .

Btw, its been so long we talked about this 😂