New draft statuses and magic branches
Adds draft status to Change. DRAFT status in change occurs before NEW and will be for a change that is not meant for review. Also adds magic branches refs/draft/ and refs/publish/ that will handle whether or not a patchset is a draft or goes straight to review. refs/for/ should be deprecated in favor of explicitly marking a patchset as a draft or directly to review. For now, the draft flag and status are unused and refs/draft and refs/publish just mimic refs/for's functionality. Change-Id: I95cb336f1944552f842f0b99dc299eb51e586801
This commit is contained in:
		@@ -23,11 +23,11 @@ import com.google.gerrit.reviewdb.Branch;
 | 
			
		||||
import com.google.gerrit.reviewdb.Project;
 | 
			
		||||
import com.google.gerrit.server.IdentifiedUser;
 | 
			
		||||
import com.google.gerrit.server.git.GitRepositoryManager;
 | 
			
		||||
import com.google.gerrit.server.git.ReceiveCommits;
 | 
			
		||||
import com.google.gerrit.server.git.ReplicationQueue;
 | 
			
		||||
import com.google.gerrit.server.project.NoSuchProjectException;
 | 
			
		||||
import com.google.gerrit.server.project.ProjectControl;
 | 
			
		||||
import com.google.gerrit.server.project.RefControl;
 | 
			
		||||
import com.google.gerrit.server.util.MagicBranch;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
import com.google.inject.assistedinject.Assisted;
 | 
			
		||||
 | 
			
		||||
