commit d64425d0015df6f403b616d543bf0672be559bbe
author David Ostrovsky <david@ostrovsky.org> Mon Sep 17 23:12:06 2018 +0200
committer David Ostrovsky <david@ostrovsky.org> Fri Sep 21 08:38:35 2018 +0200
tree 03331f9a9935aebccd0e6db0ac72c56897eb2702
parent f323dd26ab69e96198ea4a4dffa52c49b59dbdbc

Revert "AsyncReceiveCommits: Move ReceiveCommits into Worker"

This reverts commit 095467aa8fa68a19749062517893601ef98072f9.

The reason for revert: pre-receive hook chain in ReceivePack must be
established before PreReceiveHook#onPreReceive() actually gets called
(in ctor of ARC) and not during the PreReceiveHook#onPreReceive() call.

rc.init() initializes the hooks and establishes the hook chain in
receive pack instance (eventually):

  void init() {
    for (ReceivePackInitializer i : initializers) {
      i.init(projectState.getNameKey(), rp);
    }
  }

SignedPushModule is one from the initializers:

  List<PreReceiveHook> hooks = new ArrayList<>(3);
  if (ps.isRequireSignedPush()) {
    hooks.add(SignedPushPreReceiveHook.Required.INSTANCE);
  }
  hooks.add(hook);
  hooks.add(rp.getPreReceiveHook());
  rp.setPreReceiveHook(PreReceiveHookChain.newChain(hooks));

However, after the move of rc.init() from ARC ctor to Worker ctor that
is called from onPreReceive(), the rc.init() gets actually called from
ReceivePack#service line 280:

  preReceive.onPreReceive(this, filterCommands(Result.NOT_ATTEMPTED));

The whole stack trace:

  AsyncReceiveCommits.onPreReceive(ReceivePack, Collection<ReceiveCommand>) line: 278
	ReceivePack.service() line: 280
	ReceivePack.receive(InputStream, OutputStream, OutputStream) line: 221
	Receive.runImpl() line: 104
	Receive(AbstractGitCommand).service() line: 97
	AbstractGitCommand.access$0(AbstractGitCommand) line: 86
	AbstractGitCommand$1.run() line: 63
	BaseCommand$TaskThunk.run() line: 456
	LoggingContextAwareRunnable.run() line: 83

So that the pre-receive chain is mutated as a side effect of
ReceiveCommits#init() in the preReceive instance variable of ReceivePack
instance but only during the invocation of the very first hook in this
class and thus this mutation became a no-op. As the consequence other
pre-receive hooks don't get called.

New acceptance test is added in follow-up change to avoid such
regression in future.

Test Plan:

1. Set up signed push for gerrit site
2. Enable signed push for project foo
3. Enable require signed push for project foo
4. Push to project foo without passing in --signed option should fail:

  $ git push gerrita HEAD:refs/for/master
  Counting objects: 3, done.
  Writing objects: 100% (3/3), 317 bytes | 317.00 KiB/s, done.
  Total 3 (delta 0), reused 0 (delta 0)
  remote: ERROR: Signed push is required

Bug: Issue 7750
Change-Id: Ia63ddb0554542a77d6c28b5d9e6093a0bbd56a0e