From e3f24aa1aa6eb389a0512670db0090cf03014506 Mon Sep 17 00:00:00 2001 From: "Paul J. Davis" Date: Fri, 7 Jun 2019 15:02:19 -0500 Subject: Fix revision generation on attachment upload When uploading an attachment we hadn't yet flushed data to FoundationDB which caused the md5 to be empty. The `new_revid` algorithm then declared that was because it was an old style attachment and thus our new revision would be a random number. This fix just flushes our attachments earlier in the process of updating a document. --- src/fabric/src/fabric2_db.erl | 103 +++++++++++++++++++++++++++-------------- src/fabric/src/fabric2_fdb.erl | 9 +--- 2 files changed, 70 insertions(+), 42 deletions(-) diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl index 02a18fa23..acd473f12 100644 --- a/src/fabric/src/fabric2_db.erl +++ b/src/fabric/src/fabric2_db.erl @@ -120,7 +120,7 @@ %% validate_dbname/1, %% make_doc/5, - new_revid/1 + new_revid/2 ]). @@ -604,9 +604,7 @@ read_attachment(Db, DocId, AttId) -> write_attachment(Db, DocId, Att) -> Data = couch_att:fetch(data, Att), - {ok, AttId} = fabric2_fdb:transactional(Db, fun(TxDb) -> - fabric2_fdb:write_attachment(TxDb, DocId, Data) - end), + {ok, AttId} = fabric2_fdb:write_attachment(Db, DocId, Data), couch_att:store(data, {loc, Db, DocId, AttId}, Att). @@ -630,33 +628,69 @@ fold_changes(Db, SinceSeq, UserFun, UserAcc, Options) -> end). -new_revid(Doc) -> +maybe_add_sys_db_callbacks(Db) -> + IsReplicatorDb = fabric2_util:dbname_ends_with(Db, <<"_replicator">>), + + CfgUsersSuffix = config:get("couchdb", "users_db_suffix", "_users"), + IsCfgUsersDb = fabric2_util:dbname_ends_with(Db, ?l2b(CfgUsersSuffix)), + IsGlobalUsersDb = fabric2_util:dbname_ends_with(Db, <<"_users">>), + IsUsersDb = IsCfgUsersDb orelse IsGlobalUsersDb, + + {BDU, ADR} = if + IsReplicatorDb -> + { + fun couch_replicator_docs:before_doc_update/3, + fun couch_replicator_docs:after_doc_read/2 + }; + IsUsersDb -> + { + fun fabric2_users_db:before_doc_update/3, + fun fabric2_users_db:after_doc_read/2 + }; + true -> + {undefined, undefined} + end, + + Db#{ + before_doc_update := BDU, + after_doc_read := ADR + }. + + +new_revid(Db, Doc) -> #doc{ + id = DocId, body = Body, revs = {OldStart, OldRevs}, atts = Atts, deleted = Deleted } = Doc, - DigestedAtts = lists:foldl(fun(Att, Acc) -> - [N, T, M] = couch_att:fetch([name, type, md5], Att), - case M == <<>> of - true -> Acc; - false -> [{N, T, M} | Acc] + {NewAtts, AttSigInfo} = lists:mapfoldl(fun(Att, Acc) -> + [Name, Type, Data, Md5] = couch_att:fetch([name, type, data, md5], Att), + case Data of + {loc, _, _, _} -> + {Att, [{Name, Type, Md5} | Acc]}; + _ -> + Att1 = couch_att:flush(Db, DocId, Att), + Att2 = couch_att:store(revpos, OldStart + 1, Att1), + {Att2, [{Name, Type, couch_att:fetch(md5, Att2)} | Acc]} end end, [], Atts), - Rev = case DigestedAtts of - Atts2 when length(Atts) =/= length(Atts2) -> - % We must have old style non-md5 attachments - list_to_binary(integer_to_list(couch_util:rand32())); - Atts2 -> + Rev = case length(Atts) == length(AttSigInfo) of + true -> OldRev = case OldRevs of [] -> 0; [OldRev0 | _] -> OldRev0 end, - SigTerm = [Deleted, OldStart, OldRev, Body, Atts2], - couch_hash:md5_hash(term_to_binary(SigTerm, [{minor_version, 1}])) + SigTerm = [Deleted, OldStart, OldRev, Body, AttSigInfo], + couch_hash:md5_hash(term_to_binary(SigTerm, [{minor_version, 1}])); + false -> + erlang:error(missing_att_info) end, - Doc#doc{revs = {OldStart + 1, [Rev | OldRevs]}}. + Doc#doc{ + revs = {OldStart + 1, [Rev | OldRevs]}, + atts = NewAtts + }. maybe_set_user_ctx(Db, Options) -> @@ -970,12 +1004,11 @@ update_doc_interactive(Db, Doc0, Future, _Options) -> % Validate the doc update and create the % new revinfo map Doc2 = prep_and_validate(Db, Doc1, Target), + #doc{ deleted = NewDeleted, revs = {NewRevPos, [NewRev | NewRevPath]} - } = Doc3 = new_revid(Doc2), - - Doc4 = update_attachment_revpos(Doc3), + } = Doc3 = new_revid(Db, Doc2), NewRevInfo = #{ winner => undefined, @@ -988,9 +1021,9 @@ update_doc_interactive(Db, Doc0, Future, _Options) -> % Gather the list of possible winnig revisions Possible = case Target == Winner of - true when not Doc4#doc.deleted -> + true when not Doc3#doc.deleted -> [NewRevInfo]; - true when Doc4#doc.deleted -> + true when Doc3#doc.deleted -> case SecondPlace of #{} -> [NewRevInfo, SecondPlace]; not_found -> [NewRevInfo] @@ -1015,7 +1048,7 @@ update_doc_interactive(Db, Doc0, Future, _Options) -> ok = fabric2_fdb:write_doc( Db, - Doc4, + Doc3, NewWinner, Winner, ToUpdate, @@ -1076,6 +1109,7 @@ update_doc_replicated(Db, Doc0, _Options) -> LeafPath = get_leaf_path(RevPos, Rev, AllLeafsFull), PrevRevInfo = find_prev_revinfo(RevPos, LeafPath), Doc2 = prep_and_validate(Db, Doc1, PrevRevInfo), + Doc3 = flush_doc_atts(Db, Doc2), % Possible winners are the previous winner and % the new DocRevInfo @@ -1097,7 +1131,7 @@ update_doc_replicated(Db, Doc0, _Options) -> ok = fabric2_fdb:write_doc( Db, - Doc2, + Doc3, NewWinner, Winner, ToUpdate, @@ -1119,19 +1153,20 @@ update_local_doc(Db, Doc0, _Options) -> {ok, {0, integer_to_binary(Rev)}}. -update_attachment_revpos(#doc{revs = {RevPos, _Revs}, atts = Atts0} = Doc) -> - Atts = lists:map(fun(Att) -> +flush_doc_atts(Db, Doc) -> + #doc{ + id = DocId, + atts = Atts + } = Doc, + NewAtts = lists:map(fun(Att) -> case couch_att:fetch(data, Att) of - {loc, _Db, _DocId, _AttId} -> - % Attachment was already on disk + {loc, _, _, _} -> Att; _ -> - % We will write this attachment with this update - % so mark it with the RevPos that will be written - couch_att:store(revpos, RevPos, Att) + couch_att:flush(Db, DocId, Att) end - end, Atts0), - Doc#doc{atts = Atts}. + end, Atts), + Doc#doc{atts = NewAtts}. get_winning_rev_futures(Db, Docs) -> diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl index 0a4f2981b..788bbc62a 100644 --- a/src/fabric/src/fabric2_fdb.erl +++ b/src/fabric/src/fabric2_fdb.erl @@ -924,7 +924,7 @@ doc_to_fdb(Db, #doc{} = Doc) -> body = Body, atts = Atts, deleted = Deleted - } = doc_flush_atts(Db, Doc), + } = Doc, Key = erlfdb_tuple:pack({?DB_DOCS, Id, Start, Rev}, DbPrefix), Val = {Body, Atts, Deleted}, @@ -977,13 +977,6 @@ fdb_to_local_doc(_Db, _DocId, not_found) -> {not_found, missing}. -doc_flush_atts(Db, Doc) -> - Atts = lists:map(fun(Att) -> - couch_att:flush(Db, Doc#doc.id, Att) - end, Doc#doc.atts), - Doc#doc{atts = Atts}. - - chunkify_attachment(Data) -> case Data of <<>> -> -- cgit v1.2.1