From a82bd68f3c2a2af4f12c7fe80e73076565466a6e Mon Sep 17 00:00:00 2001 From: Scott Lystig Fritchie Date: Fri, 19 Jun 2015 12:28:31 +0900 Subject: [PATCH] Overhaul the 0.1 PB definition. Again. Many thanks to @seancribbs for a suggestion to avoid the PB design mistake/feature of the original Riak KV PB API. --- .gitignore | 3 +- plugins/riak_pb_msgcodegen.erl | 148 --------------------------------- src/machi.proto | 100 +++++++++++++++++----- src/machi_pb_messages.csv | 43 ---------- 4 files changed, 78 insertions(+), 216 deletions(-) delete mode 100644 plugins/riak_pb_msgcodegen.erl delete mode 100644 src/machi_pb_messages.csv diff --git a/.gitignore b/.gitignore index c88c93c..5243bad 100644 --- a/.gitignore +++ b/.gitignore @@ -9,5 +9,4 @@ erl_crash.dump edoc # PB artifacts for Erlang -include/*.hrl -src/machi_pb_messages.erl +include/machi_pb.hrl diff --git a/plugins/riak_pb_msgcodegen.erl b/plugins/riak_pb_msgcodegen.erl deleted file mode 100644 index a688f16..0000000 --- a/plugins/riak_pb_msgcodegen.erl +++ /dev/null @@ -1,148 +0,0 @@ -%% @doc Generates a codec-mapping module from a CSV mapping of message -%% codes to messages in .proto files. --module(riak_pb_msgcodegen). --export([preprocess/2, - clean/2]). - -%% -include_lib("rebar/include/rebar.hrl"). --define(FAIL, rebar_utils:abort()). --define(ABORT(Str, Args), rebar_utils:abort(Str, Args)). - --define(CONSOLE(Str, Args), io:format(Str, Args)). - --define(DEBUG(Str, Args), rebar_log:log(debug, Str, Args)). --define(INFO(Str, Args), rebar_log:log(info, Str, Args)). --define(WARN(Str, Args), rebar_log:log(warn, Str, Args)). --define(ERROR(Str, Args), rebar_log:log(error, Str, Args)). - --define(FMT(Str, Args), lists:flatten(io_lib:format(Str, Args))). - --define(MODULE_COMMENTS(CSV), - ["%% @doc This module contains message code mappings generated from\n%% ", - CSV,". DO NOT EDIT OR COMMIT THIS FILE!\n"]). - -%% =================================================================== -%% Public API -%% =================================================================== -preprocess(Config, _AppFile) -> - case rebar_config:get(Config, current_command, undefined) of - 'compile' -> - case rebar_utils:find_files("src", ".*\\.csv") of - [] -> - ok; - FoundFiles -> - Targets = [{CSV, fq_erl_file(CSV)} || CSV <- FoundFiles ], - generate_each(Config, Targets) - end; - _Else -> ok - end, - {ok, Config, []}. - -clean(_Config, _AppFile) -> - CSVs = rebar_utils:find_files("src", ".*\\.csv"), - ErlFiles = [fq_erl_file(CSV) || CSV <- CSVs], - case ErlFiles of - [] -> ok; - _ -> delete_each(ErlFiles) - end. - -%% =================================================================== -%% Internal functions -%% =================================================================== - -generate_each(_Config, []) -> - ok; -generate_each(Config, [{CSV, Erl}|Rest]) -> - case is_modified(CSV, Erl) of - false -> - ok; - true -> - Tuples = load_csv(CSV), - Module = generate_module(mod_name(CSV), Tuples), - Formatted = erl_prettypr:format(Module), - ok = file:write_file(Erl, [?MODULE_COMMENTS(CSV), Formatted]), - ?CONSOLE("Generated ~s~n", [Erl]) - end, - generate_each(Config, Rest). - -is_modified(CSV, Erl) -> - not filelib:is_regular(Erl) orelse - filelib:last_modified(CSV) > filelib:last_modified(Erl). - -mod_name(SourceFile) -> - filename:basename(SourceFile, ".csv"). - -fq_erl_file(SourceFile) -> - filename:join(["src", erl_file(SourceFile)]). - -erl_file(SourceFile) -> - mod_name(SourceFile) ++ ".erl". - -load_csv(SourceFile) -> - {ok, Bin} = file:read_file(SourceFile), - csv_to_tuples(unicode:characters_to_list(Bin, latin1)). - -csv_to_tuples(String) -> - Lines = string:tokens(String, [$\r,$\n]), - [ begin - [Code, Message, Proto] = string:tokens(Line, ","), - {list_to_integer(Code), string:to_lower(Message), Proto ++ "_pb"} - end - || Line <- Lines, length(Line) > 0 andalso hd(Line) /= $#]. - -generate_module(Name, Tuples) -> - %% TODO: Add generated doc comment at the top - Mod = erl_syntax:attribute(erl_syntax:atom(module), - [erl_syntax:atom(Name)]), - ExportsList = [ - erl_syntax:arity_qualifier(erl_syntax:atom(Fun), erl_syntax:integer(1)) - || Fun <- [msg_type, msg_code, decoder_for] ], - - Exports = erl_syntax:attribute(erl_syntax:atom(export), - [erl_syntax:list(ExportsList)]), - - Clauses = generate_msg_type(Tuples) ++ - generate_msg_code(Tuples) ++ - generate_decoder_for(Tuples), - - erl_syntax:form_list([Mod, Exports|Clauses]). - -generate_decoder_for(Tuples) -> - Spec = erl_syntax:text("-spec decoder_for(non_neg_integer()) -> module().\n"), - Name = erl_syntax:atom(decoder_for), - Clauses = [ - erl_syntax:clause([erl_syntax:integer(Code)], - none, - [erl_syntax:atom(Mod)]) - || {Code, _, Mod} <- Tuples ], - [ Spec, erl_syntax:function(Name, Clauses) ]. - -generate_msg_code(Tuples) -> - Spec = erl_syntax:text("-spec msg_code(atom()) -> non_neg_integer()."), - Name = erl_syntax:atom(msg_code), - Clauses = [ - erl_syntax:clause([erl_syntax:atom(Msg)], none, [erl_syntax:integer(Code)]) - || {Code, Msg, _} <- Tuples ], - [ Spec, erl_syntax:function(Name, Clauses) ]. - -generate_msg_type(Tuples) -> - Spec = erl_syntax:text("-spec msg_type(non_neg_integer()) -> atom()."), - Name = erl_syntax:atom(msg_type), - Clauses = [ - erl_syntax:clause([erl_syntax:integer(Code)], none, [erl_syntax:atom(Msg)]) - || {Code, Msg, _} <- Tuples ], - CatchAll = erl_syntax:clause([erl_syntax:underscore()], none, [erl_syntax:atom(undefined)]), - [ Spec, erl_syntax:function(Name, Clauses ++ [CatchAll]) ]. - -delete_each([]) -> - ok; -delete_each([File | Rest]) -> - case file:delete(File) of - ok -> - ok; - {error, enoent} -> - ok; - {error, Reason} -> - ?ERROR("Failed to delete ~s: ~p\n", [File, Reason]) - end, - delete_each(Rest). diff --git a/src/machi.proto b/src/machi.proto index 57fd1f1..05f8a0e 100644 --- a/src/machi.proto +++ b/src/machi.proto @@ -1,8 +1,8 @@ /* ------------------------------------------------------------------- ** -** machi.proto: Protocol buffers for Machi +** machi.proto: Protocol Buffers definition for Machi ** -** Copyright (c) 2007-2015 Basho Technologies, Inc. All Rights Reserved. +** Copyright (c) 2014-2015 Basho Technologies, Inc. All Rights Reserved. ** ** This file is provided to you under the Apache License, ** Version 2.0 (the "License"); you may not use this file @@ -29,15 +29,17 @@ option java_package = "com.basho.machi.protobuf"; option java_outer_classname = "MachiPB"; -enum MpbStatusCode { - OK = 0; - ERROR = 1; -} +////////////////////////////////////////// +// +// enums +// +////////////////////////////////////////// -enum MpbErrorGeneral { - BAD_ARG = 0; - WEDGED = 1; - BAD_CHECKSUM = 2; +enum MpbGeneralStatusCode { + OK = 0; + BAD_ARG = 1; + WEDGED = 2; + BAD_CHECKSUM = 3; } // Must match with machi.hrl's values! @@ -48,6 +50,12 @@ enum MpbCSumType { CSUM_TAG_SERVER_REGEN = 3; } +////////////////////////////////////////// +// +// basic data types +// +////////////////////////////////////////// + // chunk_pos() type message MpbChunkPos { required uint64 offset = 1; @@ -73,7 +81,13 @@ message MpbErrorResp { required uint32 errcode = 2; } -// ping() request +////////////////////////////////////////// +// +// requests & responses +// +////////////////////////////////////////// + +// ping() request & response message MpbEchoReq { optional string message = 1; @@ -83,7 +97,20 @@ message MpbEchoResp { optional string message = 1; } -// append_chunk() request +// Authentication request & response + +message MpbAuthReq { + required bytes user = 1; + required bytes password = 2; +} + +message MpbAuthResp { + required uint32 code = 1; + // TODO: not implemented yet +} + +// append_chunk() request & response + message MpbAppendChunkReq { required string prefix = 1; optional bytes placement_key = 2; @@ -91,22 +118,49 @@ message MpbAppendChunkReq { optional uint32 chunk_extra = 4; } -// append_chunk() response message MpbAppendChunkResp { - required MpbStatusCode status = 1; + required MpbGeneralStatusCode status = 1; optional MpbChunkPos chunk_pos = 2; optional MbpGeneralError = 3; } -// Authentication request -message MpbAuthReq { - required bytes user = 1; - required bytes password = 2; +////////////////////////////////////////// +// +// request & response wrapper +// +////////////////////////////////////////// + +message MpbRequest_v1 { + // TODO: If we wish to support pipelined requests sometime in the + // future, this is the placeholder to do it. + required bytes req_id = 1; + + // The client should only define one request message. If the client + // includes multiple requests here, the server may pick/choose an + // arbitrary one. + // NOTE: The erlang protobuffs compiler doesn't support 'oneof'. + // But 'oneof' appears to be a very tiny memory optimization + // that not all languages might care about? (Erlang doesn't) + optional MpbEchoReq echo = 10; + optional MpbAuthReq auth = 11; + optional MpbAppendChunkReq append_chunk = 12; } -// Dummy authentication request, not used! Included for demo purposes only. -message MpbDummyAuthReq { - required bytes user = 1; - required bytes password = 2; -} +message MpbResponse_v1 { + // TODO: If we wish to support pipelined requests sometime in the + // future, this is the placeholder to do it. + required bytes req_id = 1; + // The server will define only one of the optional responses below. + + // Generic error response, typically used when something quite + // bad/unexpected happened within the server. + // Clients should always check this response and, if defined, + // ignroe any request-specific response at codes 10+. + optional MpbErrorResp = 2; + + // Specific responses. + optional MpbEchoResp echo = 10; + optional MpbAuthResp auth = 11; + optional MpbAppendChunkResp append_chunk = 12; +} diff --git a/src/machi_pb_messages.csv b/src/machi_pb_messages.csv deleted file mode 100644 index 28d4093..0000000 --- a/src/machi_pb_messages.csv +++ /dev/null @@ -1,43 +0,0 @@ -# Comments begin with pound/hash/octothorpe -# Blank lines are permitted, but the line must be truly blank (no -# hidden whitespace)! - -# https://developers.google.com/protocol-buffers/docs/proto3 -# -# The machi.proto file defines the message types, but it doesn't -# define how the wire format will distinguish between different kinds -# of messages. For example: -# -# 38> Dummy = list_to_binary(machi_pb:encode_mpbdummyauthreq({mpbdummyauthreq, <<"dummy-u">>, <<"dummy-p">>})). -# <<10,7,100,117,109,109,121,45,117,18,7,100,117,109,109,121,45,112>> -# -# 39> machi_pb:decode_mpbauthreq(Dummy). -#{mpbauthreq,<<"dummy-u">>,<<"dummy-p">>} -# -# The above demonstrates that we cannot tell the difference between a -# MpbAuthReq message and a MpbDummyAuthReq message because their -# Protocol Buffers encodings are identical. -# -# Basho's Riak KV uses the following conventions to create a working -# message passing service. -# -# 1. Use the first 4 bytes to describe the length of the message that -# follows. This is analogous to Erlang/OTP's use of {packet,4} style -# PDU boundaries. This length includes the bytes from items #2 and #3 -# below. -# 2. Use the next one byte defined below to tag a particular Protocol Buffer -# message type. -# 3. The encoded Protocol Buffer message bytes follow. -# -# We will adopt the same convention for Machi's PB interface. - -0,MpbErrorResp,machi_pbundefined - -1,MpbEchoReq,machi_pbundefined -2,MpbEchoResp,machi_pbundefined - -3,MpbAuthReq,machi_pbundefined -4,MpbAuthResp,machi_pbundefined - -5,MpbAppendChunkReq,machi_pbundefined -6,MpbAppendChunkResp,machi_pbundefined