Skip to content

Remove Lombok from several Parameters classes #919

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

Merged
merged 2 commits into from
Mar 17, 2025
Merged

Remove Lombok from several Parameters classes #919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7833386
Remove lombok from several classes
Avery-Dunn Mar 7, 2025
e4d34a8
Merge branch 'dev' of https://github.com/AzureAD/microsoft-authentica…
Avery-Dunn Mar 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AppTokenProviderParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,8 @@

package com.microsoft.aad.msal4j;

import lombok.AllArgsConstructor;
Copy link
Member

Choose a reason for hiding this comment

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

What is the argument for removing lombok? I thought Lombok generates this code at compile time?

Copy link

Choose a reason for hiding this comment

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

Couple things motivated the ask: 1) Lombok needs plugins in IDEs to make the experience reasonable and 2) the debugging experience can be pretty frustrating.

@JonathanGiles any additional thoughts?

Copy link

Choose a reason for hiding this comment

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

Lombok is also not an official Java annotation processor, in the sense that is does not uses public APIs exposed by the Java compiler. This is risky and may be broken at any time. It feels bad to build such critical library code on such a weak foundation, especially when the code being generated by Lombok can easily be written by us (or our IDE) once, without these concerns in perpetuity.

import lombok.Getter;
import lombok.Setter;

import java.util.Set;

