unregister<T> unregisters all services of type T

[spydon]

In get_it 7.6.8 unregister<T> unregisters all services of type T, even if they have been registered with an id, this was not the case in 7.6.7.

Reproduction:

class Service extends Object {}

class Service2 extends Service {}

final _locator = GetIt.asNewInstance();

/// Returns whether a service is registered with the locator.
bool hasService<T extends Object>({String? id}) {
  return _locator.isRegistered<T>(instanceName: id);
}

/// Registers a service with the locator.
void registerService<T extends Object>(
  T Function() create, {
  String? id,
  FutureOr<void> Function(T service)? dispose,
}) {
  _locator.registerLazySingleton<T>(create, dispose: dispose, instanceName: id);
}

/// Locates and returns an injected service.
T getService<T extends Object>({String? id}) => _locator<T>(instanceName: id);

/// Unregisters a service instance with the locator.
void unregisterService<T extends Object>({
  String? id,
  FutureOr<void> Function(T service)? dispose,
}) {
  if (hasService<T>(id: id)) {
    _locator.unregister<T>(instanceName: id, disposingFunction: dispose);
  }
}

void main() {
  test('service id', () {
    registerService<Service>(Service.new);
    registerService<Service>(Service2.new, id: '2');

    expect(getService<Service>(), isA<Service>());
    expect(getService<Service>(id: '2'), isA<Service2>());

    unregisterService<Service>();

    expect(() => getService<Service>(), throwsStateError);
    expect(getService<Service>(id: '2'), isA<Service2>()); // <--- Fails here
  }
}

The result is:

Bad state: GetIt: Object/factory with with name 2 and type Service is not registered inside GetIt. 
(Did you accidentally do GetIt sl=GetIt.instance(); instead of GetIt sl=GetIt.instance;
Did you forget to register it?)

[venkata-reddy-dev]

@spydon can you please provide implementation of hasService<T>(id: id)

i am unable to run code which you have posted. please post compilable code. i will try to look into this.

[spydon]

@venkata-reddy-dev ah sorry, missed that one, added now!

[spydon]

It's probably this PR that caused the regression: #354
Might have been intentional, but weird to only do a patch bump for a breaking change.

[escamoteur]

Hi Lukas, Sorry for the inconvenience. This was for sure not intended. Will look into it as soon as possible Cheers Thomas Am 10. Apr. 2024, 17:04 +0200 schrieb Lukas Klingsbo ***@***.***>:

…

Fairly certain that it's this PR that caused the regression: #354 Might have been intentional, but weird to only do a patch bump for a breaking change. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[spydon]

No problem @escamoteur
It's actually not a feature that we're using, but the tests started breaking in ubuntu_service so I thought I'd report it.

[escamoteur]

could you both have a look at

  FutureOr unregister<T extends Object>({
    Object? instance,
    String? instanceName,
    FutureOr Function(T)? disposingFunction,
  }) async {
    final factoryToRemove = instance != null
        ? _findFactoryByInstance(instance)
        : _findFactoryByNameAndType<T>(instanceName);

    throwIf(
      factoryToRemove.objectsWaiting.isNotEmpty,
      StateError(
        'There are still other objects waiting for this instance so signal ready',
      ),
    );

    final typeRegistration = factoryToRemove.registeredIn;

    if (instanceName != null) {
      typeRegistration.namedFactories.remove(instanceName);
    } else {
      typeRegistration.factories.remove(factoryToRemove);
    }
    if (typeRegistration.isEmpty) {
      factoryToRemove.registrationScope.typeRegistrations.remove(T);
    }

I think this should be the correct implementation

[escamoteur]

@spydon just wondering, why are you using constructor tear offs there? or better why wrapping the get_it function at all?

[spydon]

@spydon just wondering, why are you using constructor tear offs there? or better why wrapping the get_it function at all?

There are no constructor tear-offs in the code I posted as far as I can see? I don't know what the reason for wrapping get_it was, maybe due to some testing requirements or such, the ubuntu_service package was developed a long time before my contract with canonical started.

Edit: Aah, the Service.new, no idea...
Edit2: Since the registerService method is using the registerLazySingleton under the hood, the constructor tear-offs make sense I think

[escamoteur]

should be fixed in 7.6.9