NestedTypeRegistry: increase type limit with backward compatibility and id overriding

[xal]

I am not sure about API, perhaps it may be implemented in a better way. Fork with this PR used in Fedi app - https://github.com/Big-Fig/Fediverse.app I combined two features in one PR because they work better together. The CI/CD pipeline fails because it fails in the original hive master branch.

Goal

• increase the limit of possible types from 224 to (224*224)
• add the possibility to specify typeId for adapter during registration
• Backward compatibility with existing hive files

Added

• OverrideIdAdapter to set up typeId outside annotations
• createNestedTypeRegistryAdapter & registerNestedTypeRegistryAdapter to TypeRegistry

Not implemented

• performance check. NestedTypeRegistryAdapter may be slower than simple adapters
• example for new features & documentation
• tests

Related

• Fix for #525
• Possible fix for #505
• Fix for #735
• Fix for #718
• Possible fix for #505

Readme Update

Hive has limitation 224 (256 - 32 reserved) types limit per type registry. However, you can use Hive.createNestedTypeRegistryAdapter & Hive.registerNestedTypeRegistryAdapter. Pros:

• avoid typeIds conflicts if you several libraries with own adapters list and ids conflict
• you can use 224*224 type ids
• compatible with already exist Hive storages Cons:
• slower performance

@HiveType(typeId: 0)
class Person extends HiveObject {
}

@HiveType(typeId: 0)
class Home extends HiveObject {
}
@HiveType(typeId: 1)
class Work extends HiveObject {
}

// 1 is typeId
var nestedAdapter = Hive.createNestedTypeRegistryAdapter(1);

nestedAdapter.registerAdapter(HomeAdapter());
nestedAdapter.registerAdapter(WorkAdapter());

Hive.registerAdapter(PersonAdapter());
Hive.registerAdapter(nestedAdapter);

Another possible issue is required typeId field in @HiveType. Sometimes it is better to specify all ids in one files during registration. You can achieve this via OverrideIdAdapter

Hive.registerAdapter(
  OverrideIdAdapter(
    100,
    PersonAdapter(),
  ),
);

[themisir]

I've recently fixed CI pipeline to make sure we won't publish a broken package, so can you please rebase with upstream to check if tests passes or not?

git fetch upstream
git merge upstream/master # You can also use "rebase" instead of "merge" but you'll have to force push the changes

[themisir]

It looks like the PR doesn't passes tests.. Would you like to work on failing tests, or I can implement this feature myself if you're not able to work on this.

[xal]

It looks like the PR doesn't passes tests.. Would you like to work on failing tests, or I can implement this feature myself if you're not able to work on this.

Thanks for the feedback, I saw CI fail but didn't have enough time to fix. I plan to prepare fix in 1-2 days. However, I still not sure about API design, so I think you can implement your solution(or modify my) if you have better ideas regarding API

[xal]

@themisir I fixed CI fail and added tests for features added in this PR

[xal]

@themisir Hi, any updates on this? Perhaps you want additional changes in PR?

[themisir]

Hi, sorry for late response. I'm just thinking if is there a "better" way of implementing nested registries. Don't get me wrong your implementations is very good but I worry if it might confuse some users.

Also we might use this chance to improve hive usage on third party libraries. Currently if some third party lib uses hive they have to share same typeid space with the app and other libraries, and it's hard to keep track of which typeid is used by which package. So, maybe if we implement nested adapters as something like namespaces for type adapters, and consumers can use strings instead of ints for declaring namespaces for nested adapters. I know that using ints for identification would be much faster but also might cause collision issues with 3rd party libraries if 2 lib used same typeId for registering their type sets.

And on backend side we could use a reserved typeId for serializing types in namespace <Reserved TypeId: int> + <Namespace: string> + <TypeId: int>. Yes it's a bit longer and probably a lot slower to resolve those types, but it also solves collision issues that might be caused by different packages as long as they use different identifiers.

Hive.registerTypespace('hivecache', (registry) => {
  registry.registerAdapter(const CacheEntityAdapter());
});

Or we could ditch out namespace strings but instead use registration order to serialize nested types as <Reserved TypeId: int> + <Namespace Index: int> + <TypeId: int>. And consumers of namespace api would have to write something like:

// package:hivecache/hive.dart
HiveTypespace createHiveCacheTypespace() {
  return Hive.createTypespace()
    ..registerAdapter(const CacheEntityAdapter());
}

// package:myapp/myapp.dart
void main() {
  // order of `Hive.registerTypespace` calls matter here.
  Hive.registerTypespace(createHiveCacheTypespace());
  Hive.registerTypespace(Hive.createTypespace()
    ..registerAdapter(SomeAdapter()));
}

