Provide Project via injection #672
Conversation
| bind(CurrencyUnit.class) | ||
| .toProvider(CurrencyFromCountryProvider.class) | ||
| .in(RequestScoped.class); | ||
|
|
There was a problem hiding this comment.
Removed from Module.java, because this file belongs to Starter Project and is therefore never updated with new releases, what makes migration more difficult. Now moved to LocalizationModule, which is enabled by default on reference.conf (default configuration), and can be disabled via configuration as explained in #671 in order to bind your own implementations.
| @@ -64,4 +69,8 @@ default boolean isCountrySupported(final CountryCode countryCode) { | |||
| default boolean isCurrencySupported(final CurrencyUnit currency) { | |||
| return currencies().contains(currency); | |||
| } | |||
|
|
|||
| static ProjectContext of(final Configuration globalConfig, final String configPath, final Project project) { | |||
There was a problem hiding this comment.
To me it looks like this method isn't used or do you plan to use it in tests?
|
|
||
| import javax.inject.Singleton; | ||
|
|
||
| public final class ProjectModule extends AbstractModule { |
There was a problem hiding this comment.
The name clearly states that it provides a project, but as an user you have to look at the source code to see what this module provides. What do you think about adding some javadoc here?
|
|
||
| import static io.sphere.sdk.client.SphereClientUtils.blockingWait; | ||
|
|
||
| public final class ProjectProvider implements Provider<Project> { |
There was a problem hiding this comment.
We now could even remove the public modifier from this class. WDYT?
There was a problem hiding this comment.
No, I would like to let developers to bind using this provider, in case they need to override some parts of an injection module and reuse others.
| @@ -13,7 +14,7 @@ | |||
| private final CountryInSession countryInSession; | |||
|
|
|||
| @Inject | |||
| public CountryFromSessionProvider(final ProjectContext projectContext, final CountryInSession countryInSession) { | |||
| CountryFromSessionProvider(final ProjectContext projectContext, final CountryInSession countryInSession) { | |||
There was a problem hiding this comment.
Nice that you removed the public modifier here. And we now could remove the public modifier from this class too.
| import javax.money.CurrencyUnit; | ||
| import java.util.Locale; | ||
|
|
||
| public final class LocalizationModule extends AbstractModule { |
There was a problem hiding this comment.
As above: Would be nice to add some javadoc here.
|
|
||
| import javax.inject.Singleton; | ||
|
|
||
| public final class TemplateModule extends AbstractModule { |
There was a problem hiding this comment.
As above: Would be nice to add some javadoc here.
|
@katmatt applied the changes you requested and some more documentation :) |
|
Looks very good to me 👍 |
So as the name explains, the main objective was to extract the Project fetching from the
ProjectContextand grant it its own Provider and Module (which is then enabled inreference.confon Sunrise framework level). And provide the new configuration structure.To make tests work, other injection modules had to be created as well (localization, sphereclient and template).