[findbugs] Null checks on closing a repository in ITicketService

James Moger

2014-04-17 9ee8b1b793113e74a2fffc1e2aa764279d1c686d

 src/main/java/com/gitblit/tickets/ITicketService.java

@@ -167,13 +167,13 @@

   /**
    * Start the service.
    *
    * @since 1.4.0
    */
   public abstract ITicketService start();

   /**
    * Stop the service.
    *
    * @since 1.4.0
    */
   public final ITicketService stop() {
      indexer.close();
@@ -185,7 +185,7 @@

   /**
    * Creates a ticket notifier.  The ticket notifier is not thread-safe!
    *
    * @since 1.4.0
    */
   public TicketNotifier createNotifier() {
      return new TicketNotifier(
@@ -200,6 +200,7 @@
    * Returns the ready status of the ticket service.
    *
    * @return true if the ticket service is ready
    * @since 1.4.0
    */
   public boolean isReady() {
      return true;
@@ -210,6 +211,7 @@
    *
    * @param repository
    * @return true if patchsets are being accepted
    * @since 1.4.0
    */
   public boolean isAcceptingNewPatchsets(RepositoryModel repository) {
      return isReady()
@@ -224,6 +226,7 @@
    *
    * @param repository
    * @return true if tickets are being accepted
    * @since 1.4.0
    */
   public boolean isAcceptingNewTickets(RepositoryModel repository) {
      return isReady()
@@ -237,6 +240,7 @@
    *
    * @param repository
    * @return true if tickets are allowed to be updated
    * @since 1.4.0
    */
   public boolean isAcceptingTicketUpdates(RepositoryModel repository) {
      return isReady()
@@ -249,6 +253,7 @@
    * Returns true if the repository has any tickets
    * @param repository
    * @return true if the repository has tickets
    * @since 1.4.0
    */
   public boolean hasTickets(RepositoryModel repository) {
      return indexer.hasTickets(repository);
@@ -256,11 +261,13 @@

   /**
    * Closes any open resources used by this service.
    * @since 1.4.0
    */
   protected abstract void close();

   /**
    * Reset all caches in the service.
    * @since 1.4.0
    */
   public final synchronized void resetCaches() {
      ticketsCache.invalidateAll();
@@ -269,10 +276,15 @@
      resetCachesImpl();
   }

   /**
    * Reset all caches in the service.
    * @since 1.4.0
    */
   protected abstract void resetCachesImpl();

   /**
    * Reset any caches for the repository in the service.
    * @since 1.4.0
    */
   public final synchronized void resetCaches(RepositoryModel repository) {
      List<TicketKey> repoKeys = new ArrayList<TicketKey>();
@@ -287,6 +299,12 @@
      resetCachesImpl(repository);
   }

   /**
    * Reset the caches for the specified repository.
    *
    * @param repository
    * @since 1.4.0
    */
   protected abstract void resetCachesImpl(RepositoryModel repository);


@@ -295,6 +313,7 @@
    *
    * @param repository
    * @return the list of labels
    * @since 1.4.0
    */
   public List<TicketLabel> getLabels(RepositoryModel repository) {
      String key = repository.name;
@@ -327,6 +346,7 @@
    * @param repository
    * @param label
    * @return a TicketLabel
    * @since 1.4.0
    */
   public TicketLabel getLabel(RepositoryModel repository, String label) {
      for (TicketLabel tl : getLabels(repository)) {
@@ -346,6 +366,7 @@
    * @param milestone
    * @param createdBy
    * @return the label
    * @since 1.4.0
    */
   public synchronized TicketLabel createLabel(RepositoryModel repository, String label, String createdBy) {
      TicketLabel lb = new TicketMilestone(label);
@@ -358,7 +379,9 @@
      } catch (IOException e) {
         log.error("failed to create label " + label + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return lb;
   }
@@ -370,6 +393,7 @@
    * @param label
    * @param createdBy
    * @return true if the update was successful
    * @since 1.4.0
    */
   public synchronized boolean updateLabel(RepositoryModel repository, TicketLabel label, String createdBy) {
      Repository db = null;
@@ -383,7 +407,9 @@
      } catch (IOException e) {
         log.error("failed to update label " + label + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return false;
   }
@@ -396,6 +422,7 @@
    * @param newName
    * @param createdBy
    * @return true if the rename was successful
    * @since 1.4.0
    */
   public synchronized boolean renameLabel(RepositoryModel repository, String oldName, String newName, String createdBy) {
      if (StringUtils.isEmpty(newName)) {
@@ -421,7 +448,9 @@
      } catch (IOException e) {
         log.error("failed to rename label " + oldName + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return false;
   }
@@ -433,6 +462,7 @@
    * @param label
    * @param createdBy
    * @return true if the delete was successful
    * @since 1.4.0
    */
   public synchronized boolean deleteLabel(RepositoryModel repository, String label, String createdBy) {
      if (StringUtils.isEmpty(label)) {
@@ -449,7 +479,9 @@
      } catch (IOException e) {
         log.error("failed to delete label " + label + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return false;
   }
@@ -459,6 +491,7 @@
    *
    * @param repository
    * @return the list of milestones
    * @since 1.4.0
    */
   public List<TicketMilestone> getMilestones(RepositoryModel repository) {
      String key = repository.name;
@@ -500,6 +533,7 @@
    * @param repository
    * @param status
    * @return the list of milestones
    * @since 1.4.0
    */
   public List<TicketMilestone> getMilestones(RepositoryModel repository, Status status) {
      List<TicketMilestone> matches = new ArrayList<TicketMilestone>();
@@ -517,6 +551,7 @@
    * @param repository
    * @param milestone
    * @return the milestone or null if it does not exist
    * @since 1.4.0
    */
   public TicketMilestone getMilestone(RepositoryModel repository, String milestone) {
      for (TicketMilestone ms : getMilestones(repository)) {
@@ -536,6 +571,7 @@
    * @param milestone
    * @param createdBy
    * @return the milestone
    * @since 1.4.0
    */
   public synchronized TicketMilestone createMilestone(RepositoryModel repository, String milestone, String createdBy) {
      TicketMilestone ms = new TicketMilestone(milestone);
@@ -551,7 +587,9 @@
      } catch (IOException e) {
         log.error("failed to create milestone " + milestone + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return ms;
   }
@@ -563,6 +601,7 @@
    * @param milestone
    * @param createdBy
    * @return true if successful
    * @since 1.4.0
    */
   public synchronized boolean updateMilestone(RepositoryModel repository, TicketMilestone milestone, String createdBy) {
      Repository db = null;
@@ -582,7 +621,9 @@
      } catch (IOException e) {
         log.error("failed to update milestone " + milestone + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return false;
   }
@@ -595,6 +636,7 @@
    * @param newName
    * @param createdBy
    * @return true if successful
    * @since 1.4.0
    */
   public synchronized boolean renameMilestone(RepositoryModel repository, String oldName, String newName, String createdBy) {
      if (StringUtils.isEmpty(newName)) {
@@ -629,10 +671,13 @@
      } catch (IOException e) {
         log.error("failed to rename milestone " + oldName + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return false;
   }

   /**
    * Deletes a milestone.
    *
@@ -640,6 +685,7 @@
    * @param milestone
    * @param createdBy
    * @return true if successful
    * @since 1.4.0
    */
   public synchronized boolean deleteMilestone(RepositoryModel repository, String milestone, String createdBy) {
      if (StringUtils.isEmpty(milestone)) {
@@ -658,7 +704,9 @@
      } catch (IOException e) {
         log.error("failed to delete milestone " + milestone + " in " + repository, e);
      } finally {
         db.close();
         if (db != null) {
            db.close();
         }
      }
      return false;
   }
@@ -668,6 +716,7 @@
    *
    * @param repository
    * @return a new ticket id
    * @since 1.4.0
    */
   public abstract long assignNewId(RepositoryModel repository);

@@ -677,6 +726,7 @@
    * @param repository
    * @param ticketId
    * @return true if the ticket exists
    * @since 1.4.0
    */
   public abstract boolean hasTicket(RepositoryModel repository, long ticketId);

@@ -685,6 +735,7 @@
    *
    * @param repository
    * @return all tickets
    * @since 1.4.0
    */
   public List<TicketModel> getTickets(RepositoryModel repository) {
      return getTickets(repository, null);
@@ -700,6 +751,7 @@
    * @param filter
    *            optional issue filter to only return matching results
    * @return a list of tickets
    * @since 1.4.0
    */
   public abstract List<TicketModel> getTickets(RepositoryModel repository, TicketFilter filter);

@@ -709,31 +761,35 @@
    * @param repository
    * @param ticketId
    * @return a ticket, if it exists, otherwise null
    * @since 1.4.0
    */
   public final TicketModel getTicket(RepositoryModel repository, long ticketId) {
      TicketKey key = new TicketKey(repository, ticketId);
      TicketModel ticket = ticketsCache.getIfPresent(key);

      // if ticket not cached
      if (ticket == null) {
         // load & cache ticket
         //load ticket
         ticket = getTicketImpl(repository, ticketId);
         if (ticket.hasPatchsets()) {
            Repository r = repositoryManager.getRepository(repository.name);
            try {
               Patchset patchset = ticket.getCurrentPatchset();
               DiffStat diffStat = DiffUtils.getDiffStat(r, patchset.base, patchset.tip);
               // diffstat could be null if we have ticket data without the
               // commit objects.  e.g. ticket replication without repo
               // mirroring
               if (diffStat != null) {
                  ticket.insertions = diffStat.getInsertions();
                  ticket.deletions = diffStat.getDeletions();
               }
            } finally {
               r.close();
            }
         }
         // if ticket exists
         if (ticket != null) {
            if (ticket.hasPatchsets()) {
               Repository r = repositoryManager.getRepository(repository.name);
               try {
                  Patchset patchset = ticket.getCurrentPatchset();
                  DiffStat diffStat = DiffUtils.getDiffStat(r, patchset.base, patchset.tip);
                  // diffstat could be null if we have ticket data without the
                  // commit objects.  e.g. ticket replication without repo
                  // mirroring
                  if (diffStat != null) {
                     ticket.insertions = diffStat.getInsertions();
                     ticket.deletions = diffStat.getDeletions();
                  }
               } finally {
                  r.close();
               }
            }
            //cache ticket
            ticketsCache.put(key, ticket);
         }
      }
@@ -746,6 +802,7 @@
    * @param repository
    * @param ticketId
    * @return a ticket, if it exists, otherwise null
    * @since 1.4.0
    */
   protected abstract TicketModel getTicketImpl(RepositoryModel repository, long ticketId);

@@ -754,6 +811,7 @@
    *
    * @param ticket
    * @return the ticket url
    * @since 1.4.0
    */
   public String getTicketUrl(TicketModel ticket) {
      final String canonicalUrl = settings.getString(Keys.web.canonicalUrl, "https://localhost:8443");
@@ -767,6 +825,7 @@
    * @param base
    * @param tip
    * @return the compare url
    * @since 1.4.0
    */
   public String getCompareUrl(TicketModel ticket, String base, String tip) {
      final String canonicalUrl = settings.getString(Keys.web.canonicalUrl, "https://localhost:8443");
@@ -778,6 +837,7 @@
    * Returns true if attachments are supported.
    *
    * @return true if attachments are supported
    * @since 1.4.0
    */
   public abstract boolean supportsAttachments();

@@ -788,6 +848,7 @@
    * @param ticketId
    * @param filename
    * @return an attachment, if found, null otherwise
    * @since 1.4.0
    */
   public abstract Attachment getAttachment(RepositoryModel repository, long ticketId, String filename);

@@ -799,6 +860,7 @@
    * @param repository
    * @param change
    * @return true if successful
    * @since 1.4.0
    */
   public TicketModel createTicket(RepositoryModel repository, Change change) {
      return createTicket(repository, 0L, change);
@@ -813,6 +875,7 @@
    * @param ticketId (if <=0 the ticket id will be assigned)
    * @param change
    * @return true if successful
    * @since 1.4.0
    */
   public TicketModel createTicket(RepositoryModel repository, long ticketId, Change change) {

@@ -861,6 +924,7 @@
    * @param ticketId
    * @param change
    * @return the ticket model if successful
    * @since 1.4.0
    */
   public final TicketModel updateTicket(RepositoryModel repository, long ticketId, Change change) {
      if (change == null) {
@@ -899,6 +963,7 @@
    * Deletes all tickets in every repository.
    *
    * @return true if successful
    * @since 1.4.0
    */
   public boolean deleteAll() {
      List<String> repositories = repositoryManager.getRepositoryList();
@@ -921,6 +986,7 @@
    * Deletes all tickets in the specified repository.
    * @param repository
    * @return true if succesful
    * @since 1.4.0
    */
   public boolean deleteAll(RepositoryModel repository) {
      boolean success = deleteAllImpl(repository);
@@ -932,6 +998,12 @@
      return success;
   }

   /**
    * Delete all tickets for the specified repository.
    * @param repository
    * @return true if successful
    * @since 1.4.0
    */
   protected abstract boolean deleteAllImpl(RepositoryModel repository);

   /**
@@ -940,6 +1012,7 @@
    * @param oldRepositoryName
    * @param newRepositoryName
    * @return true if successful
    * @since 1.4.0
    */
   public boolean rename(RepositoryModel oldRepository, RepositoryModel newRepository) {
      if (renameImpl(oldRepository, newRepository)) {
@@ -951,6 +1024,14 @@
      return false;
   }

   /**
    * Renames a repository.
    *
    * @param oldRepository
    * @param newRepository
    * @return true if successful
    * @since 1.4.0
    */
   protected abstract boolean renameImpl(RepositoryModel oldRepository, RepositoryModel newRepository);

   /**
@@ -960,6 +1041,7 @@
    * @param ticketId
    * @param deletedBy
    * @return true if successful
    * @since 1.4.0
    */
   public boolean deleteTicket(RepositoryModel repository, long ticketId, String deletedBy) {
      TicketModel ticket = getTicket(repository, ticketId);
@@ -981,6 +1063,7 @@
    * @param ticket
    * @param deletedBy
    * @return true if successful
    * @since 1.4.0
    */
   protected abstract boolean deleteTicketImpl(RepositoryModel repository, TicketModel ticket, String deletedBy);

@@ -996,6 +1079,7 @@
    * @param comment
    *            the revised comment
    * @return the revised ticket if the change was successful
    * @since 1.4.0
    */
   public final TicketModel updateComment(TicketModel ticket, String commentId,
         String updatedBy, String comment) {
@@ -1016,6 +1100,7 @@
    * @param deletedBy
    *          the user deleting the comment
    * @return the revised ticket if the deletion was successful
    * @since 1.4.0
    */
   public final TicketModel deleteComment(TicketModel ticket, String commentId, String deletedBy) {
      Change deletion = new Change(deletedBy);
@@ -1034,6 +1119,7 @@
    * @param ticketId
    * @param change
    * @return true, if the change was committed
    * @since 1.4.0
    */
   protected abstract boolean commitChangeImpl(RepositoryModel repository, long ticketId, Change change);

@@ -1048,6 +1134,7 @@
    * @param page
    * @param pageSize
    * @return a list of matching tickets
    * @since 1.4.0
    */
   public List<QueryResult> searchFor(RepositoryModel repository, String text, int page, int pageSize) {
      return indexer.searchFor(repository, text, page, pageSize);
@@ -1062,6 +1149,7 @@
    * @param sortBy
    * @param descending
    * @return a list of matching tickets or an empty list
    * @since 1.4.0
    */
   public List<QueryResult> queryFor(String query, int page, int pageSize, String sortBy, boolean descending) {
      return indexer.queryFor(query, page, pageSize, sortBy, descending);
@@ -1070,6 +1158,7 @@
   /**
    * Destroys an existing index and reindexes all tickets.
    * This operation may be expensive and time-consuming.
    * @since 1.4.0
    */
   public void reindex() {
      long start = System.nanoTime();
@@ -1096,6 +1185,7 @@
   /**
    * Destroys any existing index and reindexes all tickets.
    * This operation may be expensive and time-consuming.
    * @since 1.4.0
    */
   public void reindex(RepositoryModel repository) {
      long start = System.nanoTime();
@@ -1113,6 +1203,7 @@
    * of ticket updates, namely merging from the web ui.
    *
    * @param runnable
    * @since 1.4.0
    */
   public synchronized void exec(Runnable runnable) {
      runnable.run();