Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise and improve cached factory handling with params #387

Merged
merged 5 commits into from
Oct 28, 2024
Merged

Revise and improve cached factory handling with params #387

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
935cc8b
Add cached factory to excluded types on assert and save lastParams
ArtAhmetaj Oct 25, 2024
9cc5cba
add tests for the cached factories
ArtAhmetaj Oct 25, 2024
37ac31d
Add same fixes on async equivalent function
ArtAhmetaj Oct 25, 2024
06525e8
remove p1 and p2 on register scenario
ArtAhmetaj Oct 28, 2024
d118a07
Remove dispose method on testclass param
ArtAhmetaj Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions lib/get_it_impl.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,8 @@ class _ServiceFactory<T extends Object, P1, P2> {
/// returns an instance depending on the type of the registration if [async==false]
T getObject(dynamic param1, dynamic param2) {
assert(
!(factoryType != _ServiceFactoryType.alwaysNew &&
!(![_ServiceFactoryType.alwaysNew, _ServiceFactoryType.cachedFactory]
.contains(factoryType) &&
(param1 != null || param2 != null)),
'You can only pass parameters to factories!',
);
Expand Down Expand Up @@ -201,6 +202,8 @@ class _ServiceFactory<T extends Object, P1, P2> {
param2 == lastParam2) {
return weakReferenceInstance!.target!;
} else {
lastParam1 = param1 as P1?;
lastParam2 = param2 as P2?;
T newInstance;
if (creationFunctionParam != null) {
newInstance = creationFunctionParam!(param1 as P1, param2 as P2);
Expand Down Expand Up @@ -254,7 +257,8 @@ class _ServiceFactory<T extends Object, P1, P2> {
/// if [dependsOn.isNotEmpty].
Future<R> getObjectAsync<R>(dynamic param1, dynamic param2) async {
assert(
!(factoryType != _ServiceFactoryType.alwaysNew &&
!(![_ServiceFactoryType.alwaysNew, _ServiceFactoryType.cachedFactory]
.contains(factoryType) &&
(param1 != null || param2 != null)),
'You can only pass parameters to factories!',
);
Expand Down Expand Up @@ -291,6 +295,8 @@ class _ServiceFactory<T extends Object, P1, P2> {
return Future<R>.value(weakReferenceInstance!.target! as R);
} else {
if (creationFunctionParam != null) {
lastParam1 = param1 as P1?;
lastParam2 = param2 as P2?;
return asyncCreationFunctionParam!(param1 as P1, param2 as P2)
.then((value) {
weakReferenceInstance = WeakReference(value);
Expand Down Expand Up @@ -1546,6 +1552,8 @@ class _GetItImplementation implements GetIt {
FactoryFuncAsync<T>? factoryFuncAsync,
FactoryFuncParamAsync<T, P1, P2>? factoryFuncParamAsync,
T? instance,
P1? param1,
escamoteur marked this conversation as resolved.
Show resolved Hide resolved
P2? param2,
required String? instanceName,
required bool isAsync,
Iterable<Type>? dependsOn,
Expand Down
84 changes: 82 additions & 2 deletions test/get_it_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,17 @@ class TestClassDisposable extends TestBaseClass with Disposable {

class TestClass2 {}

class TestClassParam {
class TestClassParam extends TestBaseClass {
final String? param1;
final int? param2;

TestClassParam({this.param1, this.param2});
TestClassParam({this.param1, this.param2}) {
constructorCounter++;
}

void dispose() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't use the dispose in your test so the linter complains and the CI can't run.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the method given that its not used on the test class atm, added it for consistency since it was on other classes but I dont use dispose counter to assert anything atm, if you want I can add it with your guidance.

disposeCounter++;
}
}

class TestClassDisposableWithDependency with Disposable {
Expand Down Expand Up @@ -216,6 +222,80 @@ void main() {
GetIt.I.reset();
});

test('register cached factory', () {
final getIt = GetIt.instance;
constructorCounter = 0;
getIt.registerCachedFactory<TestBaseClass>(() => TestClass());
final TestBaseClass instance1 = getIt<TestBaseClass>();
expect(instance1 is TestClass, true);
final instance2 = getIt.get<TestBaseClass>();
expect(getIt.isRegistered<TestBaseClass>(), true);
expect(instance1, instance2);
expect(constructorCounter, 1);
});

test('register cached factory with one param ', () {
final getIt = GetIt.instance;
constructorCounter = 0;
getIt.registerCachedFactoryParam<TestClassParam, String, void>(
(s, _) => TestClassParam(param1: s),
);
final instance1 = getIt<TestClassParam>(param1: 'abc');
final instance2 = getIt<TestClassParam>(param1: 'abc');
expect(instance1 is TestClassParam, true);
expect(instance1.param1, 'abc');
expect(instance1, instance2);
expect(constructorCounter, 1);
});

test('register cached factory with different params', () {
final getIt = GetIt.instance;
constructorCounter = 0;
getIt.registerCachedFactoryParam<TestClassParam, String, void>(
(s, _) => TestClassParam(param1: s),
);
final instance1 = getIt<TestClassParam>(param1: 'abc');
final instance2 = getIt<TestClassParam>(param1: '123');
expect(instance1 is TestClassParam, true);
expect(instance1.param1, 'abc');
expect(instance2.param1, '123');
expect(instance2 is TestClassParam, true);
expect(instance1, isNot(instance2));
expect(constructorCounter, 2);
});

test('register cached factory with two equal params', () {
final getIt = GetIt.instance;
constructorCounter = 0;
getIt.registerCachedFactoryParam<TestClassParam, String, int>(
(f,s) => TestClassParam(param1: f,param2:s),
);
final instance1 = getIt<TestClassParam>(param1: 'abc', param2: 1);
final instance2 = getIt<TestClassParam>(param1: 'abc', param2: 1);
expect(instance1 is TestClassParam, true);
expect(instance1.param1, 'abc');
expect(instance1, instance2);
expect(constructorCounter, 1);
});

test('register cached factory with one different param out of two', () {
final getIt = GetIt.instance;
constructorCounter = 0;
getIt.registerCachedFactoryParam<TestClassParam, String, int>(
(f,s) => TestClassParam(param1: f,param2: s),
);
final instance1 = getIt<TestClassParam>(param1: 'abc', param2: 1);
final instance2 = getIt<TestClassParam>(param1: 'abc', param2: 2);
expect(instance1 is TestClassParam, true);
expect(instance1.param1, 'abc');
expect(instance1.param2, 1);
expect(instance2.param1, 'abc');
expect(instance2.param2, 2);
expect(instance2 is TestClassParam, true);
expect(instance1, isNot(instance2));
expect(constructorCounter, 2);
});

test('register constant', () {
final getIt = GetIt.instance;
constructorCounter = 0;
Expand Down