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.

diff --git a/Annex/Cluster.hs b/Annex/Cluster.hs
index e43cc737bd435bdb3b90da6043172b7f30455dc7..db98c7a508c035344a8f5abaa8fccfbc60fa2468 100644 (file)
--- a/Annex/Cluster.hs
+++ b/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

diff --git a/P2P/Proxy.hs b/P2P/Proxy.hs
index 765664ef48c836368083cc32fabb4e70b532c8d0..5e5835b418248347dc3c65f43851b0dd82c08610 100644 (file)
--- a/P2P/Proxy.hs
+++ b/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 
-                                       (const requestcomplete)
+                               handleLOCKCONTENT remoteside requestmessage
                        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")
-               giveup "protocol error M"
+               _ <- client $ net $ sendMessage (ERROR "protocol error")
+               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.

diff --git a/doc/design/p2p_protocol.mdwn b/doc/design/p2p_protocol.mdwn
index 5e1629957ed67631f28d333b871fe3536b84aad8..ef4b4065837eb29b9406a418893d92780e961fad 100644 (file)
--- a/doc/design/p2p_protocol.mdwn
+++ b/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
-locked until the client sends its next message, which must be:
+The former indicates the content is locked.
+
+After SUCCESS, the content will remain locked until the
+client sends its next message, which must be:
 
        UNLOCKCONTENT Key
 

diff --git a/doc/todo/git-annex_proxies.mdwn b/doc/todo/git-annex_proxies.mdwn
index 7e2eda6be04ffe8db1b5c96dd71b03ad9a7d16f4..9726837adc25966563dcb63d038ad64711ecdad1 100644 (file)
--- a/doc/todo/git-annex_proxies.mdwn
+++ b/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.