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.

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

[WFCORE-2080] Instantiating the WildFlyElytronProvider is an expensive operation so only do it once. #2003

Merged
merged 2 commits into from
Dec 7, 2016
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@
*/
public class SecurityRealmService implements Service<SecurityRealm>, SecurityRealm {

private static final Provider ELYTRON_PROVIDER = new WildFlyElytronProvider();
Copy link
Contributor

Choose a reason for hiding this comment

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

This could use a comment as to why it's ok to store a mutable class in a static field. (I looked at how it's used and it's only read after construction so it's ok, but 4 years from now someone else will ask the same question.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thinking about this I can probably get away with moving this into the service lifecycle, the reported problem was it was being created per request.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as it is not created in both providers directly it is fine.
I was testing locally by just having
Provider[] providers = new Provider[]{new WildFlyElytronProvider()}
in service code and than returning that in both providers.


public static final String LOADED_USERNAME_KEY = SecurityRealmService.class.getName() + ".LOADED_USERNAME";
public static final String SKIP_GROUP_LOADING_KEY = SecurityRealmService.class.getName() + ".SKIP_GROUP_LOADING";

Expand Down Expand Up @@ -181,7 +183,7 @@ public void start(StartContext context) throws StartException {
HttpAuthenticationFactory.Builder httpBuilder = HttpAuthenticationFactory.builder();
httpBuilder.setSecurityDomain(securityDomain);

HttpServerAuthenticationMechanismFactory httpServerFactory = new SecurityProviderServerMechanismFactory(() -> new Provider[] {new WildFlyElytronProvider()});
HttpServerAuthenticationMechanismFactory httpServerFactory = new SecurityProviderServerMechanismFactory(() -> new Provider[] {ELYTRON_PROVIDER});
httpServerFactory = new SetMechanismInformationMechanismFactory(httpServerFactory);
httpServerFactory = new FilterServerMechanismFactory(httpServerFactory, (s) -> {
AuthMechanism mechanism = toAuthMechanism("HTTP", s);
Expand All @@ -201,7 +203,7 @@ public void start(StartContext context) throws StartException {
SaslAuthenticationFactory.Builder saslBuilder = SaslAuthenticationFactory.builder();
saslBuilder.setSecurityDomain(securityDomain);

SaslServerFactory saslServerFactory = new SecurityProviderSaslServerFactory(() -> new Provider[] {new WildFlyElytronProvider()});
SaslServerFactory saslServerFactory = new SecurityProviderSaslServerFactory(() -> new Provider[] {ELYTRON_PROVIDER});
saslServerFactory = new FilterMechanismSaslServerFactory(saslServerFactory, (s) -> {
AuthMechanism mechanism = toAuthMechanism("SASL", s);
return mechanism != null && configurationMap.containsKey(mechanism);
Expand Down