This would be probably a bit faster to consume but also a bit easier to mess up since namespaces is just identified by their registration index. Of course we can also let users to use custom namespace ID rather than registration index: Hive.registerTypespace(id: number, typespace: HiveTypespace). But we have to discourage package maintainers to automatically registering their own namespaces, instead let users to decide which namespace id they want to use for specific namespaces to reduce chance of ID collision.

By the way I just want to discuss about that a bit, before implementation, maybe your current implementation is the best we can get, maybe my arguments are not worth dealing with the performance or implementation cost. What do you think about that. Also if we could decide a way to do this that's different from your implementation don't worry about implementing it, I could implement it myself if needed, let's just figure out how we could implement this.

[xal]

• I think the solution should have backward compatibility or at least a super-quality migration tool. Otherwise, a lot of people just will stop updating Hive and dive in dependency version conflicts hell during updates to other libraries.
• I still don't fully understand why Adapter ID is declared in HiveType annotation (in the previous versions adapter id was declared outside during registering). It is really hard to maintain used Hive Type IDs when they are declared in 100+ different files.
• Typespace/Namespace looks interesting, but It looks like it is not possible to implement it with backward compatibility
• Also, I think libraries authors(who use Hive) shouldn't specify any hardcoded TypeIds(perhaps only default values for quickstart) due to possible conflicts with other libraries. App owners should have the ability to specify own unique ID for each library registry. Two different apps can't share the same hive DB file, so it is not important to have hardcoded TypeIds in the library.

P.S. I don't like my solution from PR, it really looks hacky and it should be reworked. All my projects use different approaches and patterns. So it's really hard for me to enhance Hive

[themisir]

• I think the solution should have backward compatibility or at least a super-quality migration tool. Otherwise, a lot of people just will stop updating Hive and dive in dependency version conflicts hell during updates to other libraries.

I completely agree with that.

• I still don't fully understand why Adapter ID is declared in HiveType annotation (in the previous versions adapter id was declared outside during registering). It is really hard to maintain used Hive Type IDs when they are declared in 100+ different files.

I don't know actually. The change was applied before I started maintaining hive, probably to make the api more "user-friendly". But changing that case is both outside of scope of this PR and might cause backward incompatibility.

• Typespace/Namespace looks interesting, but It looks like it is not possible to implement it with backward compatibility

I think it's possible to implement those concepts as a new non-breaking feature. It will be backwards compatible in a way, because there's no concept of namespace in previous versions that we have to be compatible with. And in theory we can implement namespaces using one of the reserved type ids just like your implementation.

[heafox]

TypeID 223 is too few

[NachiketaVadera]

@themisir @xal

Hey guys. Really looking forward for this feature. Any updates on this?

[themisir]

Hey guys. Really looking forward for this feature. Any updates on this?

I decided that I don't want this to be implemented. If anyone has too many models they are probably better off using a proper relational database like sqlite.

Adding this will lead to another complexity layer -> more foot-guns -> more confusion among the user -> yet another thing to keep track of. It's a trade off (to be unusable for such cases where a user might have lots of models) that I am willing to accept.

However hive being a community product rather than something I own, I am open to your feedback in regards of that choice. My opinion is that adding a workaround is going to reduce quality of the package and lead to unnecessary complexity.

[NachiketaVadera]

My opinion is that adding a workaround is going to reduce quality of the package and lead to unnecessary complexity.

I agree with this but does the API affect everyone who uses the package or only people who opt to use it? Speaking from my perspective, the limited typeIds is a big issue if I want to really scale my app without moving the existing storage to something like Isar (which I want to eventually).

I think that if the API is backward compatible and only affects users who consume that specific API, I'd say that it's something that should be implemented with proper caution in documentation of the feature.

I understand if you or @xal decide against this but as someone who's been using hive throughout my organization, this feature (or another solution to typeIds) is seriously need.

Edit: fixed formatting & tagging

[themisir]

Hey @NachiketaVadera,

Thanks for providing your input, I appreciate it and will take into consideration.

I will try adding nested type registry support in coming days (can't provide any ETA tho, might as well take a few weeks since I'm on holiday break right now, trying to stay away from coding for some time) using the existing implementation by @xal. I need to re-review and see what's missing or needs to be changed. I will keep you updated ☺️

Best wishes and Happy new year!

[themisir]

I merged this into a temporary nested-adapters branch to make some modifications, then I'll merge the final changes into master.

[Mamasodikov]

We need this feature as we have a big app