Skip to content

SK-2519: extract hard coded strings to constants and update linting r…#272

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
saileshwar-skyflow merged 12 commits into from
Feb 5, 2026
Merged

SK-2519: extract hard coded strings to constants and update linting r… #272

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
767fe7c
SK-2519: extract hard coded strings to constants and update linting r…
skyflow-himanshu Jan 30, 2026
84cb6df
aadarsh-st/SK-2521-Enhance content-type handling and add validation t…
aadarsh-st Feb 2, 2026
834b79e
aadarsh-st/SK-2521:Fixed copilot comments
aadarsh-st Feb 2, 2026
cdde053
aadarsh-st/SK-2521-Refactor request handling in ConnectionController …
aadarsh-st Feb 2, 2026
e31e590
aadarsh-st/SK-2521-Fixed test cases and coverage
aadarsh-st Feb 2, 2026
4957bf1
aadarsh-st/SK-2521-fixed test cases
aadarsh-st Feb 2, 2026
d3d9d90
Merge branch 'himanshu/SK-2519-update-linting-rules-and-fix-hardcoded…
aadarsh-st Feb 3, 2026
b2fc152
aadarsh-st/SK-2521: Removed hardecoded values
aadarsh-st Feb 3, 2026
87d151b
aadarsh-st/SK-2521-Added new constant
aadarsh-st Feb 3, 2026
01404e9
aadarsh-st/SK-2521: Fixed undefined error
aadarsh-st Feb 3, 2026
d4a542c
aadarsh-st/SK-2521: fixed test cases
aadarsh-st Feb 3, 2026
f517484
Merge pull request #273 from skyflowapi/aadarsh-st/SK-2521-fixed-bug-…
aadarsh-st Feb 4, 2026
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
45 changes: 44 additions & 1 deletion checkstyle.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,5 +16,48 @@
<module name="StaticVariableName"/>
<module name="ConstantName"/>
<module name="TypeName"/>

<module name="MultipleStringLiterals">
<property name="allowedDuplicates" value="1"/>
<property name="ignoreOccurrenceContext" value="ANNOTATION"/>
<property name="ignoreStringsRegexp" value='^""$|^"."$'/>
</module>

<module name="StringLiteralEquality"/>

<module name="MagicNumber">
<property name="ignoreNumbers" value="-1,0,1,2,3,64,200,201,204,299,400,401,403,404,500,1000,60000"/>
<property name="ignoreFieldDeclaration" value="true"/>
<property name="ignoreAnnotation" value="true"/>
</module>

<!-- Detect common hardcoded JSON field names in .get() calls -->
<module name="RegexpSinglelineJava">
<property name="format" value='\.get\("(error|message|tokens|records|fields|details|grpc_code|http_status|Body|exp|skyflow_id|privateKey|clientID|keyID|tokenURI)"\)'/>
<property name="message" value="Use Constants.JsonFieldNames or Constants.CredentialFields instead of hardcoded field names. Use IDE search-replace for: error, message, tokens, records, fields, details, grpc_code, http_status, Body, exp, skyflow_id, privateKey, clientID, keyID, tokenURI"/>
<property name="ignoreComments" value="true"/>
</module>

<!-- Detect common hardcoded JSON field names in .put() calls -->
<module name="RegexpSinglelineJava">
<property name="format" value='\.put\("(error|value|token|type|requestIndex|requestId)"\s*,'/>
<property name="message" value="Use Constants.JsonFieldNames instead of hardcoded field names. Use IDE search-replace for: error, value, token, type, requestIndex, requestId"/>
<property name="ignoreComments" value="true"/>
</module>

<!-- Detect common hardcoded JSON field names in .add() and .addProperty() calls -->
<module name="RegexpSinglelineJava">
<property name="format" value='\.(add|addProperty)\("(errors|tokens|value|token|type|error|updatedField)"\s*,'/>
<property name="message" value="Use Constants.JsonFieldNames instead of hardcoded field names. Use IDE search-replace for: errors, tokens, value, token, type, error, updatedField"/>
<property name="ignoreComments" value="true"/>
</module>

