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

Switch order of literals to prevent NullPointerException #868

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Switch order of literals to prevent NullPointerException #868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e2d1f71
Switch order of literals to prevent NullPointerException
pixeebot Jan 20, 2024
4064ddf
Merge pull request #6 from citizenjosh/pixeebot/drip-2024-01-20-pixee…
citizenjosh Jan 23, 2024
be1ab4d
remove change
citizenjosh Jan 24, 2024
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
2 changes: 1 addition & 1 deletion agent/src/main/java/com/walmartlabs/concord/agent/executors/runner/JobDependencies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ private static Map<String, String> getDependencyVersionsFromFile(RunnerJob job)
}

private static boolean isLatestVersion(String v) {
return v.equalsIgnoreCase("latest");
return "latest".equalsIgnoreCase(v);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok.

}

private static String assertVersion(String dep, Map<String, String> versions) {
Expand Down
4 changes: 2 additions & 2 deletions plugins/tasks/lock/src/main/java/com/walmartlabs/concord/plugins/lock/LockTaskCommon.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ private String checkScope(String scope) {
return null;
}

if (!scope.equalsIgnoreCase("ORG")
&& !scope.equalsIgnoreCase("PROJECT")) {
if (!"ORG".equalsIgnoreCase(scope)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Checked before.

Copy link
Author

Choose a reason for hiding this comment

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

The purpose of this change is to be proactively defensive. Though it may be negligibly faster to run this way, it also conforms to a best practice and makes the code easier to read. Also, since the behavior is otherwise equivalent, there is no cost to this change.

&& !"PROJECT".equalsIgnoreCase(scope)) {

throw new IllegalArgumentException("Unknown scope '" + scope + "', expected: org or project");
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/v1/impl/src/main/java/com/walmartlabs/concord/runner/ConcordSecurityManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void checkPermission(Permission perm) {
String path = perm.getName();

// allow all local paths
if (!path.startsWith("/") && !path.equals(ALL_FILES_TOKEN)) {
if (!path.startsWith("/") && !ALL_FILES_TOKEN.equals(path)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cannot be null here.

Copy link
Author

Choose a reason for hiding this comment

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

The purpose of this change is to be proactively defensive. Though it may be negligibly faster to run this way, it also conforms to a best practice and makes the code easier to read. Also, since the behavior is otherwise equivalent, there is no cost to this change.

return;
}

Expand Down
2 changes: 1 addition & 1 deletion ...ect-model/src/main/java/com/walmartlabs/concord/project/yaml/converter/StepConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ default Set<VariableMapping> getVarMap(Map<String, Object> options, String key)
result.add(new VariableMapping(null, sourceExpr, sourceValue, target, true));
}

if (key.equals("in") && getWithItems(options) != null) {
if ("in".equals(key) && getWithItems(options) != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same.

Copy link
Author

Choose a reason for hiding this comment

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

The purpose of this change is to be proactively defensive. Though it may be negligibly faster to run this way, it also conforms to a best practice and makes the code easier to read. Also, since the behavior is otherwise equivalent, there is no cost to this change.

result = appendWithItemsVar(result);
}
return result;
Expand Down
4 changes: 2 additions & 2 deletions server/impl/src/main/java/com/walmartlabs/concord/server/org/project/ProjectValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ private static String getEntryPoint(Trigger t) {

private static void validateSpec(Trigger t, List<String> errors) {
String triggerName = t.name();
if (!triggerName.equals("cron")) {
if (!"cron".equals(triggerName)) {
return;
}

Expand All @@ -121,7 +121,7 @@ private static void validateSpec(Trigger t, List<String> errors) {

private static void validateTimezone(Trigger t, List<String> errors) {
String triggerName = t.name();
if (!triggerName.equals("cron")) {
if (!"cron".equals(triggerName)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe useful?

Copy link
Author

Choose a reason for hiding this comment

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

The purpose of this change is to be proactively defensive. Though it may be negligibly faster to run this way, it also conforms to a best practice and makes the code easier to read. Also, since the behavior is otherwise equivalent, there is no cost to this change.

return;
}

Expand Down
2 changes: 1 addition & 1 deletion server/impl/src/main/java/com/walmartlabs/concord/server/user/UserManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ private UserType assertUserType(String username, String domain, UserType type) {
}

private UserType assertSsoUserType(UserPrincipal u, UserType type) {
if (u.getRealm().equals(SSO_REALM_NAME)) {
if (SSO_REALM_NAME.equals(u.getRealm())) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not.

if (userInfoProviders.get(UserType.SSO) != null) {
return UserType.SSO;
}
Expand Down
2 changes: 1 addition & 1 deletion ...ibase-ext/src/main/java/com/walmartlabs/concord/server/liquibase/ext/ApiTokenCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public String getConfirmationMessage() {

@Override
public void setUp() {
if (this.skip.equals("true")) {
if ("true".equals(this.skip)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok.

this.token = null;
return;
}
Expand Down
2 changes: 1 addition & 1 deletion .../src/main/java/com/walmartlabs/concord/server/plugins/oidc/OidcAuthenticationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public AuthenticationToken createToken(ServletRequest request, ServletResponse r
String header = req.getHeader(AUTHORIZATION_HEADER);
if (header != null) {
String[] as = header.split(" ");
if (as.length != 2 || !as[0].equals(HEADER_PREFIX)) {
if (as.length != 2 || !HEADER_PREFIX.equals(as[0])) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cannot be null.

Copy link
Author

@citizenjosh citizenjosh Jan 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The purpose of this change is to be proactively defensive. Though it may be negligibly faster to run this way, it also conforms to a best practice and makes the code easier to read. Also, since the behavior is otherwise equivalent, there is no cost to this change.

return null;
}

Expand Down
Loading