@@ -106,8 +106,8 @@ class AddBranch extends Handler<ListBranchesResult> {
 | 
			
		||||
    if (!Repository.isValidRefName(refname)) {
 | 
			
		||||
      throw new InvalidNameException();
 | 
			
		||||
    }
 | 
			
		||||
    if (refname.startsWith(ReceiveCommits.NEW_CHANGE)) {
 | 
			
		||||
      throw new BranchCreationNotAllowedException(ReceiveCommits.NEW_CHANGE);
 | 
			
		||||
    if (MagicBranch.isMagicBranch(refname)) {
 | 
			
		||||
      throw new BranchCreationNotAllowedException(refname);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    final Branch.NameKey name = new Branch.NameKey(projectName, refname);
 | 
			
		||||
 
 | 
			
		||||
@@ -185,6 +185,8 @@ public final class Change {
 | 
			
		||||
  protected static final char STATUS_NEW = 'n';
 | 
			
		||||
  /** Database constant for {@link Status#SUBMITTED}. */
 | 
			
		||||
  protected static final char STATUS_SUBMITTED = 's';
 | 
			
		||||
  /** Database constant for {@link Status#DRAFT}. */
 | 
			
		||||
  protected static final char STATUS_DRAFT = 'd';
 | 
			
		||||
  /** Maximum database status constant for an open change. */
 | 
			
		||||
  private static final char MAX_OPEN = 'z';
 | 
			
		||||
 | 
			
		||||
@@ -245,6 +247,24 @@ public final class Change {
 | 
			
		||||
     */
 | 
			
		||||
    SUBMITTED(STATUS_SUBMITTED),
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * Change is a draft change that only consists of draft patchsets.
 | 
			
		||||
     *
 | 
			
		||||
     * <p>
 | 
			
		||||
     * This is a change that is not meant to be submitted or reviewed yet. If
 | 
			
		||||
     * the uploader publishes the change, it becomes a NEW change.
 | 
			
		||||
     * Publishing is a one-way action, a change cannot return to DRAFT status.
 | 
			
		||||
     * Draft changes are only visible to the uploader and those explicitly
 | 
			
		||||
     * added as reviewers.
 | 
			
		||||
     *
 | 
			
		||||
     * <p>
 | 
			
		||||
     * Changes in the DRAFT state can be moved to:
 | 
			
		||||
     * <ul>
 | 
			
		||||
     * <li>{@link #NEW} - when the change is published, it becomes a new change;
 | 
			
		||||
     * </ul>
 | 
			
		||||
     */
 | 
			
		||||
    DRAFT(STATUS_DRAFT),
 | 
			
		||||
 | 
			
		||||
    /**
 | 
			
		||||
     * Change is closed, and submitted to its destination branch.
 | 
			
		||||
     *
 | 
			
		||||
@@ -494,4 +514,4 @@ public final class Change {
 | 
			
		||||
  public void setMergeable(boolean mergeable) {
 | 
			
		||||
    this.mergeable = mergeable;
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
@@ -49,6 +49,7 @@ import com.google.gerrit.server.project.ProjectCache;
 | 
			
		||||
import com.google.gerrit.server.project.ProjectControl;
 | 
			
		||||
import com.google.gerrit.server.project.ProjectState;
 | 
			
		||||
import com.google.gerrit.server.project.RefControl;
 | 
			
		||||
import com.google.gerrit.server.util.MagicBranch;
 | 
			
		||||
import com.google.gwtorm.client.AtomicUpdate;
 | 
			
		||||
import com.google.gwtorm.client.OrmException;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
@@ -75,8 +76,8 @@ import org.eclipse.jgit.revwalk.filter.RevFilter;
 | 
			
		||||
import org.eclipse.jgit.transport.PostReceiveHook;
 | 
			
		||||
import org.eclipse.jgit.transport.PreReceiveHook;
 | 
			
		||||
import org.eclipse.jgit.transport.ReceiveCommand;
 | 
			
		||||
import org.eclipse.jgit.transport.ReceivePack;
 | 
			
		||||
import org.eclipse.jgit.transport.ReceiveCommand.Result;
 | 
			
		||||
import org.eclipse.jgit.transport.ReceivePack;
 | 
			
		||||
import org.slf4j.Logger;
 | 
			
		||||
import org.slf4j.LoggerFactory;
 | 
			
		||||
 | 
			
		||||
@@ -100,7 +101,6 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
 | 
			
		||||
  private static final Logger log =
 | 
			
		||||
      LoggerFactory.getLogger(ReceiveCommits.class);
 | 
			
		||||
 | 
			
		||||
  public static final String NEW_CHANGE = "refs/for/";
 | 
			
		||||
  private static final Pattern NEW_PATCHSET =
 | 
			
		||||
      Pattern.compile("^refs/changes/(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/new)?$");
 | 
			
		||||
 | 
			
		||||
@@ -305,29 +305,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
 | 
			
		||||
      return result;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    // Don't permit receive-pack to be executed if a refs/for/branch_name
 | 
			
		||||
    // reference exists in the destination repository. These block the
 | 
			
		||||
    // client from being able to even send us a pack file, as it is very
 | 
			
		||||
    // unlikely the user passed the --force flag and the new commit is
 | 
			
		||||
    // probably not going to fast-forward the branch.
 | 
			
		||||
    //
 | 
			
		||||
    Map<String, Ref> blockingFors;
 | 
			
		||||
    try {
 | 
			
		||||
      blockingFors = repo.getRefDatabase().getRefs("refs/for/");
 | 
			
		||||
    } catch (IOException err) {
 | 
			
		||||
      String projName = project.getName();
 | 
			
		||||
      log.warn("Cannot scan refs in '" + projName + "'", err);
 | 
			
		||||
      return new Capable("Server process cannot read '" + projName + "'");
 | 
			
		||||
    }
 | 
			
		||||
    if (!blockingFors.isEmpty()) {
 | 
			
		||||
      String projName = project.getName();
 | 
			
		||||
      log.error("Repository '" + projName
 | 
			
		||||
          + "' needs the following refs removed to receive changes: "
 | 
			
		||||
          + blockingFors.keySet());
 | 
			
		||||
      return new Capable("One or more refs/for/ names blocks change upload");
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    return Capable.OK;
 | 
			
		||||
    return MagicBranch.checkMagicBranchRefs(repo, project);
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  @Override
 | 
			
		||||
@@ -377,7 +355,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
 | 
			
		||||
              ps.getProject().getDescription());
 | 
			
		||||
        }
 | 
			
		||||
 | 
			
		||||
        if (!c.getRefName().startsWith(NEW_CHANGE)) {
 | 
			
		||||
        if (!MagicBranch.isMagicBranch(c.getRefName())) {
 | 
			
		||||
          // We only schedule direct refs updates for replication.
 | 
			
		||||
          // Change refs are scheduled when they are created.
 | 
			
		||||
          //
 | 
			
		||||
@@ -423,7 +401,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
 | 
			
		||||
        continue;
 | 
			
		||||
      }
 | 
			
		||||
 | 
			
		||||
      if (cmd.getRefName().startsWith(NEW_CHANGE)) {
 | 
			
		||||
      if (MagicBranch.isMagicBranch(cmd.getRefName())) {
 | 
			
		||||
        parseNewChangeCommand(cmd);
 | 
			
		||||
        continue;
 | 
			
		||||
      }
 | 
			
		||||
@@ -612,7 +590,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    newChange = cmd;
 | 
			
		||||
    String destBranchName = cmd.getRefName().substring(NEW_CHANGE.length());
 | 
			
		||||
    String destBranchName = MagicBranch.getDestBranchName(cmd.getRefName());
 | 
			
		||||
    if (!destBranchName.startsWith(Constants.R_REFS)) {
 | 
			
		||||
      destBranchName = Constants.R_HEADS + destBranchName;
 | 
			
		||||
    }
 | 
			
		||||
@@ -1557,7 +1535,7 @@ public class ReceiveCommits implements PreReceiveHook, PostReceiveHook {
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    final List<String> idList = c.getFooterLines(CHANGE_ID);
 | 
			
		||||
    if ((cmd.getRefName().startsWith(NEW_CHANGE) || NEW_PATCHSET.matcher(cmd.getRefName()).matches())) {
 | 
			
		||||
    if ((MagicBranch.isMagicBranch(cmd.getRefName()) || NEW_PATCHSET.matcher(cmd.getRefName()).matches())) {
 | 
			
		||||
      if (idList.isEmpty()) {
 | 
			
		||||
        if (project.isRequireChangeID()) {
 | 
			
		||||
          String errMsg = "missing Change-Id in commit message";
 | 
			
		||||
 
 | 
			
		||||
@@ -0,0 +1,106 @@
 | 
			
		||||
// Copyright (C) 2011 The Android Open Source Project
 | 
			
		||||
//
 | 
			
		||||
// Licensed under the Apache License, Version 2.0 (the "License");
 | 
			
		||||
// you may not use this file except in compliance with the License.
 | 
			
		||||
// You may obtain a copy of the License at
 | 
			
		||||
//
 | 
			
		||||
// http://www.apache.org/licenses/LICENSE-2.0
 | 
			
		||||
//
 | 
			
		||||
// Unless required by applicable law or agreed to in writing, software
 | 
			
		||||
// distributed under the License is distributed on an "AS IS" BASIS,
 | 
			
		||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 | 
			
		||||
// See the License for the specific language governing permissions and
 | 
			
		||||
// limitations under the License.
 | 
			
		||||
 | 
			
		||||
package com.google.gerrit.server.util;
 | 
			
		||||
 | 
			
		||||
import com.google.gerrit.common.data.Capable;
 | 
			
		||||
import com.google.gerrit.reviewdb.Project;
 | 
			
		||||
 | 
			
		||||
import org.eclipse.jgit.lib.Ref;
 | 
			
		||||
import org.eclipse.jgit.lib.Repository;
 | 
			
		||||
import org.slf4j.Logger;
 | 
			
		||||
import org.slf4j.LoggerFactory;
 | 
			
		||||
 | 
			
		||||
import java.io.IOException;
 | 
			
		||||
import java.util.Map;
 | 
			
		||||
 | 
			
		||||
public final class MagicBranch {
 | 
			
		||||
  private static final Logger log =
 | 
			
		||||
    LoggerFactory.getLogger(MagicBranch.class);
 | 
			
		||||
 | 
			
		||||
  public static final String NEW_CHANGE = "refs/for/";
 | 
			
		||||
  public static final String NEW_DRAFT_CHANGE = "refs/draft/";
 | 
			
		||||
  public static final String NEW_PUBLISH_CHANGE = "refs/publish/";
 | 
			
		||||
 | 
			
		||||
  /** Extracts the destination from a ref name */
 | 
			
		||||
  public static String getDestBranchName(String refName) {
 | 
			
		||||
    String magicBranch = NEW_CHANGE;
 | 
			
		||||
    if (refName.startsWith(NEW_DRAFT_CHANGE)) {
 | 
			
		||||
      magicBranch = NEW_DRAFT_CHANGE;
 | 
			
		||||
    } else if (refName.startsWith(NEW_PUBLISH_CHANGE)) {
 | 
			
		||||
      magicBranch = NEW_PUBLISH_CHANGE;
 | 
			
		||||
    }
 | 
			
		||||
    return refName.substring(magicBranch.length());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /** Checks if the supplied ref name is a magic branch */
 | 
			
		||||
  public static boolean isMagicBranch(String refName) {
 | 
			
		||||
    if (refName.startsWith(NEW_DRAFT_CHANGE) ||
 | 
			
		||||
        refName.startsWith(NEW_PUBLISH_CHANGE) ||
 | 
			
		||||
        refName.startsWith(NEW_CHANGE)) {
 | 
			
		||||
      return true;
 | 
			
		||||
    }
 | 
			
		||||
    return false;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  /**
 | 
			
		||||
   * Checks if a (magic branch)/branch_name reference exists in the
 | 
			
		||||
   * destination repository and only returns Capable.OK if it does not match any.
 | 
			
		||||
   *
 | 
			
		||||
   * These block the client from being able to even send us a pack file, as it
 | 
			
		||||
   * is very unlikely the user passed the --force flag and the new commit is
 | 
			
		||||
   * probably not going to fast-forward the branch.
 | 
			
		||||
   */
 | 
			
		||||
  public static Capable checkMagicBranchRefs(Repository repo,
 | 
			
		||||
      Project project) {
 | 
			
		||||
    Capable result = checkMagicBranchRef(NEW_CHANGE, repo, project);
 | 
			
		||||
    if (result != Capable.OK) {
 | 
			
		||||
      return result;
 | 
			
		||||
    }
 | 
			
		||||
    result = checkMagicBranchRef(NEW_DRAFT_CHANGE, repo, project);
 | 
			
		||||
    if (result != Capable.OK) {
 | 
			
		||||
      return result;
 | 
			
		||||
    }
 | 
			
		||||
    result = checkMagicBranchRef(NEW_PUBLISH_CHANGE, repo, project);
 | 
			
		||||
    if (result != Capable.OK) {
 | 
			
		||||
      return result;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    return Capable.OK;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private static Capable checkMagicBranchRef(String branchName, Repository repo,
 | 
			
		||||
      Project project) {
 | 
			
		||||
    Map<String, Ref> blockingFors;
 | 
			
		||||
    try {
 | 
			
		||||
      blockingFors = repo.getRefDatabase().getRefs(branchName);
 | 
			
		||||
    } catch (IOException err) {
 | 
			
		||||
      String projName = project.getName();
 | 
			
		||||
      log.warn("Cannot scan refs in '" + projName + "'", err);
 | 
			
		||||
      return new Capable("Server process cannot read '" + projName + "'");
 | 
			
		||||
    }
 | 
			
		||||
    if (!blockingFors.isEmpty()) {
 | 
			
		||||
      String projName = project.getName();
 | 
			
		||||
      log.error("Repository '" + projName
 | 
			
		||||
          + "' needs the following refs removed to receive changes: "
 | 
			
		||||
          + blockingFors.keySet());
 | 
			
		||||
      return new Capable("One or more " + branchName + " names blocks change upload");
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    return Capable.OK;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private MagicBranch() {
 | 
			
		||||
  }
 | 
			
		||||
}
 | 
			
		||||
		Reference in New Issue
	
	Block a user