[GitHub] [couchdb] iilyak commented on a change in pull request #3686: Fix response code for existing att and wrong rev

[GitBox <gi...@apache.org>]

iilyak commented on a change in pull request #3686:
URL: https://github.com/apache/couchdb/pull/3686#discussion_r680184261



##########
File path: src/chttpd/src/chttpd_db.erl
##########
@@ -1442,120 +1442,115 @@ couch_doc_open(Db, DocId, Rev, Options0) ->
       end
   end.
 
-get_attachment(Req, Db, DocId, FileName) ->
-    % Check if attachment exists
-    #doc_query_args{
-        rev=Rev,
-        options=Options
-    } = parse_doc_query(Req),
-    #doc{
-        atts=Atts
-    } = Doc = couch_doc_open(Db, DocId, Rev, Options),
+
+get_existing_attachment(Atts, FileName) ->
+    % Check if attachment exists, if not throw not_found
     case [A || A <- Atts, couch_att:fetch(name, A) == FileName] of
-        [] ->
-            {error, missing};
-        [Att] ->
-            {ok, Doc, Att}
+        [] -> throw({not_found, "Document is missing attachment"});
+        [Att] -> Att
     end.
 
 % Attachment request handlers
 
 db_attachment_req(#httpd{method='GET',mochi_req=MochiReq}=Req, Db, DocId, FileNameParts) ->
     FileName = list_to_binary(mochiweb_util:join(lists:map(fun binary_to_list/1,
         FileNameParts),"/")),
-    case get_attachment(Req, Db, DocId, FileName) of
-    {error, missing} ->
-        throw({not_found, "Document is missing attachment"});
-    {ok, Doc, Att} ->
-        [Type, Enc, DiskLen, AttLen, Md5] = couch_att:fetch([type, encoding, disk_len, att_len, md5], Att),
-        Refs = monitor_attachments(Att),
-        try
-        Etag = case Md5 of
-            <<>> -> chttpd:doc_etag(Doc);
-            _ -> "\"" ++ ?b2l(base64:encode(Md5)) ++ "\""
-        end,
-        ReqAcceptsAttEnc = lists:member(
-           atom_to_list(Enc),
-           couch_httpd:accepted_encodings(Req)
-        ),
-        Headers = [
-            {"ETag", Etag},
-            {"Cache-Control", "must-revalidate"},
-            {"Content-Type", binary_to_list(Type)}
-        ] ++ case ReqAcceptsAttEnc of
-        true when Enc =/= identity ->
-            % RFC 2616 says that the 'identify' encoding should not be used in
-            % the Content-Encoding header
-            [{"Content-Encoding", atom_to_list(Enc)}];
+    #doc_query_args{
+        rev=Rev,
+        options=Options
+    } = parse_doc_query(Req),
+    #doc{
+        atts=Atts
+    } = Doc = couch_doc_open(Db, DocId, Rev, Options),
+    Att = get_existing_attachment(Atts, FileName),
+    [Type, Enc, DiskLen, AttLen, Md5] = couch_att:fetch([type, encoding, disk_len, att_len, md5], Att),
+    Refs = monitor_attachments(Att),
+    try
+    Etag = case Md5 of
+        <<>> -> chttpd:doc_etag(Doc);
+        _ -> "\"" ++ ?b2l(base64:encode(Md5)) ++ "\""
+    end,
+    ReqAcceptsAttEnc = lists:member(
+       atom_to_list(Enc),
+       couch_httpd:accepted_encodings(Req)
+    ),
+    Headers = [
+        {"ETag", Etag},
+        {"Cache-Control", "must-revalidate"},
+        {"Content-Type", binary_to_list(Type)}
+    ] ++ case ReqAcceptsAttEnc of

Review comment:
       this a really complex condition to understand. It could possibly be expressed as few clauses under `case {ReqAcceptsAttEnc, Enc} of`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org