<!-- Detect hardcoded RSA algorithm -->
<module name="RegexpSinglelineJava">
<property name="format" value='getInstance\("RSA"\)'/>
<property name="message" value="Use Constants.CryptoAlgorithm.RSA instead of hardcoded 'RSA'"/>
<property name="ignoreComments" value="true"/>
</module>

</module>
</module>
</module>
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -200,7 +200,7 @@
<failsOnError>false</failsOnError>
<consoleOutput>true</consoleOutput>

<excludes>**/generated/**</excludes>
<excludes>**/generated/**,**/logs/**</excludes>

@saileshwar-skyflowsaileshwar-skyflowFeb 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are we excluding logs ?

@skyflow-himanshuskyflow-himanshuFeb 3, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

as log constains strings, and checkstyle linter will catch them, so excluded logs to avoid it catching

</configuration>
<executions>
<execution>
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/skyflow/VaultClient.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -808,9 +808,9 @@ private TokenTypeMapping buildTokenType(TokenFormat tokenFormat,

private FileDataDeidentifyAudioDataFormat mapAudioDataFormat(String dataFormat) throws SkyflowException {
switch (dataFormat) {
case "mp3":
case Constants.FileFormatType.MP3:
return FileDataDeidentifyAudioDataFormat.MP_3;
case "wav":
case Constants.FileFormatType.WAV:
return FileDataDeidentifyAudioDataFormat.WAV;
default:
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.InvalidAudioFileType.getMessage());
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/skyflow/errors/ErrorMessage.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2,6 +2,7 @@

import com.skyflow.utils.Constants;

@SuppressWarnings("checkstyle:MultipleStringLiterals")

@saileshwar-skyflowsaileshwar-skyflowFeb 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why did we add SuppressWarnings annotation here ? What warnings are we suppressing ?

@skyflow-himanshuskyflow-himanshuFeb 3, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

as the file constains strings, and checkstyle linter will catch them, so added SuppressWarnings to avoid it catching

public enum ErrorMessage {
// Client initialization
VaultIdAlreadyInConfigList("%s0 Validation error. VaultId is present in an existing config. Specify a new vaultId in config."),
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/com/skyflow/errors/SkyflowException.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -57,7 +57,7 @@ public SkyflowException(int httpCode, Throwable cause, Map<String, List<String>>

private void setResponseBody(String responseBody, Map<String, List<String>> responseHeaders) {
this.responseBody = JsonParser.parseString(responseBody).getAsJsonObject();
if (this.responseBody.get("error") != null) {
if (this.responseBody.get(Constants.JsonFieldNames.ERROR) != null) {
setGrpcCode();
setHttpStatus();
setMessage();
Expand All@@ -75,17 +75,17 @@ private void setRequestId(Map<String, List<String>> responseHeaders) {
}

private void setMessage() {
JsonElement messageElement = ((JsonObject) responseBody.get("error")).get("message");
JsonElement messageElement = ((JsonObject) responseBody.get(Constants.JsonFieldNames.ERROR)).get(Constants.JsonFieldNames.MESSAGE);
this.message = messageElement == null ? null : messageElement.getAsString();
}

private void setGrpcCode() {
JsonElement grpcElement = ((JsonObject) responseBody.get("error")).get("grpc_code");
JsonElement grpcElement = ((JsonObject) responseBody.get(Constants.JsonFieldNames.ERROR)).get(Constants.JsonFieldNames.GRPC_CODE);
this.grpcCode = grpcElement == null ? null : grpcElement.getAsInt();
}

private void setHttpStatus() {
JsonElement statusElement = ((JsonObject) responseBody.get("error")).get("http_status");
JsonElement statusElement = ((JsonObject) responseBody.get(Constants.JsonFieldNames.ERROR)).get(Constants.JsonFieldNames.HTTP_STATUS);
this.httpStatus = statusElement == null ? null : statusElement.getAsString();
}

Expand All@@ -98,7 +98,7 @@ public JsonArray getDetails() {
}

private void setDetails(Map<String, List<String>> responseHeaders) {
JsonElement detailsElement = ((JsonObject) responseBody.get("error")).get("details");
JsonElement detailsElement = ((JsonObject) responseBody.get(Constants.JsonFieldNames.ERROR)).get(Constants.JsonFieldNames.DETAILS);
List<String> errorFromClientHeader = responseHeaders.get(Constants.ERROR_FROM_CLIENT_HEADER_KEY);
if (detailsElement != null) {
this.details = detailsElement.getAsJsonArray();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/skyflow/logs/InfoLogs.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@ public enum InfoLogs {

// Bearer token generation
EMPTY_BEARER_TOKEN("Bearer token is empty."),
BEARER_TOKEN_EXPIRED("Bearer token is expired."),
BEARER_TOKEN_EXPIRED("Bearer token is invalid or expired."),
GET_BEARER_TOKEN_TRIGGERED("getBearerToken method triggered."),
GET_BEARER_TOKEN_SUCCESS("Bearer token generated."),
GET_SIGNED_DATA_TOKENS_TRIGGERED("getSignedDataTokens method triggered."),
Expand Down
30 changes: 15 additions & 15 deletions src/main/java/com/skyflow/serviceaccount/util/BearerToken.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -92,25 +92,25 @@ private static V1GetAuthTokenResponse getBearerTokenFromCredentials(
JsonObject credentials, String context, ArrayList<String> roles
) throws SkyflowException {
try {
JsonElement privateKey = credentials.get("privateKey");
JsonElement privateKey = credentials.get(Constants.CredentialFields.PRIVATE_KEY);
if (privateKey == null) {
LogUtil.printErrorLog(ErrorLogs.PRIVATE_KEY_IS_REQUIRED.getLog());
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingPrivateKey.getMessage());
}

JsonElement clientID = credentials.get("clientID");
JsonElement clientID = credentials.get(Constants.CredentialFields.CLIENT_ID);
if (clientID == null) {
LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog());
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage());
}

JsonElement keyID = credentials.get("keyID");
JsonElement keyID = credentials.get(Constants.CredentialFields.KEY_ID);
if (keyID == null) {
LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog());
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage());
}

JsonElement tokenURI = credentials.get("tokenURI");
JsonElement tokenURI = credentials.get(Constants.CredentialFields.TOKEN_URI);
if (tokenURI == null) {
LogUtil.printErrorLog(ErrorLogs.TOKEN_URI_IS_REQUIRED.getLog());
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingTokenUri.getMessage());
Expand All@@ -123,7 +123,7 @@ private static V1GetAuthTokenResponse getBearerTokenFromCredentials(

String basePath = Utils.getBaseURL(tokenURI.getAsString());
API_CLIENT_BUILDER.url(basePath);
ApiClient apiClient = API_CLIENT_BUILDER.token("token").build();
ApiClient apiClient = API_CLIENT_BUILDER.token(Constants.ApiToken.TOKEN).build();
AuthenticationClient authenticationApi = apiClient.authentication();

V1GetAuthTokenRequest._FinalStage authTokenBuilder = V1GetAuthTokenRequest.builder().grantType(Constants.GRANT_TYPE).assertion(signedUserJWT);
Expand All@@ -149,11 +149,11 @@ private static String getSignedToken(
final Date createdDate = new Date();
final Date expirationDate = new Date(createdDate.getTime() + (3600 * 1000));
return Jwts.builder()
.claim("iss", clientID)
.claim("key", keyID)
.claim("aud", tokenURI)
.claim("sub", clientID)
.claim("ctx", context)
.claim(Constants.JwtClaims.ISS, clientID)
.claim(Constants.JwtClaims.KEY, keyID)
.claim(Constants.JwtClaims.AUD, tokenURI)
.claim(Constants.JwtClaims.SUB, clientID)
.claim(Constants.JwtClaims.CTX, context)
.expiration(expirationDate)
.signWith(pvtKey, Jwts.SIG.RS256)
.compact();
Expand All@@ -163,7 +163,7 @@ private static String getScopeUsingRoles(ArrayList<String> roles) {
StringBuilder scope = new StringBuilder();
if (roles != null) {
for (String role : roles) {
scope.append(" role:").append(role);
scope.append(Constants.ApiToken.ROLE_PREFIX).append(role);
}
}
return scope.toString();
Expand All@@ -173,10 +173,10 @@ public synchronized String getBearerToken() throws SkyflowException {
LogUtil.printInfoLog(InfoLogs.GET_BEARER_TOKEN_TRIGGERED.getLog());
V1GetAuthTokenResponse response;
String accessToken = null;
if (this.credentialsFile != null && Objects.equals(this.credentialsType, "FILE")) {
if (this.credentialsFile != null && Objects.equals(this.credentialsType, Constants.CredentialTypeValues.FILE)) {
response = generateBearerTokenFromCredentials(this.credentialsFile, this.ctx, this.roles);
accessToken = response.getAccessToken().get();
} else if (this.credentialsString != null && Objects.equals(this.credentialsType, "STRING")) {
} else if (this.credentialsString != null && Objects.equals(this.credentialsType, Constants.CredentialTypeValues.STRING)) {
response = generateBearerTokenFromCredentialString(this.credentialsString, this.ctx, this.roles);
accessToken = response.getAccessToken().get();
}
Expand All@@ -200,13 +200,13 @@ private void setCredentialsType(String credentialsType) {
}

public BearerTokenBuilder setCredentials(File credentialsFile) {
setCredentialsType("FILE");
setCredentialsType(Constants.CredentialTypeValues.FILE);
this.credentialsFile = credentialsFile;
return this;
}

public BearerTokenBuilder setCredentials(String credentialsString) {
setCredentialsType("STRING");
setCredentialsType(Constants.CredentialTypeValues.STRING);
this.credentialsString = credentialsString;
return this;
}
Expand Down
27 changes: 14 additions & 13 deletions src/main/java/com/skyflow/serviceaccount/util/SignedDataTokens.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,7 @@
import com.skyflow.errors.SkyflowException;
import com.skyflow.logs.ErrorLogs;
import com.skyflow.logs.InfoLogs;
import com.skyflow.utils.Constants;
import com.skyflow.utils.Utils;
import com.skyflow.utils.logger.LogUtil;
import io.jsonwebtoken.Jwts;
Expand DownExpand Up@@ -93,19 +94,19 @@ private static List<SignedDataTokenResponse> generateSignedTokensFromCredentials
) throws SkyflowException {
List<SignedDataTokenResponse> signedDataTokens = null;
try {
JsonElement privateKey = credentials.get("privateKey");
JsonElement privateKey = credentials.get(Constants.CredentialFields.PRIVATE_KEY);
if (privateKey == null) {
LogUtil.printErrorLog(ErrorLogs.PRIVATE_KEY_IS_REQUIRED.getLog());
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingPrivateKey.getMessage());
}

JsonElement clientID = credentials.get("clientID");
JsonElement clientID = credentials.get(Constants.CredentialFields.CLIENT_ID);
if (clientID == null) {
LogUtil.printErrorLog(ErrorLogs.CLIENT_ID_IS_REQUIRED.getLog());
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingClientId.getMessage());
}

JsonElement keyID = credentials.get("keyID");
JsonElement keyID = credentials.get(Constants.CredentialFields.KEY_ID);
if (keyID == null) {
LogUtil.printErrorLog(ErrorLogs.KEY_ID_IS_REQUIRED.getLog());
throw new SkyflowException(ErrorCode.INVALID_INPUT.getCode(), ErrorMessage.MissingKeyId.getMessage());
Expand DownExpand Up@@ -136,12 +137,12 @@ private static List<SignedDataTokenResponse> getSignedToken(
List<SignedDataTokenResponse> list = new ArrayList<>();
for (String dataToken : dataTokens) {
String eachSignedDataToken = Jwts.builder()
.claim("iss", "sdk")
.claim("iat", (createdDate.getTime() / 1000))
.claim("key", keyID)
.claim("sub", clientID)
.claim("ctx", context)
.claim("tok", dataToken)
.claim(Constants.JwtClaims.ISS, Constants.JwtClaims.SDK)
.claim(Constants.JwtClaims.IAT, (createdDate.getTime() / 1000))
.claim(Constants.JwtClaims.KEY, keyID)
.claim(Constants.JwtClaims.SUB, clientID)
.claim(Constants.JwtClaims.CTX, context)
.claim(Constants.JwtClaims.TOK, dataToken)
.expiration(expirationDate)
.signWith(pvtKey, Jwts.SIG.RS256)
.compact();
Expand All@@ -154,9 +155,9 @@ private static List<SignedDataTokenResponse> getSignedToken(
public synchronized List<SignedDataTokenResponse> getSignedDataTokens() throws SkyflowException {
LogUtil.printInfoLog(InfoLogs.GET_SIGNED_DATA_TOKENS_TRIGGERED.getLog());
List<SignedDataTokenResponse> signedToken = new ArrayList<>();
if (this.credentialsFile != null && Objects.equals(this.credentialsType, "FILE")) {
if (this.credentialsFile != null && Objects.equals(this.credentialsType, Constants.CredentialTypeValues.FILE)) {
signedToken = generateSignedTokenFromCredentialsFile(this.credentialsFile, this.dataTokens, this.timeToLive, this.ctx);
} else if (this.credentialsString != null && Objects.equals(this.credentialsType, "STRING")) {
} else if (this.credentialsString != null && Objects.equals(this.credentialsType, Constants.CredentialTypeValues.STRING)) {
signedToken = generateSignedTokensFromCredentialsString(this.credentialsString, this.dataTokens, this.timeToLive, this.ctx);
}
LogUtil.printInfoLog(InfoLogs.GET_SIGNED_DATA_TOKEN_SUCCESS.getLog());
Expand All@@ -179,13 +180,13 @@ private void setCredentialsType(String credentialsType) {
}

public SignedDataTokensBuilder setCredentials(File credentialsFile) {
setCredentialsType("FILE");
setCredentialsType(Constants.CredentialTypeValues.FILE);
this.credentialsFile = credentialsFile;
return this;
}

public SignedDataTokensBuilder setCredentials(String credentialsString) {
setCredentialsType("STRING");
setCredentialsType(Constants.CredentialTypeValues.STRING);
this.credentialsString = credentialsString;
return this;
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/skyflow/serviceaccount/util/Token.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,6 +8,7 @@
import com.skyflow.errors.SkyflowException;
import com.skyflow.logs.ErrorLogs;
import com.skyflow.logs.InfoLogs;
import com.skyflow.utils.Constants;
import com.skyflow.utils.logger.LogUtil;
import org.apache.commons.codec.binary.Base64;

Expand All@@ -25,7 +26,7 @@ public static boolean isExpired(String token) {
}

currentTime = new Date().getTime() / 1000;
expiryTime = decoded(token).get("exp").getAsLong();
expiryTime = decoded(token).get(Constants.JsonFieldNames.EXP).getAsLong();

} catch (JsonSyntaxException | SkyflowException e) {
LogUtil.printErrorLog(ErrorLogs.INVALID_BEARER_TOKEN.getLog());
Expand Down
Loading
Loading