Skip to content

Commit

Permalink
Fix ProviderObserver.didUpdateProvider being called with an incorre…
Browse files Browse the repository at this point in the history 
…ct "provider" parameter when the provider is overridden.
  • Loading branch information
@rrousselGit
rrousselGit committed Nov 15, 2023
1 parent 717e89b commit 3b872c5
Show file tree
Hide file tree
Showing 5 changed files with 195 additions and 124 deletions.
5 changes: 5 additions & 0 deletions packages/riverpod/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## Unreleased patch

- Fix `ProviderObserver.didUpdateProvider` being called with an incorrect
"provider" parameter when the provider is overridden.

## 2.4.6 - 2023-11-13

- Exceptions in asynchronous providers are now correctly received
Expand Down
3 changes: 3 additions & 0 deletions packages/riverpod/lib/src/framework/container.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,9 @@ final b = Provider((ref) => ref.watch(a), dependencies: [a]);
);
}
}

@override
String toString() => 'ProviderContainer#$hashCode';
}

/// An object that listens to the changes of a [ProviderContainer].
Expand Down
6 changes: 3 additions & 3 deletions packages/riverpod/lib/src/framework/element.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ abstract class ProviderElementBase<State> implements Ref<State>, Node {
/// The [ProviderContainer] that owns this [ProviderElementBase].
@override
ProviderContainer get container => _container;
late ProviderContainer _container;
late final ProviderContainer _container;

/// Whether this [ProviderElementBase] is currently listened to or not.
///
Expand Down Expand Up @@ -573,7 +573,7 @@ The provider ${_debugCurrentlyBuildingElement!.origin} modified $origin while bu
for (final observer in _container.observers) {
runQuaternaryGuarded(
observer.didUpdateProvider,
provider,
origin,
previousState,
newState.stateOrNull,
_container,
Expand All @@ -586,7 +586,7 @@ The provider ${_debugCurrentlyBuildingElement!.origin} modified $origin while bu
error: (newState) {
runQuaternaryGuarded(
observer.providerDidFail,
provider,
origin,
newState.error,
newState.stackTrace,
_container,
Expand Down
296 changes: 175 additions & 121 deletions packages/riverpod/test/framework/provider_observer_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,49 +69,48 @@ void main() {
verifyNever(observer.providerDidFail(any, any, any, any));
});

// test(
// 'on scoped ProviderContainer, applies both child and ancestors observers',
// () {
// final provider = StateNotifierProvider<StateController<int>, int>(
// (ref) => StateController(0),
// );
// final observer = ObserverMock();
// final observer2 = ObserverMock();
// final observer3 = ObserverMock();
// final root = createContainer(observers: [observer]);
// final mid = createContainer(
// parent: root,
// observers: [observer2],
// );
// final child = createContainer(
// parent: mid,
// overrides: [provider.overrideWithValue(StateController(42))],
// observers: [observer3],
// );

// expect(child.read(provider), 42);
// expect(mid.read(provider), 0);

// clearInteractions(observer3);
// clearInteractions(observer2);
// clearInteractions(observer);

// child.read(provider.notifier).state++;

// verifyInOrder([
// observer3.didUpdateProvider(provider, 42, 43, child),
// observer2.didUpdateProvider(provider, 42, 43, child),
// observer.didUpdateProvider(provider, 42, 43, child),
// ]);

// mid.read(provider.notifier).state++;

// verify(observer.didUpdateProvider(provider, 0, 1, root)).called(1);

// verifyNoMoreInteractions(observer3);
// verifyNoMoreInteractions(observer2);
// verifyNoMoreInteractions(observer);
// });
test(
'on scoped ProviderContainer, applies both child and ancestors observers',
() {
final provider = StateNotifierProvider<StateController<int>, int>(
(ref) => StateController(0),
);
final observer = ObserverMock('a');
final observer2 = ObserverMock('b');
final observer3 = ObserverMock('c');

final root = createContainer(observers: [observer]);
final mid = createContainer(
parent: root,
observers: [observer2],
);
final child = createContainer(
parent: mid,
overrides: [provider.overrideWith((ref) => StateController(42))],
observers: [observer3],
);

expect(child.read(provider), 42);
expect(mid.read(provider), 0);

clearInteractions(observer3);
clearInteractions(observer2);
clearInteractions(observer);

expect(child.read(provider.notifier).state++, 42);

verify(observer.didUpdateProvider(provider, 42, 43, child)).called(1);
verify(observer2.didUpdateProvider(provider, 42, 43, child)).called(1);
verify(observer3.didUpdateProvider(provider, 42, 43, child)).called(1);

mid.read(provider.notifier).state++;

verify(observer.didUpdateProvider(provider, 0, 1, root)).called(1);

verifyNoMoreInteractions(observer3);
verifyNoMoreInteractions(observer2);
verifyNoMoreInteractions(observer);
});

test('handles computed provider update', () async {
final observer = ObserverMock();
Expand Down Expand Up @@ -150,84 +149,76 @@ void main() {
).called(1);
});

// test('didUpdateProviders', () {
// final observer = ObserverMock();
// final observer2 = ObserverMock();
// final provider = StateNotifierProvider<Counter, int>((_) => Counter());
// final counter = Counter();
// final container = createContainer(
// overrides: [
// provider.overrideWithValue(counter),
// ],
// observers: [observer, observer2],
// );
// final listener = Listener<int>();

// container.listen(provider, listener, fireImmediately: true);

// verify(listener(null, 0)).called(1);
// verifyNoMoreInteractions(listener);
// verifyInOrder([
// observer.didAddProvider(
// provider.notifier,
// counter,
// container,
// ),
// observer2.didAddProvider(provider.notifier, counter, container),
// observer.didAddProvider(provider, 0, container),
// observer2.didAddProvider(provider, 0, container),
// ]);
// verifyNoMoreInteractions(observer);
// verifyNoMoreInteractions(observer2);

// counter.increment();

// verifyInOrder([
// listener(0, 1),
// observer.didUpdateProvider(provider, 0, 1, container),
// observer2.didUpdateProvider(provider, 0, 1, container),
// ]);
// verifyNoMoreInteractions(listener);
// verifyNoMoreInteractions(observer);
// verifyNoMoreInteractions(observer2);
// });

// test('guards didUpdateProviders', () {
// final observer = ObserverMock();
// when(observer.didUpdateProvider(any, any, any, any))
// .thenThrow('error1');
// final observer2 = ObserverMock();
// when(observer2.didUpdateProvider(any, any, any, any))
// .thenThrow('error2');
// final observer3 = ObserverMock();
// final provider = StateNotifierProvider<Counter, int>((_) => Counter());
// final counter = Counter();
// final container = createContainer(
// overrides: [
// provider.overrideWithValue(counter),
// ],
// observers: [observer, observer2, observer3],
// );

// container.read(provider);

// clearInteractions(observer);
// clearInteractions(observer2);
// clearInteractions(observer3);

// final errors = <Object>[];
// runZonedGuarded(counter.increment, (err, stack) => errors.add(err));

// expect(errors, ['error1', 'error2']);
// verifyInOrder([
// observer.didUpdateProvider(provider, 0, 1, container),
// observer2.didUpdateProvider(provider, 0, 1, container),
// observer3.didUpdateProvider(provider, 0, 1, container),
// ]);
// verifyNoMoreInteractions(observer);
// verifyNoMoreInteractions(observer2);
// verifyNoMoreInteractions(observer3);
// });
test('didUpdateProviders', () {
final observer = ObserverMock('a');
final observer2 = ObserverMock('b');
final provider = StateNotifierProvider<Counter, int>((_) => Counter());
final counter = Counter();
final container = createContainer(
overrides: [
provider.overrideWith((ref) => counter),
],
observers: [observer, observer2],
);
final listener = Listener<int>();

container.listen(provider, listener.call, fireImmediately: true);

verify(listener(null, 0)).called(1);
verifyNoMoreInteractions(listener);
verifyInOrder([
observer.didAddProvider(provider, 0, container),
observer2.didAddProvider(provider, 0, container),
]);
verifyNoMoreInteractions(observer);
verifyNoMoreInteractions(observer2);

counter.increment();

verifyInOrder([
listener(0, 1),
observer.didUpdateProvider(provider, 0, 1, container),
observer2.didUpdateProvider(provider, 0, 1, container),
]);
verifyNoMoreInteractions(listener);
verifyNoMoreInteractions(observer);
verifyNoMoreInteractions(observer2);
});

test('guards didUpdateProviders', () {
final observer = ObserverMock();
when(observer.didUpdateProvider(any, any, any, any))
.thenThrow('error1');
final observer2 = ObserverMock();
when(observer2.didUpdateProvider(any, any, any, any))
.thenThrow('error2');
final observer3 = ObserverMock();
final provider = StateNotifierProvider<Counter, int>((_) => Counter());
final counter = Counter();
final container = createContainer(
overrides: [provider.overrideWith((ref) => counter)],
observers: [observer, observer2, observer3],
);

container.read(provider);

clearInteractions(observer);
clearInteractions(observer2);
clearInteractions(observer3);

final errors = <Object>[];
runZonedGuarded(counter.increment, (err, stack) => errors.add(err));

expect(errors, ['error1', 'error2']);
verifyInOrder([
observer.didUpdateProvider(provider, 0, 1, container),
observer2.didUpdateProvider(provider, 0, 1, container),
observer3.didUpdateProvider(provider, 0, 1, container),
]);
verifyNoMoreInteractions(observer);
verifyNoMoreInteractions(observer2);
verifyNoMoreInteractions(observer3);
});

test("Computed don't call didUpdateProviders when value doesn't change",
() async {
Expand Down Expand Up @@ -288,6 +279,69 @@ void main() {
});

group('providerDidFail', () {
test(
'on scoped ProviderContainer, applies both child and ancestors observers',
() async {
final dep = StateProvider((ref) => 0);
final provider = StateNotifierProvider<StateController<int>, int>(
(ref) => StateController(0),
);
final observer = ObserverMock('a');
final observer2 = ObserverMock('b');
final observer3 = ObserverMock('c');

final root = createContainer(observers: [observer]);
final mid = createContainer(
parent: root,
observers: [observer2],
);
final child = createContainer(
parent: mid,
overrides: [
provider.overrideWith((ref) {
if (ref.watch(dep) != 0) {
Error.throwWithStackTrace('error', StackTrace.empty);
}
return StateController(42);
})
],
observers: [observer3],
);

child.listen(provider, (_, __) {}, onError: (_, __) {});

expect(child.read(provider), 42);
expect(mid.read(provider), 0);

clearInteractions(observer3);
clearInteractions(observer2);
clearInteractions(observer);

child.read(dep.notifier).state++;
await child.pump();

verifyInOrder([
observer.didDisposeProvider(provider, child),
observer.didUpdateProvider(dep, 0, 1, root),
observer.didUpdateProvider(provider, 42, null, child),
observer.providerDidFail(provider, 'error', StackTrace.empty, child),
]);
verifyInOrder([
observer2.didDisposeProvider(provider, child),
observer2.didUpdateProvider(provider, 42, null, child),
observer2.providerDidFail(provider, 'error', StackTrace.empty, child),
]);
verifyInOrder([
observer3.didDisposeProvider(provider, child),
observer3.didUpdateProvider(provider, 42, null, child),
observer3.providerDidFail(provider, 'error', StackTrace.empty, child),
]);

verifyNoMoreInteractions(observer3);
verifyNoMoreInteractions(observer2);
verifyNoMoreInteractions(observer);
});

test('is called when FutureProvider emits an error', () async {
final observer = ObserverMock();
final container = createContainer(observers: [observer]);
Expand Down
9 changes: 9 additions & 0 deletions packages/riverpod/test/utils.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,15 @@ class _EqualsIgnoringHashCodes extends Matcher {
}

class ObserverMock extends Mock implements ProviderObserver {
ObserverMock([this.label]);

final String? label;

@override
String toString() {
return label ?? super.toString();
}

@override
void didDisposeProvider(
ProviderBase<Object?>? provider,
Expand Down

0 comments on commit 3b872c5

Please sign in to comment.