Avoid crash if button configured having none existing repo/project #154

Author: tomasbjerre

CHANGELOG.md

@@ -3,12 +3,12 @@
 Changelog of Pull Request Notifier for Bitbucket.
 
 ## Unreleased
-### No issue
-  Replacing unresolved variables with nothing
+### GitHub [#159](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/issues/159) Replacement failure can result in bad URLs
+  Replacing unresolved variables with nothing 
 
  * If a crash happens, like NPE, when a variable was resolved. Then that variable would be kept unchanged. Resulting in illegal chars in URL.
 
-  [f279f19796c0fed](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/f279f19796c0fed) Tomas Bjerre *2016-11-16 20:23:33*
+  [316cb796c3eedcf](https://github.com/tomasbjerre/pull-request-notifier-for-bitbucket/commit/316cb796c3eedcf) Tomas Bjerre *2016-11-16 20:39:20*
 
 ## 2.38
 ### No issue

src/main/java/se/bjurr/prnfb/service/UserCheckService.java

@@ -6,15 +6,15 @@
 import static com.google.common.base.Strings.emptyToNull;
 import static com.google.common.base.Throwables.propagate;
 import static com.google.common.collect.Iterables.filter;
+import static org.slf4j.LoggerFactory.getLogger;
 import static se.bjurr.prnfb.settings.USER_LEVEL.ADMIN;
 import static se.bjurr.prnfb.settings.USER_LEVEL.EVERYONE;
 
 import java.util.List;
 
 import javax.annotation.Nullable;
 
-import se.bjurr.prnfb.settings.PrnfbButton;
-import se.bjurr.prnfb.settings.USER_LEVEL;
+import org.slf4j.Logger;
 
 import com.atlassian.bitbucket.permission.PermissionService;
 import com.atlassian.bitbucket.project.Project;
@@ -29,7 +29,11 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Predicate;
 
+import se.bjurr.prnfb.settings.PrnfbButton;
+import se.bjurr.prnfb.settings.USER_LEVEL;
+
 public class UserCheckService {
+ private static final Logger LOG = getLogger(UserCheckService.class);
  private final PermissionService permissionService;
  private final ProjectService projectService;
  private final RepositoryService repositoryService;
@@ -57,8 +61,40 @@ public boolean apply(PrnfbButton input) {
   return allowedButtons;
  }
 
+ @VisibleForTesting
+ private Project getProject(String projectKey) {
+  try {
+   return securityService//
+     .withPermission(SYS_ADMIN, "Getting project")//
+     .call(new Operation<Project, Exception>() {
+      @Override
+      public Project perform() throws Exception {
+       return projectService.getByKey(projectKey);
+      }
+     });
+  } catch (Exception e) {
+   throw propagate(e);
+  }
+ }
+
+ @VisibleForTesting
+ Repository getRepo(String projectKey, String repositorySlug) {
+  try {
+   return securityService//
+     .withPermission(SYS_ADMIN, "Getting repo")//
+     .call(new Operation<Repository, Exception>() {
+      @Override
+      public Repository perform() throws Exception {
+       return repositoryService.getBySlug(projectKey, repositorySlug);
+      }
+     });
+  } catch (Exception e) {
+   throw propagate(e);
+  }
+ }
+
  public boolean isAdmin(UserKey userKey, String projectKey, String repositorySlug) {
-  boolean isAdmin = this.userManager.isAdmin(userKey);
+  boolean isAdmin = userManager.isAdmin(userKey);
   if (isAdmin) {
    return isAdmin;
   }
@@ -68,15 +104,24 @@ public boolean isAdmin(UserKey userKey, String projectKey, String repositorySlug
 
   if (projectKey != null && repositorySlug == null) {
    Project project = getProject(projectKey);
-   boolean isAllowed = this.permissionService.hasProjectPermission(project, PROJECT_ADMIN);
+   if (project == null) {
+    LOG.error("Button with project " + projectKey + " configured. But no such project exists!");
+    return false;
+   }
+   boolean isAllowed = permissionService.hasProjectPermission(project, PROJECT_ADMIN);
    if (isAllowed) {
     return true;
    }
   }
 
   if (projectKey != null && repositorySlug != null) {
    Repository repository = getRepo(projectKey, repositorySlug);
-   return this.permissionService.hasRepositoryPermission(repository, REPO_ADMIN);
+   if (repository == null) {
+    LOG.error(
+      "Button with project " + projectKey + " and repo " + repositorySlug + " configured. But no such repo exists!");
+    return false;
+   }
+   return permissionService.hasRepositoryPermission(repository, REPO_ADMIN);
   }
   return false;
  }
@@ -85,14 +130,28 @@ public boolean isAdmin(UserKey userKey, String projectKey, String repositorySlug
   * null if global.
   */
  public boolean isAdminAllowed(@Nullable String projectKey, @Nullable String repositorySlug) {
-  final UserProfile user = this.userManager.getRemoteUser();
+  final UserProfile user = userManager.getRemoteUser();
   if (user == null) {
    return false;
   }
-  USER_LEVEL adminRestriction = this.settingsService.getPrnfbSettingsData().getAdminRestriction();
+  USER_LEVEL adminRestriction = settingsService.getPrnfbSettingsData().getAdminRestriction();
   return isAdminAllowed(adminRestriction, projectKey, repositorySlug);
  }
 
+ private boolean isAdminAllowed(USER_LEVEL adminRestriction, @Nullable String projectKey,
+   @Nullable String repositorySlug) {
+  UserKey userKey = userManager.getRemoteUser().getUserKey();
+  boolean isAdmin = isAdmin(userKey, projectKey, repositorySlug);
+  boolean isSystemAdmin = isSystemAdmin(userKey);
+  return isAdminAllowedCheck(adminRestriction, isAdmin, isSystemAdmin);
+ }
+
+ boolean isAdminAllowedCheck(USER_LEVEL userLevel, boolean isAdmin, boolean isSystemAdmin) {
+  return userLevel == EVERYONE //
+    || isSystemAdmin //
+    || isAdmin && userLevel == ADMIN;
+ }
+
  public boolean isAllowedUseButton(PrnfbButton candidate) {
   return isAdminAllowed(//
     candidate.getUserLevel(), //
@@ -101,61 +160,15 @@ public boolean isAllowedUseButton(PrnfbButton candidate) {
  }
 
  public boolean isSystemAdmin(UserKey userKey) {
-  return this.userManager.isSystemAdmin(userKey);
+  return userManager.isSystemAdmin(userKey);
  }
 
  public boolean isViewAllowed() {
-  UserProfile user = this.userManager.getRemoteUser();
+  UserProfile user = userManager.getRemoteUser();
   if (user == null) {
    return false;
   }
   return true;
  }
 
- @VisibleForTesting
- private Project getProject(String projectKey) {
-  try {
-   return this.securityService//
-     .withPermission(SYS_ADMIN, "Getting project")//
-     .call(new Operation<Project, Exception>() {
-      @Override
-      public Project perform() throws Exception {
-       return UserCheckService.this.projectService.getByKey(projectKey);
-      }
-     });
-  } catch (Exception e) {
-   throw propagate(e);
-  }
- }
-
- private boolean isAdminAllowed(USER_LEVEL adminRestriction, @Nullable String projectKey,
-   @Nullable String repositorySlug) {
-  UserKey userKey = this.userManager.getRemoteUser().getUserKey();
-  boolean isAdmin = isAdmin(userKey, projectKey, repositorySlug);
-  boolean isSystemAdmin = isSystemAdmin(userKey);
-  return isAdminAllowedCheck(adminRestriction, isAdmin, isSystemAdmin);
- }
-
- @VisibleForTesting
- Repository getRepo(String projectKey, String repositorySlug) {
-  try {
-   return this.securityService//
-     .withPermission(SYS_ADMIN, "Getting repo")//
-     .call(new Operation<Repository, Exception>() {
-      @Override
-      public Repository perform() throws Exception {
-       return UserCheckService.this.repositoryService.getBySlug(projectKey, repositorySlug);
-      }
-     });
-  } catch (Exception e) {
-   throw propagate(e);
-  }
- }
-
- boolean isAdminAllowedCheck(USER_LEVEL userLevel, boolean isAdmin, boolean isSystemAdmin) {
-  return userLevel == EVERYONE //
-    || isSystemAdmin //
-    || isAdmin && userLevel == ADMIN;
- }
-
 }