-
Notifications
You must be signed in to change notification settings - Fork 827
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
Miscellaneous refactors #2608
Miscellaneous refactors #2608
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,12 +16,14 @@ | |
import com.fasterxml.jackson.annotation.JsonIgnore; | ||
import com.fasterxml.jackson.annotation.JsonIgnoreProperties; | ||
import com.fasterxml.jackson.annotation.JsonInclude; | ||
import lombok.Getter; | ||
|
||
import java.lang.reflect.ParameterizedType; | ||
import java.net.URL; | ||
import java.util.List; | ||
import java.util.Objects; | ||
|
||
@Getter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not really a fan of converting existing class to use Lombok. To me, it is most useful for new code. Regardless, if you wanted to change it to use Lombok, why stop at Getter? Why not also use @Setter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the Setters, our existing implementations of the setters in this class is not "standard", e.g.:
So I can't use the lombok setter without more code changes. It can be done though, just leaving it aside for now. As of the motivation of this refactor - it's mainly sort of a response to this prior discussion with Bruce. With this PR, I'm making the point that the "isSomeBoolean" getter pattern is now a common pattern, since it's what lombok would generate, though it might not be necessarily grammatically correct. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. |
||
@JsonIgnoreProperties(ignoreUnknown = true) | ||
public abstract class AbstractExternalOAuthIdentityProviderDefinition<T extends AbstractExternalOAuthIdentityProviderDefinition> extends ExternalIdentityProviderDefinition { | ||
public enum OAuthGroupMappingMode { | ||
|
@@ -39,165 +41,97 @@ public enum OAuthGroupMappingMode { | |
private boolean clientAuthInBody = false; | ||
private boolean skipSslValidation; | ||
private String relyingPartyId; | ||
@JsonInclude(JsonInclude.Include.NON_NULL) | ||
private String relyingPartySecret; | ||
private List<String> scopes; | ||
private String issuer; | ||
private String responseType = "code"; | ||
private String userPropagationParameter; | ||
@JsonInclude(JsonInclude.Include.NON_NULL) | ||
private OAuthGroupMappingMode groupMappingMode; | ||
private boolean pkce = true; | ||
private boolean performRpInitiatedLogout = true; | ||
|
||
public URL getAuthUrl() { | ||
return authUrl; | ||
} | ||
|
||
public T setAuthUrl(URL authUrl) { | ||
this.authUrl = authUrl; | ||
return (T) this; | ||
} | ||
|
||
public URL getTokenUrl() { | ||
return tokenUrl; | ||
} | ||
|
||
public T setTokenUrl(URL tokenUrl) { | ||
this.tokenUrl = tokenUrl; | ||
return (T) this; | ||
} | ||
|
||
public URL getTokenKeyUrl() { | ||
return tokenKeyUrl; | ||
} | ||
|
||
public T setTokenKeyUrl(URL tokenKeyUrl) { | ||
this.tokenKeyUrl = tokenKeyUrl; | ||
return (T) this; | ||
} | ||
|
||
public String getTokenKey() { | ||
return tokenKey; | ||
} | ||
|
||
public T setTokenKey(String tokenKey) { | ||
this.tokenKey = tokenKey; | ||
return (T) this; | ||
} | ||
|
||
public URL getUserInfoUrl() { | ||
return userInfoUrl; | ||
} | ||
|
||
public T setUserInfoUrl(URL userInfoUrl) { | ||
this.userInfoUrl = userInfoUrl; | ||
return (T) this; | ||
} | ||
|
||
public URL getLogoutUrl() { | ||
return logoutUrl; | ||
} | ||
|
||
public T setLogoutUrl(URL logoutUrl) { | ||
this.logoutUrl = logoutUrl; | ||
return (T) this; | ||
} | ||
|
||
public String getLinkText() { | ||
return linkText; | ||
} | ||
|
||
public T setLinkText(String linkText) { | ||
this.linkText = linkText; | ||
return (T) this; | ||
} | ||
|
||
public boolean isClientAuthInBody() { | ||
return clientAuthInBody; | ||
} | ||
|
||
public T setClientAuthInBody(boolean clientAuthInBody) { | ||
this.clientAuthInBody = clientAuthInBody; | ||
return (T) this; | ||
} | ||
|
||
public boolean isShowLinkText() { | ||
return showLinkText; | ||
} | ||
|
||
public T setShowLinkText(boolean showLinkText) { | ||
this.showLinkText = showLinkText; | ||
return (T) this; | ||
} | ||
|
||
public String getRelyingPartyId() { | ||
return relyingPartyId; | ||
} | ||
|
||
public T setRelyingPartyId(String relyingPartyId) { | ||
this.relyingPartyId = relyingPartyId; | ||
return (T) this; | ||
} | ||
|
||
@JsonInclude(JsonInclude.Include.NON_NULL) | ||
public String getRelyingPartySecret() { | ||
return relyingPartySecret; | ||
} | ||
|
||
public T setRelyingPartySecret(String relyingPartySecret) { | ||
this.relyingPartySecret = relyingPartySecret; | ||
return (T) this; | ||
} | ||
|
||
public boolean isSkipSslValidation() { | ||
return skipSslValidation; | ||
} | ||
|
||
public T setSkipSslValidation(boolean skipSslValidation) { | ||
this.skipSslValidation = skipSslValidation; | ||
return (T) this; | ||
} | ||
|
||
public List<String> getScopes() { | ||
return scopes; | ||
} | ||
|
||
public T setScopes(List<String> scopes) { | ||
this.scopes = scopes; | ||
return (T) this; | ||
} | ||
|
||
public String getIssuer() { | ||
return issuer; | ||
} | ||
|
||
public T setIssuer(String issuer) { | ||
this.issuer = issuer; | ||
return (T) this; | ||
} | ||
|
||
public String getResponseType() { | ||
return responseType; | ||
} | ||
|
||
public T setResponseType(String responseType) { | ||
this.responseType = responseType; | ||
return (T) this; | ||
} | ||
|
||
public String getUserPropagationParameter() { | ||
return userPropagationParameter; | ||
} | ||
|
||
public T setUserPropagationParameter(String userPropagationParameter) { | ||
this.userPropagationParameter = userPropagationParameter; | ||
return (T) this; | ||
} | ||
|
||
@JsonInclude(JsonInclude.Include.NON_NULL) // prevent json data for default | ||
public OAuthGroupMappingMode getGroupMappingMode() { | ||
return groupMappingMode; | ||
} | ||
|
||
public T setGroupMappingMode(OAuthGroupMappingMode externalGroupMappingMode) { | ||
if (externalGroupMappingMode == OAuthGroupMappingMode.EXPLICITLY_MAPPED) { | ||
this.groupMappingMode = null; | ||
|
@@ -211,14 +145,6 @@ public void setPkce(final boolean pkce) { | |
this.pkce = pkce; | ||
} | ||
|
||
public boolean isPkce() { | ||
return this.pkce; | ||
} | ||
|
||
public boolean isPerformRpInitiatedLogout() { | ||
return performRpInitiatedLogout; | ||
} | ||
|
||
public void setPerformRpInitiatedLogout(boolean performRpInitiatedLogout) { | ||
this.performRpInitiatedLogout = performRpInitiatedLogout; | ||
} | ||
|
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.
Nice! I'm a big fan of Lombok.