@Getter
@Setter
@AllArgsConstructor
/// The authentication parameters provided to the app token provider callback.
public class AppTokenProviderParameters {

Expand All @@ -23,4 +16,43 @@ public class AppTokenProviderParameters {
public String claims;
/// tenant id
public String tenantId;

public AppTokenProviderParameters(Set<String> scopes, String correlationId, String claims, String tenantId) {
Copy link
Member

Choose a reason for hiding this comment

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

@Avery-Dunn - is there value in reviewing these changes ? I.e. did you de-lombok all of this, in which case it's generaetd code and we can take it at face value?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For this first PR, yes:

  • Helps to understand what Lombok was doing, and what effects removing it will have
  • Demonstrates what the Lombok plugin will produce and confirms it's correct
  • Shows what still needs to be manually adjusted, mainly the comments used in API docs

Going forward I don't think so. The next PR will affect a lot more classes, but as long as there are no API changes (as confirmed via RevAPI) and tests pass then it shouldn't require any sort of deep review.

this.scopes = scopes;
this.correlationId = correlationId;
this.claims = claims;
this.tenantId = tenantId;
}

public Set<String> getScopes() {
return this.scopes;
}

public String getCorrelationId() {
return this.correlationId;
}

public String getClaims() {
return this.claims;
}

public String getTenantId() {
return this.tenantId;
}

public void setScopes(Set<String> scopes) {
this.scopes = scopes;
}

public void setCorrelationId(String correlationId) {
this.correlationId = correlationId;
}

public void setClaims(String claims) {
this.claims = claims;
}

public void setTenantId(String tenantId) {
this.tenantId = tenantId;
}
}
178 changes: 143 additions & 35 deletions msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthorizationCodeParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,70 +3,50 @@

package com.microsoft.aad.msal4j;

import lombok.*;
import lombok.experimental.Accessors;

import java.net.URI;
import java.util.Map;
import java.util.Set;

import static com.microsoft.aad.msal4j.ParameterValidationUtils.validateNotBlank;
import static com.microsoft.aad.msal4j.ParameterValidationUtils.validateNotNull;

/**
* Object containing parameters for authorization code flow. Can be used as parameter to
* {@link PublicClientApplication#acquireToken(AuthorizationCodeParameters)} or to
* {@link ConfidentialClientApplication#acquireToken(AuthorizationCodeParameters)}
*/
@Builder
@Accessors(fluent = true)
@Getter
@AllArgsConstructor(access = AccessLevel.PRIVATE)
public class AuthorizationCodeParameters implements IAcquireTokenParameters {

/**
* Authorization code acquired in the first step of OAuth2.0 authorization code flow. For more
* details, see https://aka.ms/msal4j-authorization-code-flow
*/
@NonNull
private String authorizationCode;

/**
* Redirect URI registered in the Azure portal, and which was used in the first step of OAuth2.0
* authorization code flow. For more details, see https://aka.ms/msal4j-authorization-code-flow
*/
@NonNull
private URI redirectUri;

/**
* Scopes to which the application is requesting access
*/
private Set<String> scopes;

/**
* Claims to be requested through the OIDC claims request parameter, allowing requests for standard and custom claims
*/
private ClaimsRequest claims;

/**
* Code verifier used for PKCE. For more details, see https://tools.ietf.org/html/rfc7636
*/
private String codeVerifier;

/**
* Adds additional headers to the token request
*/
private Map<String, String> extraHttpHeaders;

/**
* Adds additional query parameters to the token request
*/
private Map<String, String> extraQueryParameters;

/**
* Overrides the tenant value in the authority URL for this request
*/
private String tenant;

private AuthorizationCodeParameters(String authorizationCode, URI redirectUri,
Set<String> scopes, ClaimsRequest claims,
String codeVerifier, Map<String, String> extraHttpHeaders,
Map<String, String> extraQueryParameters, String tenant) {
this.authorizationCode = authorizationCode;
this.redirectUri = redirectUri;
this.scopes = scopes;
this.claims = claims;
this.codeVerifier = codeVerifier;
this.extraHttpHeaders = extraHttpHeaders;
this.extraQueryParameters = extraQueryParameters;
this.tenant = tenant;
}

private static AuthorizationCodeParametersBuilder builder() {

return new AuthorizationCodeParametersBuilder();
Expand All @@ -87,4 +67,132 @@ public static AuthorizationCodeParametersBuilder builder(String authorizationCod
.authorizationCode(authorizationCode)
.redirectUri(redirectUri);
}

public String authorizationCode() {
return this.authorizationCode;
}

public URI redirectUri() {
return this.redirectUri;
}

public Set<String> scopes() {
return this.scopes;
}

public ClaimsRequest claims() {
return this.claims;
}

public String codeVerifier() {
return this.codeVerifier;
}

public Map<String, String> extraHttpHeaders() {
return this.extraHttpHeaders;
}

public Map<String, String> extraQueryParameters() {
return this.extraQueryParameters;
}

public String tenant() {
return this.tenant;
}

public static class AuthorizationCodeParametersBuilder {
private String authorizationCode;
private URI redirectUri;
private Set<String> scopes;
private ClaimsRequest claims;
private String codeVerifier;
private Map<String, String> extraHttpHeaders;
private Map<String, String> extraQueryParameters;
private String tenant;

AuthorizationCodeParametersBuilder() {
}

/**
* Authorization code acquired in the first step of OAuth2.0 authorization code flow. For more
* details, see https://aka.ms/msal4j-authorization-code-flow
* <p>
* Cannot be null.
*/
public AuthorizationCodeParametersBuilder authorizationCode(String authorizationCode) {
validateNotNull("authorizationCode", authorizationCode);

this.authorizationCode = authorizationCode;
return this;
}

/**
* Redirect URI registered in the Azure portal, and which was used in the first step of OAuth2.0
* authorization code flow. For more details, see https://aka.ms/msal4j-authorization-code-flow
* <p>
* Cannot be null.
*/
public AuthorizationCodeParametersBuilder redirectUri(URI redirectUri) {
validateNotNull("redirectUri", redirectUri);

this.redirectUri = redirectUri;
return this;
}

/**
* Scopes to which the application is requesting access
*/
public AuthorizationCodeParametersBuilder scopes(Set<String> scopes) {
this.scopes = scopes;
return this;
}

/**
* Claims to be requested through the OIDC claims request parameter, allowing requests for standard and custom claims
*/
public AuthorizationCodeParametersBuilder claims(ClaimsRequest claims) {
this.claims = claims;
return this;
}

/**
* Code verifier used for PKCE. For more details, see https://tools.ietf.org/html/rfc7636
*/
public AuthorizationCodeParametersBuilder codeVerifier(String codeVerifier) {
this.codeVerifier = codeVerifier;
return this;
}

/**
* Adds additional headers to the token request
*/
public AuthorizationCodeParametersBuilder extraHttpHeaders(Map<String, String> extraHttpHeaders) {
this.extraHttpHeaders = extraHttpHeaders;
return this;
}

/**
* Adds additional query parameters to the token request
*/
public AuthorizationCodeParametersBuilder extraQueryParameters(Map<String, String> extraQueryParameters) {
this.extraQueryParameters = extraQueryParameters;
return this;
}

/**
* Overrides the tenant value in the authority URL for this request
*/
public AuthorizationCodeParametersBuilder tenant(String tenant) {
this.tenant = tenant;
return this;
}

public AuthorizationCodeParameters build() {
return new AuthorizationCodeParameters(this.authorizationCode, this.redirectUri, this.scopes, this.claims, this.codeVerifier, this.extraHttpHeaders, this.extraQueryParameters, this.tenant);
}

public String toString() {
return "AuthorizationCodeParameters.AuthorizationCodeParametersBuilder(authorizationCode=" + this.authorizationCode + ", redirectUri=" + this.redirectUri + ", scopes=" + this.scopes + ", claims=" + this.claims + ", codeVerifier=" + this.codeVerifier + ", extraHttpHeaders=" + this.extraHttpHeaders + ", extraQueryParameters=" + this.extraQueryParameters + ", tenant=" + this.tenant + ")";
}
}
}
Loading