atsign-foundation / at_widgets

Flutter widgets which aid in building applications using Atsign's technology
https://docs.atsign.com
BSD 3-Clause "New" or "Revised" License
52 stars 33 forks source link

Hive type IDs clash with pre-existing IDs when integrating the atPlatform into an existing app #827

Open sotomski opened 3 months ago

sotomski commented 3 months ago

Which at_widget package you find issue with?

at_onboarding_flutter

On what platforms did you experience the issue?

Android

What version are you using?

6.1.7

What happened?

Current behavior: The onboarding process of an atsign fails on the mobile app, because some of Hive type ids used by the app are conflicting the ids used by the atSign SDK. This causes Hive to confuse registered type adapters, which breaks atSign onboarding (and everything else as well, I suppose). As a consequence, it's hard to integrate atSign SDK into an existing app.

Expected behavior: atSign SDK provides a possibility to set type ids to each of its adapters during initialization (with reasonable defaults, as this problem won't affect everyone).

Possible workaround:

  1. Run atSDK on a separate isolate. This works, but isn't ideal since communication between isolates is a burden.
  2. Change type id used by the app. This requires migration of all existing users to the new type id before atSDK could be deployed with the app.

Steps to reproduce

1. Create a class meant to be serialized and stored by Hive.
2. Set its type id to 3. `@HiveType(typeId: 3)`
3. Register the generated adapter as usual.
4. Try to onboard an atSign.

Additional info

No response

Relevant log output

I/flutter ( 3739): SEVERE|2024-03-28 16:22:50.448953|LocalSecondary (@atSign)|exception in local update:Hive error adding to commit log:HiveError: Cannot write, unknown type: CommitOp. Did you forget to register an adapter? 
I/flutter ( 3739): SEVERE|2024-03-28 16:22:50.449313|Onboarding Service|error in authenticating =>  Hive error adding to commit log:HiveError: Cannot write, unknown type: CommitOp. Did you forget to register an adapter?

Flutter analyze output

No response

Flutter doctor output

No response

gkc commented 3 months ago

@sotomski As a stop-gap, you can do this somewhere during your app's startup before it does anything with any Atsign package. I've tested this with a command-line client

// ignore: depend_on_referenced_packages
import 'package:at_persistence_secondary_server/src/utils/type_adapter_util.dart';

and

  // Change the typeIds used by atSign which are 0 to 10 inclusive by default
  for (final typeName in typeAdapterMap.keys) {
    typeAdapterMap[typeName] = typeAdapterMap[typeName] + 100;
  }

I'm open to suggestions about how to make this cleaner. In the short term I can make a PR to at least have at_persistence_secondary_server export type_adapter_util.dart

sotomski commented 2 months ago

@gkc Thank you for the workaround!

Depending on how many users have this problem (currently it's just me, I assume), it may be beneficial to be more explicit, ie. provide a global function like void overrideHiveTypeAdapters(TypeAdapterConfig newConfig), where newConfig would be an abstract class TypeAdapterConfig with getters for each adapter. By providing own implemention of the config, the user of the library could inject a custom configuration without explicitly modifying the internal map/state of the library.

Also, the config class gives you, the author, more control over how much of the internal knowledge you'd be giving away. You could:

By 'leaking' I don't mean 'uncover secrets' but rather 'expose the user to unnecessary cognitive load'. I may be overthinking this at this point, though 🤔

Since I lack the broader context, it's hard for me to decide if it's worth the effort/increase in complexity. I'd be fine with whatever solution allows me to move forward and trust you as a maintainer to make the best choice 👍