UNLOCKCONTENT is not a top-level request

proxyRequest was treating UNLOCKCONTENT as a separate request.
That made it possible for there to be two different connections to the
proxied remote, with LOCKCONTENT being sent to one, and UNLOCKCONTENT
to the other one. A protocol error.

git-annex testremote now passes against a http proxied remote.

Annex/Cluster.hs

@@ -120,7 +120,6 @@ clusterProxySelector clusteruuid protocolversion (Bypass bypass) = do
 		-- instead it can be locked on individual nodes that are
 		-- proxied to the client.
 		, proxyLOCKCONTENT = const (pure Nothing)
-		, proxyUNLOCKCONTENT = pure Nothing
 		}
 	return (proxyselector, closenodes)
   where

P2P/Proxy.hs

@@ -79,7 +79,6 @@ closeRemoteSide remoteside =
 data ProxySelector = ProxySelector
 	{ proxyCHECKPRESENT :: Key -> Annex (Maybe RemoteSide)
 	, proxyLOCKCONTENT :: Key -> Annex (Maybe RemoteSide)
-	, proxyUNLOCKCONTENT :: Annex (Maybe RemoteSide)
 	, proxyREMOVE :: Key -> Annex [RemoteSide]
 	-- ^ remove from all of these remotes
 	, proxyGETTIMESTAMP :: Annex [RemoteSide]
@@ -94,7 +93,6 @@ singleProxySelector :: RemoteSide -> ProxySelector
 singleProxySelector r = ProxySelector
 	{ proxyCHECKPRESENT = const (pure (Just r))
 	, proxyLOCKCONTENT = const (pure (Just r))
-	, proxyUNLOCKCONTENT = pure (Just r)
 	, proxyREMOVE = const (pure [r])
 	, proxyGETTIMESTAMP = pure [r]
 	, proxyGET = const (pure (Just r))
@@ -261,16 +259,10 @@ proxyRequest proxydone proxyparams requestcomplete requestmessage protoerrhandle
 					client $ net $ sendMessage FAILURE
 		LOCKCONTENT k -> proxyLOCKCONTENT (proxySelector proxyparams) k >>= \case
 			Just remoteside -> 
-				proxyresponse remoteside requestmessage 
+				handleLOCKCONTENT remoteside requestmessage
-					(const requestcomplete)
 			Nothing ->
 				protoerrhandler requestcomplete $
 					client $ net $ sendMessage FAILURE
-		UNLOCKCONTENT -> proxyUNLOCKCONTENT (proxySelector proxyparams) >>= \case
-			Just remoteside ->
-				proxynoresponse remoteside requestmessage
-					requestcomplete
-			Nothing -> requestcomplete ()
 		REMOVE k -> do
 			remotesides <- proxyREMOVE (proxySelector proxyparams) k
 			servermodechecker checkREMOVEServerMode $
@@ -312,6 +304,7 @@ proxyRequest proxydone proxyparams requestcomplete requestmessage protoerrhandle
 		FAILURE_PLUS _ -> protoerr
 		DATA _ -> protoerr
 		VALIDITY _ -> protoerr
+		UNLOCKCONTENT -> protoerr
 		-- If the client errors out, give up.
 		ERROR msg -> giveup $ "client error: " ++ msg
 		-- Messages that only the server should send.
@@ -344,11 +337,6 @@ proxyRequest proxydone proxyparams requestcomplete requestmessage protoerrhandle
 			protoerrhandler (a resp) $
 				client $ net $ sendMessage resp
 	
-	-- Send a message to the remote, that it will not respond to.
-	proxynoresponse remoteside message a =
-		protoerrhandler a $
-			runRemoteSide remoteside $ net $ sendMessage message
-	
 	-- Send a message to the endpoint and get back its response.
 	getresponse endpoint message handleresp =
 		protoerrhandler (withresp handleresp) $ 
@@ -370,8 +358,16 @@ proxyRequest proxydone proxyparams requestcomplete requestmessage protoerrhandle
 					to $ net $ sendMessage message
 	
 	protoerr = do
-		_ <- client $ net $ sendMessage (ERROR "protocol error X")
+		_ <- client $ net $ sendMessage (ERROR "protocol error")
-		giveup "protocol error M"
+		giveup "protocol error"
+	
+	handleLOCKCONTENT remoteside msg =
+		proxyresponse remoteside msg $ \r () -> case r of
+			SUCCESS -> relayonemessage client
+				(runRemoteSide remoteside)
+				(const requestcomplete)
+			FAILURE -> requestcomplete ()
+			_ -> requestcomplete ()
 	
 	-- When there is a single remote, reply with its timestamp,
 	-- to avoid needing timestamp translation.

doc/design/p2p_protocol.mdwn

@@ -114,8 +114,10 @@ the client sends:
 	LOCKCONTENT Key
 
 The server responds with either SUCCESS or FAILURE.
-The former indicates the content is locked. It will remain
+The former indicates the content is locked.
-locked until the client sends its next message, which must be:
+
+After SUCCESS, the content will remain locked until the
+client sends its next message, which must be:
 
 	UNLOCKCONTENT Key
 

doc/todo/git-annex_proxies.mdwn

@@ -28,20 +28,6 @@ Planned schedule of work:
 
 ## work notes
 
-* This against a http proxied remote leads to a protocol error:
-
-    git-annex move foo --to origin-c
-    git-annex get foo --from origin-c
-
-    ERROR expected UNLOCKCONTENT
-
-  May need to run the commands a few times before it happens.
-
-  I think it's because proxyRequest treats LOCKCONTENT as a single
-  command+reponse, with UNLOCKCONTENT separately. So it's possible for
-  there to be two different connections to the proxied remote,
-  with LOCKCONTENT being sent to one, and UNLOCKCONTENT to the other one.
-
 * test http server proxying with special remotes
 
 * Make http server support clusters.