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

@@ -9,5 +9,4 @@ erl_crash.dump
 edoc
 
 # PB artifacts for Erlang
-include/*.hrl
+include/machi_pb.hrl
-src/machi_pb_messages.erl

plugins/riak_pb_msgcodegen.erl

@@ -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).

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 {
+enum MpbGeneralStatusCode {
-     BAD_ARG = 0;
+     OK = 0;
-     WEDGED = 1;
+     BAD_ARG = 1;
-     BAD_CHECKSUM = 2;
+     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;
+// request & response wrapper
-    required bytes password = 2;
+//
+//////////////////////////////////////////
+
+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 MpbResponse_v1 {
-message MpbDummyAuthReq {
+    // TODO: If we wish to support pipelined requests sometime in the
-    required bytes user = 1;
+    // future, this is the placeholder to do it.
-    required bytes password = 2;
+    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;
+}

src/machi_pb_messages.csv

@@ -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