-
-
Notifications
You must be signed in to change notification settings - Fork 727
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.
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
Fix DeclaredScopedInstance into adapted ScopedInstanceFactory #2111
Conversation
… scope directly, and avoid leak the initial value
|
||
@PublishedApi | ||
internal fun saveValue(id : ScopeID, value : T){ | ||
values[id] = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This accessor is not thread safe
} | ||
} | ||
return values[context.scope.id] ?: error("Scoped instance not found for ${context.scope.id} in $beanDefinition") | ||
return values[context.scope.id] ?: throw MissingScopeValueException("Factory.get -Scoped instance not found for ${context.scope.id} in $beanDefinition") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a breaking change. Make sure you increase major version before release
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not in that case, as with such new behavior we would have ISE and can't use getOrNull
. Here external behavior is the same for the user, "just" a side effect internally to handle that case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then this api, including exception, should be internal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, it is still an internal API. I can tag them @KoinInternalApi in 4.1 branch.
Fix DeclaredScopedInstance solution to provide instance declared in rework previous with ScopedInstanceFactory