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

Jump to bottom

refactor: session creator is no longer singleton #154

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor: session creator is no longer singleton #154

wants to merge 1 commit into from

Conversation

jbutler
Copy link
Contributor

@jbutler jbutler commented Oct 6, 2022

Issue #, if available:

Description of changes:
Minor refactor. This moves IPC registration until the end of the install phase, and also makes the SessionManage injectable.

Why is this change necessary:

How was this change tested:

Any additional information or context required to review the change:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions
Copy link

github-actions bot commented Oct 6, 2022 •
edited
Loading

Unit Tests Coverage Report

File Coverage Lines Branches
All files 69% 77% 61% ✅
com.aws.greengrass.clientdevices.auth.infra.NetworkState 36% 36% 0% ❌
com.aws.greengrass.clientdevices.auth.infra.NetworkState$ConnectionState 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.infra.NetworkState$1 20% 20% 0% ❌
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils 78% 82% 74% ✅
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils$Operation 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.PermissionEvaluationUtils$Resource 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.CertificateManager 78% 92% 63% ✅
com.aws.greengrass.clientdevices.auth.ClientDevicesAuthService 79% 90% 67% ✅
com.aws.greengrass.clientdevices.auth.DeviceAuthClient 72% 80% 64% ✅
com.aws.greengrass.clientdevices.auth.certificate.ClientCertificateGenerator 96% 93% 100% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateHelper$ProviderType 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateHelper 73% 92% 54% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateStore 65% 77% 52% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor 77% 88% 67% ✅
com.aws.greengrass.clientdevices.auth.certificate.ServerCertificateGenerator 95% 90% 100% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateGenerator 70% 90% 50% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateStore$CAType 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificateExpiryMonitor$CertRotationDecider 90% 100% 80% ✅
com.aws.greengrass.clientdevices.auth.certificate.CertificatesConfig 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyIotCertificate 73% 96% 50% ✅
com.aws.greengrass.clientdevices.auth.iot.usecases.VerifyThingAttachedToCertificate 76% 85% 67% ✅
com.aws.greengrass.clientdevices.auth.iot.usecases.CreateIoTThingSession 87% 90% 83% ✅
com.aws.greengrass.ipc.IPCUtils 83% 67% 100% ✅
com.aws.greengrass.ipc.VerifyClientDeviceIdentityOperationHandler 61% 71% 50% ✅
com.aws.greengrass.ipc.GetClientDeviceAuthTokenOperationHandler 87% 98% 75% ✅
com.aws.greengrass.ipc.AuthorizeClientDeviceActionOperationHandler 80% 93% 67% ✅
com.aws.greengrass.ipc.SubscribeToCertificateUpdatesOperationHandler 77% 87% 67% ✅
com.aws.greengrass.clientdevices.auth.session.SessionConfig 92% 100% 83% ✅
com.aws.greengrass.clientdevices.auth.session.SessionManager$1 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.session.MqttSessionFactory 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.session.SessionCreator 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.session.SessionManager 88% 100% 75% ✅
com.aws.greengrass.clientdevices.auth.session.SessionImpl 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.session.MqttSessionFactory$MqttCredential 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CACertificateChainChangedHandler 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CAConfigurationChangedHandler 93% 87% 100% ✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.CertificateRotationHandler 96% 91% 100% ✅
com.aws.greengrass.clientdevices.auth.certificate.handlers.SecurityConfigurationChangedHandler 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.certificate.infra.BackgroundCertificateRefresh$ThingAssociations 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.certificate.infra.ClientCertificateStore 75% 74% 75% ✅
com.aws.greengrass.clientdevices.auth.certificate.infra.BackgroundCertificateRefresh 70% 78% 63% ✅
com.aws.greengrass.clientdevices.auth.iot.infra.ThingRegistry 92% 97% 88% ✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureManagedCertificateAuthority 85% 85% 0% ✅
com.aws.greengrass.clientdevices.auth.certificate.usecases.ConfigureCustomCertificateAuthority 8% 8% 0% ❌
com.aws.greengrass.clientdevices.auth.certificate.usecases.RegisterCertificateAuthorityUseCase 66% 81% 50% ✅
com.aws.greengrass.clientdevices.auth.configuration.AuthorizationPolicyStatement$Effect 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.configuration.GroupManager 89% 94% 83% ✅
com.aws.greengrass.clientdevices.auth.configuration.ConfigurationFormatVersion 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.configuration.CAConfiguration 96% 100% 92% ✅
com.aws.greengrass.clientdevices.auth.configuration.RuntimeConfiguration 80% 96% 63% ✅
com.aws.greengrass.clientdevices.auth.configuration.SecurityConfiguration 80% 94% 67% ✅
com.aws.greengrass.clientdevices.auth.configuration.CDAConfiguration 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.configuration.GroupDefinition 75% 100% 50% ✅
com.aws.greengrass.clientdevices.auth.configuration.ExpressionVisitor 84% 94% 75% ✅
com.aws.greengrass.clientdevices.auth.configuration.GroupConfiguration 90% 95% 86% ✅
com.aws.greengrass.clientdevices.auth.api.ClientDevicesAuthServiceApi 54% 59% 50% ✅
com.aws.greengrass.clientdevices.auth.api.DomainEvents 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.api.UseCases 71% 92% 50% ✅
com.aws.greengrass.clientdevices.auth.api.DomainEvent 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.api.GetCertificateRequestOptions$CertificateType 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.iot.dto.CertificateV1DTO$Status 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.connectivity.usecases.GetConnectivityInformationUseCase 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.connectivity.usecases.RecordConnectivityChangesUseCase 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.session.attribute.StringLiteralAttribute 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.session.attribute.WildcardSuffixAttribute 88% 100% 75% ✅
com.aws.greengrass.clientdevices.auth.util.ResizableLinkedBlockingQueue 90% 80% 100% ✅
com.aws.greengrass.clientdevices.auth.util.ParseIPAddress 90% 95% 84% ✅
com.aws.greengrass.clientdevices.auth.certificate.events.CACertificateChainChanged 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.iot.Certificate$Status 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.iot.IotAuthClient$Default 57% 47% 67% ✅
com.aws.greengrass.clientdevices.auth.iot.Thing 85% 91% 79% ✅
com.aws.greengrass.clientdevices.auth.iot.Certificate 78% 89% 67% ✅
com.aws.greengrass.clientdevices.auth.iot.CertificateRegistry 93% 87% 100% ✅
com.aws.greengrass.clientdevices.auth.iot.Component 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.connectivity.CISShadowMonitor 61% 77% 46% ✅
com.aws.greengrass.clientdevices.auth.connectivity.RecordConnectivityChangesResponse 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.connectivity.HostAddress 67% 67% 0% ✅
com.aws.greengrass.clientdevices.auth.connectivity.RecordConnectivityChangesRequest 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.connectivity.ConnectivityInformation 100% 100% 100% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionConstants 100% 100% 0% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.TokenMgrError 22% 32% 12% ❌
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTokenManager 61% 65% 58% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTStart 33% 33% 0% ❌
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTAnd 67% 67% 0% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.Token 58% 58% 0% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionDefaultVisitor 0% 0% 0% ❌
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTOr 67% 67% 0% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleCharStream 28% 31% 25% ❌
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpressionTreeConstants 0% 0% 0% ❌
com.aws.greengrass.clientdevices.auth.configuration.parser.JJTRuleExpressionState 67% 65% 70% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.ASTThing 67% 67% 0% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.RuleExpression 63% 63% 62% ✅
com.aws.greengrass.clientdevices.auth.configuration.parser.SimpleNode 27% 35% 19% ❌

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against 63a3d3e

MikeDombo
MikeDombo reviewed Oct 20, 2022
View reviewed changes
src/main/java/com/aws/greengrass/clientdevices/auth/session/SessionCreator.java
@@ -35,9 +29,9 @@ private static class SessionFactorySingleton {
* @return new session if the client can be authenticated
* @throws AuthenticationException if the client fails to be authenticated
*/
public static Session createSession(String credentialType, Map<String, String> credentialMap)
public Session createSession(String credentialType, Map<String, String> credentialMap)
Copy link
Member

Choose a reason for hiding this comment

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

change impacts Moquette?

MikeDombo
MikeDombo previously approved these changes Oct 20, 2022
View reviewed changes
Copy link
Member

@MikeDombo MikeDombo left a comment

Choose a reason for hiding this comment

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

LGTM.
1 question about impact to Moquette.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MikeDombo MikeDombo MikeDombo left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jbutler @MikeDombo