You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by iilyak <gi...@git.apache.org> on 2015/09/28 20:26:08 UTC

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

GitHub user iilyak opened a pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12

    Simplify couch epi

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/cloudant/couchdb-couch-epi simplify_couch_epi

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-couch-epi/pull/12.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #12
    
----
commit 5f3fe7a16382d371c214b9f4376c6a1f50447f79
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date:   2015-09-21T19:21:16Z

    Check if Handler module exists before we try to call update

commit 2965fe6369544f9f4a08dc2b9f40bc815047f097
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date:   2015-09-24T19:15:32Z

    Refactor couch_epi to simplify it

commit cea86ed9a0034042919a468825631e8dabd7f606
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date:   2015-09-28T16:20:15Z

    Update documentation

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40654484
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -12,18 +12,33 @@
     
     -module(couch_epi_sup).
     
    +%% ----------------------------------
    +%% Important assumption
    +%% ====================
    +%% Keeper and codechange_monitor childspecs relie on underdocumented feature.
    --- End diff --
    
    And "underdocumented" I suppose - it's about having inadequate documentation while I believe documentation is fine, it's just about using undocumented behaviour.
    
    Also header upper line need to be aligned by header width.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40804099
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -32,13 +47,216 @@
     start_link() ->
         supervisor:start_link({local, ?MODULE}, ?MODULE, []).
     
    +plugin_childspecs(Plugin, Children) ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    plugin_childspecs(Plugin, Plugins, Children).
    +
     %% ===================================================================
     %% Supervisor callbacks
     %% ===================================================================
     
     init([]) ->
    -    Children = [
    -        ?CHILD(couch_epi_server, worker),
    -        ?SUP(couch_epi_keeper_sup, [])
    +    {ok, { {one_for_one, 5, 10}, keepers()} }.
    +
    +%% ------------------------------------------------------------------
    +%% Internal Function Definitions
    +%% ------------------------------------------------------------------
    +
    +keepers() ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    Definitions0 = couch_epi_plugin:grouped_definitions(Plugins),
    +    Definitions1 = reorder(Definitions0),
    +    Children = keeper_childspecs(Definitions1),
    +    remove_duplicates(Children).
    +
    +plugin_childspecs(Plugin, Plugins, Children) ->
    +    Definitions = couch_epi_plugin:grouped_definitions([Plugin]),
    +    ExtraChildren = couch_epi_plugin:plugin_processes(Plugin, Plugins),
    +    merge(ExtraChildren, Children) ++ childspecs(Definitions).
    +
    +childspecs(Definitions) ->
    +    lists:map(fun({{Kind, Key}, Defs}) ->
    +        CodeGen = couch_epi_plugin:codegen(Kind),
    +        Handle = CodeGen:get_handle(Key),
    +        Modules = lists:append([modules(Spec) || {_App, Spec} <- Defs]),
    +        Name = service_name(Key) ++ "|" ++ atom_to_list(Kind),
    +        code_monitor(Name, [Handle], [Handle|Modules])
    +    end, Definitions).
    +
    +%% ------------------------------------------------------------------
    +%% Helper Function Definitions
    +%% ------------------------------------------------------------------
    +
    +reorder(Definitions) ->
    +    lists:sort(fun(A, B) ->
    +       kinds_order(A) =< kinds_order(B)
    +    end, Definitions).
    +
    +kinds_order({{providers, _Key}, _Specs}) -> 3;
    +kinds_order({{services, _Key}, _Specs}) -> 1;
    +kinds_order({{data_providers, _Key}, _Specs}) -> 4;
    +kinds_order({{data_subscriptions, _Key}, _Specs}) -> 2.
    +
    +remove_duplicates(Definitions) ->
    +    remove_duplicates(Definitions, []).
    +
    +remove_duplicates([Spec | Rest], Acc) ->
    +    case has_duplicate(Acc, Spec) of
    +        true ->
    +            remove_duplicates(Rest, Acc);
    +        false ->
    +            remove_duplicates(Rest, [Spec | Acc])
    +    end;
    +remove_duplicates([], Acc) ->
    +    Acc.
    +
    +
    +has_duplicate(Acc, {IdA, _, _, _, _, _}) ->
    +    lists:any(fun({IdB, _, _, _, _, _}) ->
    +        IdA == IdB
    +    end, Acc).
    +
    +keeper_childspecs(Definitions) ->
    +    lists:map(fun({{Kind, Key}, _Specs}) ->
    +        Name = service_name(Key) ++ "|keeper",
    +        CodeGen = couch_epi_plugin:codegen(Kind),
    +        Handle = CodeGen:get_handle(Key),
    +        keeper(Name, [provider_kind(Kind), Key, CodeGen], [Handle])
    +    end, Definitions).
    +
    +keeper(Name, Args, Modules) ->
    +    {"couch_epi|" ++ Name, {couch_epi_module_keeper, start_link,
    +        Args}, permanent, 5000, worker, Modules}.
    +
    +code_monitor(Name, Args, Modules0) ->
    +    Modules = [couch_epi_codechange_monitor | Modules0],
    +    {"couch_epi_codechange_monitor|" ++ Name,
    +        {couch_epi_codechange_monitor, start_link, Args}, permanent, 5000, worker, Modules}.
    +
    +provider_kind(services) -> providers;
    +provider_kind(data_subscriptions) -> data_providers;
    +provider_kind(Kind) -> Kind.
    +
    +service_name({ServiceId, Key}) ->
    +    atom_to_list(ServiceId) ++ ":" ++ atom_to_list(Key);
    +service_name(ServiceId) ->
    +    atom_to_list(ServiceId).
    +
    +modules(#couch_epi_spec{kind = providers, value = Module}) ->
    +    [Module];
    +modules(#couch_epi_spec{kind = services, value = Module}) ->
    +    [Module];
    +modules(#couch_epi_spec{kind = data_providers, value = Value}) ->
    +    case Value of
    +        {module, Module} -> [Module];
    +        _ -> []
    +    end;
    +modules(#couch_epi_spec{kind = data_subscriptions, behaviour = Module}) ->
    +    [Module].
    +
    +merge([], Children) ->
    +    Children;
    +merge([{Id, _, _, _, _, _} = Spec | Rest], Children) ->
    +    merge(Rest, lists:keystore(Id, 1, Children, Spec)).
    --- End diff --
    
    ukeymerge would reorder the children


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by jaydoane <gi...@git.apache.org>.
Github user jaydoane commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40628891
  
    --- Diff: README.md ---
    @@ -53,6 +47,12 @@ it would then just do something like:
     
          couch_epi:get(Handle, Service, Key)
     
    +The service provider also have to mention the data keys it is using in it's
    --- End diff --
    
    s/it's/its


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144104309
  
    though it didn't change, could these functions in couch_epi_data_gen.erl be enhanced?;
    from
    ```
    by_key(Handle) ->
        Handle:by_key().
    ```
    
    to 
    ```
    by_key(Handle) when Handle /= undefined ->
        Handle:by_key().
    ```
    
    This might make it easier to catch some errors, the undef is changed to a function_clause. Is that a clearer error?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40695366
  
    --- Diff: src/couch_epi_data.erl ---
    @@ -12,131 +12,103 @@
     
     -module(couch_epi_data).
     
    --behaviour(gen_server).
    +-include("couch_epi.hrl").
     
     %% ------------------------------------------------------------------
     %% API Function Exports
     %% ------------------------------------------------------------------
     
    --export([childspec/4]).
    --export([start_link/4, reload/1]).
    --export([wait/1, stop/1]).
    -
    -%% ------------------------------------------------------------------
    -%% gen_server Function Exports
    -%% ------------------------------------------------------------------
    -
    --export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    -         terminate/2, code_change/3]).
    -
    --record(state, {
    -    subscriber, module, key, hash, handle,
    -    initialized = false, pending = []}).
    +-export([interval/1, data/1]).
     
     %% ------------------------------------------------------------------
     %% API Function Definitions
     %% ------------------------------------------------------------------
     
    -childspec(Id, App, EpiKey, Module) ->
    -    {
    -        Id,
    -        {?MODULE, start_link, [
    -            App,
    -            EpiKey,
    -            Module,
    -            []
    -        ]},
    -        permanent,
    -        5000,
    -        worker,
    -        [Module]
    -    }.
    -
    -start_link(SubscriberApp, {epi_key, Key}, Module, Options) ->
    -    maybe_start_keeper(Key),
    -    gen_server:start_link(?MODULE, [SubscriberApp, Module, Key, Options], []).
    -
    -reload(Server) ->
    -    gen_server:call(Server, reload).
    -
    -wait(Server) ->
    -    gen_server:call(Server, wait).
    -
    -stop(Server) ->
    -    catch gen_server:call(Server, stop).
    +interval(Specs) ->
    +    extract_minimal_interval(Specs).
     
    -%% ------------------------------------------------------------------
    -%% gen_server Function Definitions
    -%% ------------------------------------------------------------------
    -
    -init([Subscriber, Module, Key, _Options]) ->
    -    gen_server:cast(self(), init),
    -    {ok, #state{
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key,
    -        handle = couch_epi_data_gen:get_handle(Key)}}.
    -
    -handle_call(wait, _From, #state{initialized = true} = State) ->
    -    {reply, ok, State};
    -handle_call(wait, From, #state{pending = Pending} = State) ->
    -    {noreply, State#state{pending = [From | Pending]}};
    -handle_call(reload, _From, State) ->
    -    {Res, NewState} = reload_if_updated(State),
    -    {reply, Res, NewState};
    -handle_call(stop, _From, State) ->
    -    {stop, normal, State};
    -handle_call(_Request, _From, State) ->
    -    {reply, ok, State}.
    -
    -handle_cast(init, #state{pending = Pending} = State) ->
    -    {_, NewState} = reload_if_updated(State),
    -    [gen_server:reply(Client, ok) || Client <- Pending],
    -    {noreply, NewState#state{initialized = true, pending = []}};
    -handle_cast(_Msg, State) ->
    -    {noreply, State}.
    -
    -handle_info(_Info, State) ->
    -    {noreply, State}.
    -
    -terminate(_Reason, _State) ->
    -    ok.
    -
    -code_change(_OldVsn, State, _Extra) ->
    -    {_, NewState} = reload_if_updated(State),
    -    {ok, NewState}.
    +data(Specs) ->
    +    Locators = locate_sources(Specs),
    +    case lists:foldl(fun collect_data/2, {ok, [], []}, Locators) of
    +        {ok, Hashes, Data} ->
    +            {ok, couch_epi_util:hash(Hashes), Data};
    +        Error ->
    +            Error
    +    end.
     
     %% ------------------------------------------------------------------
     %% Internal Function Definitions
     %% ------------------------------------------------------------------
     
    -reload_if_updated(#state{hash = OldHash, module = Module} = State) ->
    -    case couch_epi_functions_gen:hash([Module]) of
    -        OldHash ->
    -            {ok, State};
    -        Hash ->
    -            safe_set(Hash, State)
    +collect_data({App, Locator}, {ok, HashAcc, DataAcc}) ->
    +    case definitions(Locator) of
    +        {ok, Hash, Data} ->
    +            {ok, [Hash | HashAcc], [{App, Data} | DataAcc]};
    +        Error ->
    +            Error
    +    end;
    +collect_data({_App, _Locator}, Error) ->
    +    Error.
    +
    +extract_minimal_interval(Specs) ->
    +    lists:foldl(fun minimal_interval/2, undefined, Specs).
    +
    +minimal_interval({_App, #couch_epi_spec{options = Options}}, Min) ->
    +    case lists:keyfind(interval, 1, Options) of
    +        {interval, Interval} -> min(Interval, Min);
    +        false -> Min
         end.
     
    -safe_set(Hash, #state{} = State) ->
    -    #state{
    -        handle = Handle,
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key} = State,
    -    try
    -        Data = get_from_module(Module),
    -        OldData = couch_epi_data_gen:current_data(Handle, Subscriber),
    -        ok = couch_epi_data_gen:set(Handle, Subscriber, Data),
    -        couch_epi_server:notify(Subscriber, Key, {data, OldData}, {data, Data}),
    -        {ok, State#state{hash = Hash}}
    -    catch Class:Reason ->
    -        {{Class, Reason}, State}
    +locate_sources(Specs) ->
    +    lists:map(fun({ProviderApp, #couch_epi_spec{value = Src}}) ->
    +        {ok, Locator} = locate(ProviderApp, Src),
    +        {ProviderApp, Locator}
    +    end, Specs).
    +
    +locate(App, {priv_file, FileName}) ->
    +    case priv_path(App, FileName) of
    +        {ok, FilePath} ->
    +            ok = ensure_exists(FilePath),
    +            {ok, {file, FilePath}};
    +        Else ->
    +            Else
    +    end;
    +locate(_App, {file, FilePath}) ->
    +    ok = ensure_exists(FilePath),
    +    {ok, {file, FilePath}};
    +locate(_App, Locator) ->
    +    {ok, Locator}.
    +
    +priv_path(AppName, FileName) ->
    +    case code:priv_dir(AppName) of
    +        {error, _Error} = Error ->
    +            Error;
    +        Dir ->
    +            {ok, filename:join(Dir, FileName)}
         end.
     
    -get_from_module(Module) ->
    -    Module:data().
    +ensure_exists(FilePath) ->
    +    case filelib:is_regular(FilePath) of
    +        true ->
    +            ok;
    +        false ->
    +            {error, {notfound, FilePath}}
    --- End diff --
    
    given that every call to it does `ok = ensure_exists(`, why not call filelib:is_regular directly?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/couchdb-couch-epi/pull/12


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144133292
  
    +1, I solved my issue


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by jaydoane <gi...@git.apache.org>.
Github user jaydoane commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40629128
  
    --- Diff: src/couch_epi.erl ---
    @@ -13,7 +13,7 @@
     -module(couch_epi).
     
     %% subscribtion management
    --- End diff --
    
    Should this comment change?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40699443
  
    --- Diff: src/couch_epi_data.erl ---
    @@ -12,131 +12,103 @@
     
     -module(couch_epi_data).
     
    --behaviour(gen_server).
    +-include("couch_epi.hrl").
     
     %% ------------------------------------------------------------------
     %% API Function Exports
     %% ------------------------------------------------------------------
     
    --export([childspec/4]).
    --export([start_link/4, reload/1]).
    --export([wait/1, stop/1]).
    -
    -%% ------------------------------------------------------------------
    -%% gen_server Function Exports
    -%% ------------------------------------------------------------------
    -
    --export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    -         terminate/2, code_change/3]).
    -
    --record(state, {
    -    subscriber, module, key, hash, handle,
    -    initialized = false, pending = []}).
    +-export([interval/1, data/1]).
     
     %% ------------------------------------------------------------------
     %% API Function Definitions
     %% ------------------------------------------------------------------
     
    -childspec(Id, App, EpiKey, Module) ->
    -    {
    -        Id,
    -        {?MODULE, start_link, [
    -            App,
    -            EpiKey,
    -            Module,
    -            []
    -        ]},
    -        permanent,
    -        5000,
    -        worker,
    -        [Module]
    -    }.
    -
    -start_link(SubscriberApp, {epi_key, Key}, Module, Options) ->
    -    maybe_start_keeper(Key),
    -    gen_server:start_link(?MODULE, [SubscriberApp, Module, Key, Options], []).
    -
    -reload(Server) ->
    -    gen_server:call(Server, reload).
    -
    -wait(Server) ->
    -    gen_server:call(Server, wait).
    -
    -stop(Server) ->
    -    catch gen_server:call(Server, stop).
    +interval(Specs) ->
    +    extract_minimal_interval(Specs).
     
    -%% ------------------------------------------------------------------
    -%% gen_server Function Definitions
    -%% ------------------------------------------------------------------
    -
    -init([Subscriber, Module, Key, _Options]) ->
    -    gen_server:cast(self(), init),
    -    {ok, #state{
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key,
    -        handle = couch_epi_data_gen:get_handle(Key)}}.
    -
    -handle_call(wait, _From, #state{initialized = true} = State) ->
    -    {reply, ok, State};
    -handle_call(wait, From, #state{pending = Pending} = State) ->
    -    {noreply, State#state{pending = [From | Pending]}};
    -handle_call(reload, _From, State) ->
    -    {Res, NewState} = reload_if_updated(State),
    -    {reply, Res, NewState};
    -handle_call(stop, _From, State) ->
    -    {stop, normal, State};
    -handle_call(_Request, _From, State) ->
    -    {reply, ok, State}.
    -
    -handle_cast(init, #state{pending = Pending} = State) ->
    -    {_, NewState} = reload_if_updated(State),
    -    [gen_server:reply(Client, ok) || Client <- Pending],
    -    {noreply, NewState#state{initialized = true, pending = []}};
    -handle_cast(_Msg, State) ->
    -    {noreply, State}.
    -
    -handle_info(_Info, State) ->
    -    {noreply, State}.
    -
    -terminate(_Reason, _State) ->
    -    ok.
    -
    -code_change(_OldVsn, State, _Extra) ->
    -    {_, NewState} = reload_if_updated(State),
    -    {ok, NewState}.
    +data(Specs) ->
    +    Locators = locate_sources(Specs),
    +    case lists:foldl(fun collect_data/2, {ok, [], []}, Locators) of
    +        {ok, Hashes, Data} ->
    +            {ok, couch_epi_util:hash(Hashes), Data};
    +        Error ->
    +            Error
    +    end.
     
     %% ------------------------------------------------------------------
     %% Internal Function Definitions
     %% ------------------------------------------------------------------
     
    -reload_if_updated(#state{hash = OldHash, module = Module} = State) ->
    -    case couch_epi_functions_gen:hash([Module]) of
    -        OldHash ->
    -            {ok, State};
    -        Hash ->
    -            safe_set(Hash, State)
    +collect_data({App, Locator}, {ok, HashAcc, DataAcc}) ->
    +    case definitions(Locator) of
    +        {ok, Hash, Data} ->
    +            {ok, [Hash | HashAcc], [{App, Data} | DataAcc]};
    +        Error ->
    +            Error
    +    end;
    +collect_data({_App, _Locator}, Error) ->
    +    Error.
    +
    +extract_minimal_interval(Specs) ->
    +    lists:foldl(fun minimal_interval/2, undefined, Specs).
    +
    +minimal_interval({_App, #couch_epi_spec{options = Options}}, Min) ->
    +    case lists:keyfind(interval, 1, Options) of
    +        {interval, Interval} -> min(Interval, Min);
    +        false -> Min
         end.
     
    -safe_set(Hash, #state{} = State) ->
    -    #state{
    -        handle = Handle,
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key} = State,
    -    try
    -        Data = get_from_module(Module),
    -        OldData = couch_epi_data_gen:current_data(Handle, Subscriber),
    -        ok = couch_epi_data_gen:set(Handle, Subscriber, Data),
    -        couch_epi_server:notify(Subscriber, Key, {data, OldData}, {data, Data}),
    -        {ok, State#state{hash = Hash}}
    -    catch Class:Reason ->
    -        {{Class, Reason}, State}
    +locate_sources(Specs) ->
    +    lists:map(fun({ProviderApp, #couch_epi_spec{value = Src}}) ->
    +        {ok, Locator} = locate(ProviderApp, Src),
    +        {ProviderApp, Locator}
    +    end, Specs).
    +
    +locate(App, {priv_file, FileName}) ->
    +    case priv_path(App, FileName) of
    +        {ok, FilePath} ->
    +            ok = ensure_exists(FilePath),
    +            {ok, {file, FilePath}};
    +        Else ->
    +            Else
    +    end;
    +locate(_App, {file, FilePath}) ->
    +    ok = ensure_exists(FilePath),
    +    {ok, {file, FilePath}};
    +locate(_App, Locator) ->
    +    {ok, Locator}.
    +
    +priv_path(AppName, FileName) ->
    +    case code:priv_dir(AppName) of
    +        {error, _Error} = Error ->
    +            Error;
    +        Dir ->
    +            {ok, filename:join(Dir, FileName)}
         end.
     
    -get_from_module(Module) ->
    -    Module:data().
    +ensure_exists(FilePath) ->
    +    case filelib:is_regular(FilePath) of
    +        true ->
    +            ok;
    +        false ->
    +            {error, {notfound, FilePath}}
    --- End diff --
    
    I abstracted it into a function for a cleaner error message (`{error, {FilePath, Reason}}`).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40800493
  
    --- Diff: test/couch_epi_tests.erl ---
    @@ -152,26 +228,26 @@ teardown(#ctx{kv = KV}) ->
         application:stop(couch_epi),
         ok.
     
    -upgrade_release(Pid, Module) ->
    +upgrade_release(Pid, Modules) ->
         sys:suspend(Pid),
    -    'ok' = sys:change_code(Pid, Module, 'undefined', []),
    +    ['ok' = sys:change_code(Pid, M, 'undefined', []) || M <- Modules],
    --- End diff --
    
    I'll fix it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144718944
  
    +1 after appropriate squashing


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40776575
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -32,13 +47,216 @@
     start_link() ->
         supervisor:start_link({local, ?MODULE}, ?MODULE, []).
     
    +plugin_childspecs(Plugin, Children) ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    plugin_childspecs(Plugin, Plugins, Children).
    +
     %% ===================================================================
     %% Supervisor callbacks
     %% ===================================================================
     
     init([]) ->
    -    Children = [
    -        ?CHILD(couch_epi_server, worker),
    -        ?SUP(couch_epi_keeper_sup, [])
    +    {ok, { {one_for_one, 5, 10}, keepers()} }.
    +
    +%% ------------------------------------------------------------------
    +%% Internal Function Definitions
    +%% ------------------------------------------------------------------
    +
    +keepers() ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    Definitions0 = couch_epi_plugin:grouped_definitions(Plugins),
    +    Definitions1 = reorder(Definitions0),
    +    Children = keeper_childspecs(Definitions1),
    +    remove_duplicates(Children).
    +
    +plugin_childspecs(Plugin, Plugins, Children) ->
    +    Definitions = couch_epi_plugin:grouped_definitions([Plugin]),
    +    ExtraChildren = couch_epi_plugin:plugin_processes(Plugin, Plugins),
    +    merge(ExtraChildren, Children) ++ childspecs(Definitions).
    --- End diff --
    
    `lists:ukeymerge(1, ExtraChildren, Children) ++ childspecs(Definitions).`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40776997
  
    --- Diff: test/couch_epi_tests.erl ---
    @@ -152,26 +228,26 @@ teardown(#ctx{kv = KV}) ->
         application:stop(couch_epi),
         ok.
     
    -upgrade_release(Pid, Module) ->
    +upgrade_release(Pid, Modules) ->
         sys:suspend(Pid),
    -    'ok' = sys:change_code(Pid, Module, 'undefined', []),
    +    ['ok' = sys:change_code(Pid, M, 'undefined', []) || M <- Modules],
    --- End diff --
    
    or the undefined atom?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40690028
  
    --- Diff: src/couch_epi_codechange_monitor.erl ---
    @@ -0,0 +1,63 @@
    +% Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +% use this file except in compliance with the License. You may obtain a copy of
    +% the License at
    +%
    +% http://www.apache.org/licenses/LICENSE-2.0
    +%
    +% Unless required by applicable law or agreed to in writing, software
    +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +% License for the specific language governing permissions and limitations under
    +% the License.
    +
    +-module(couch_epi_codechange_monitor).
    +
    +-behaviour(gen_server).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([start_link/1]).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    +         terminate/2, code_change/3]).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Definitions
    +%% ------------------------------------------------------------------
    +
    +start_link(Handler) ->
    +    gen_server:start_link(?MODULE, [Handler], []).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Definitions
    +%% ------------------------------------------------------------------
    +
    +init([Handler]) ->
    +    couch_epi_module_keeper:reload(Handler),
    +    {ok, Handler}.
    +
    +handle_call(_Request, _From, State) ->
    +    {reply, ok, State}.
    +
    +handle_cast(_Msg, State) ->
    +    {noreply, State}.
    +
    +handle_info(_Info, State) ->
    +    {noreply, State}.
    +
    +terminate(_Reason, _State) ->
    +    ok.
    +
    +code_change(_OldVsn, Keeper, _Extra) ->
    +    couch_epi_module_keeper:reload(Keeper),
    --- End diff --
    
    When we do `couch_epi:register_service(chttpd_epi)` from the supervisor of the application. Among other children we get things like:
    ```erlang
    {"couch_epi_codechange_monitor|my_service|providers", 
        {couch_epi_codechange_monitor, start_link, [couch_epi_functions_gen_my_service]}, 
        permanent, 5000, worker, [couch_epi_functions_gen_my_service, provider1, provider2]}
    ```
    `[couch_epi_functions_gen_my_service, provider1, provider2]` - is a list of modules to monitor. If anyone of them change release_handler would call `couch_epi_codechange_monitor:code_change/3`. We call `couch_epi_module_keeper:reload(Keeper)` in order to regenerate dispatch module if needed.
    It is also used for notification via `couch_epi_plugin Module:notify/3`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40799660
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -32,13 +47,216 @@
     start_link() ->
         supervisor:start_link({local, ?MODULE}, ?MODULE, []).
     
    +plugin_childspecs(Plugin, Children) ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    plugin_childspecs(Plugin, Plugins, Children).
    +
     %% ===================================================================
     %% Supervisor callbacks
     %% ===================================================================
     
     init([]) ->
    -    Children = [
    -        ?CHILD(couch_epi_server, worker),
    -        ?SUP(couch_epi_keeper_sup, [])
    +    {ok, { {one_for_one, 5, 10}, keepers()} }.
    +
    +%% ------------------------------------------------------------------
    +%% Internal Function Definitions
    +%% ------------------------------------------------------------------
    +
    +keepers() ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    Definitions0 = couch_epi_plugin:grouped_definitions(Plugins),
    +    Definitions1 = reorder(Definitions0),
    +    Children = keeper_childspecs(Definitions1),
    +    remove_duplicates(Children).
    +
    +plugin_childspecs(Plugin, Plugins, Children) ->
    +    Definitions = couch_epi_plugin:grouped_definitions([Plugin]),
    +    ExtraChildren = couch_epi_plugin:plugin_processes(Plugin, Plugins),
    +    merge(ExtraChildren, Children) ++ childspecs(Definitions).
    --- End diff --
    
    ukeymerge would reorder the children


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144106129
  
    @rnewson: On `by_key(Handle) when Handle /= undefined`
    I'll add the guards to `couch_epi.erl` instead.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144047435
  
    Applied all the patches, got an startup error:
    ```
    {"init terminating in do_boot",{{error,{{undef,[{couch_db_epi,providers,[],[]},{couch_epi_plugin,specs,2,[{file,"src/couch_epi_plugin.erl"},{line,123}]},{couch_epi_plugin,extract_definitions,1,[{file,"src/couch_epi_plugin.erl"},{line,102}]},{couch_epi_plugin,'-grouped_definitions/1-lc$^0/1-0-',1,[{file,"src/couch_epi_plugin.erl"},{line,62}]},{couch_epi_plugin,grouped_definitions,1,[{file,"src/couch_epi_plugin.erl"},{line,62}]},{couch_epi_sup,keepers,0,[{file,"src/couch_epi_sup.erl"},{line,67}]},{couch_epi_sup,init,1,[{file,"src/couch_epi_sup.erl"},{line,59}]},{supervisor,init,1,[{file,"supervisor.erl"},{line,272}]}]},{couch_epi_app,start,[normal,[]]}}},[{boot_node,start_app,3,[{file,"dev/boot_node.erl"},{line,134}]},{lists,foldl,3,[{file,"lists.erl"},{line,1262}]},{boot_node,start_app,3,[{file,"dev/boot_node.erl"},{line,124}]},{lists,foldl,3,[{file,"lists.erl"},{line,1262}]},{init,start_it,1,[]},{init,start_em,1,[]}]}}
    ```
    
    Did I miss something?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40695465
  
    --- Diff: src/couch_epi_data.erl ---
    @@ -12,131 +12,103 @@
     
     -module(couch_epi_data).
     
    --behaviour(gen_server).
    +-include("couch_epi.hrl").
     
     %% ------------------------------------------------------------------
     %% API Function Exports
     %% ------------------------------------------------------------------
     
    --export([childspec/4]).
    --export([start_link/4, reload/1]).
    --export([wait/1, stop/1]).
    -
    -%% ------------------------------------------------------------------
    -%% gen_server Function Exports
    -%% ------------------------------------------------------------------
    -
    --export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    -         terminate/2, code_change/3]).
    -
    --record(state, {
    -    subscriber, module, key, hash, handle,
    -    initialized = false, pending = []}).
    +-export([interval/1, data/1]).
     
     %% ------------------------------------------------------------------
     %% API Function Definitions
     %% ------------------------------------------------------------------
     
    -childspec(Id, App, EpiKey, Module) ->
    -    {
    -        Id,
    -        {?MODULE, start_link, [
    -            App,
    -            EpiKey,
    -            Module,
    -            []
    -        ]},
    -        permanent,
    -        5000,
    -        worker,
    -        [Module]
    -    }.
    -
    -start_link(SubscriberApp, {epi_key, Key}, Module, Options) ->
    -    maybe_start_keeper(Key),
    -    gen_server:start_link(?MODULE, [SubscriberApp, Module, Key, Options], []).
    -
    -reload(Server) ->
    -    gen_server:call(Server, reload).
    -
    -wait(Server) ->
    -    gen_server:call(Server, wait).
    -
    -stop(Server) ->
    -    catch gen_server:call(Server, stop).
    +interval(Specs) ->
    +    extract_minimal_interval(Specs).
     
    -%% ------------------------------------------------------------------
    -%% gen_server Function Definitions
    -%% ------------------------------------------------------------------
    -
    -init([Subscriber, Module, Key, _Options]) ->
    -    gen_server:cast(self(), init),
    -    {ok, #state{
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key,
    -        handle = couch_epi_data_gen:get_handle(Key)}}.
    -
    -handle_call(wait, _From, #state{initialized = true} = State) ->
    -    {reply, ok, State};
    -handle_call(wait, From, #state{pending = Pending} = State) ->
    -    {noreply, State#state{pending = [From | Pending]}};
    -handle_call(reload, _From, State) ->
    -    {Res, NewState} = reload_if_updated(State),
    -    {reply, Res, NewState};
    -handle_call(stop, _From, State) ->
    -    {stop, normal, State};
    -handle_call(_Request, _From, State) ->
    -    {reply, ok, State}.
    -
    -handle_cast(init, #state{pending = Pending} = State) ->
    -    {_, NewState} = reload_if_updated(State),
    -    [gen_server:reply(Client, ok) || Client <- Pending],
    -    {noreply, NewState#state{initialized = true, pending = []}};
    -handle_cast(_Msg, State) ->
    -    {noreply, State}.
    -
    -handle_info(_Info, State) ->
    -    {noreply, State}.
    -
    -terminate(_Reason, _State) ->
    -    ok.
    -
    -code_change(_OldVsn, State, _Extra) ->
    -    {_, NewState} = reload_if_updated(State),
    -    {ok, NewState}.
    +data(Specs) ->
    +    Locators = locate_sources(Specs),
    +    case lists:foldl(fun collect_data/2, {ok, [], []}, Locators) of
    +        {ok, Hashes, Data} ->
    +            {ok, couch_epi_util:hash(Hashes), Data};
    +        Error ->
    +            Error
    +    end.
     
     %% ------------------------------------------------------------------
     %% Internal Function Definitions
     %% ------------------------------------------------------------------
     
    -reload_if_updated(#state{hash = OldHash, module = Module} = State) ->
    -    case couch_epi_functions_gen:hash([Module]) of
    -        OldHash ->
    -            {ok, State};
    -        Hash ->
    -            safe_set(Hash, State)
    +collect_data({App, Locator}, {ok, HashAcc, DataAcc}) ->
    +    case definitions(Locator) of
    +        {ok, Hash, Data} ->
    +            {ok, [Hash | HashAcc], [{App, Data} | DataAcc]};
    +        Error ->
    +            Error
    +    end;
    +collect_data({_App, _Locator}, Error) ->
    +    Error.
    +
    +extract_minimal_interval(Specs) ->
    +    lists:foldl(fun minimal_interval/2, undefined, Specs).
    +
    +minimal_interval({_App, #couch_epi_spec{options = Options}}, Min) ->
    +    case lists:keyfind(interval, 1, Options) of
    +        {interval, Interval} -> min(Interval, Min);
    +        false -> Min
         end.
     
    -safe_set(Hash, #state{} = State) ->
    -    #state{
    -        handle = Handle,
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key} = State,
    -    try
    -        Data = get_from_module(Module),
    -        OldData = couch_epi_data_gen:current_data(Handle, Subscriber),
    -        ok = couch_epi_data_gen:set(Handle, Subscriber, Data),
    -        couch_epi_server:notify(Subscriber, Key, {data, OldData}, {data, Data}),
    -        {ok, State#state{hash = Hash}}
    -    catch Class:Reason ->
    -        {{Class, Reason}, State}
    +locate_sources(Specs) ->
    +    lists:map(fun({ProviderApp, #couch_epi_spec{value = Src}}) ->
    +        {ok, Locator} = locate(ProviderApp, Src),
    +        {ProviderApp, Locator}
    +    end, Specs).
    +
    +locate(App, {priv_file, FileName}) ->
    +    case priv_path(App, FileName) of
    +        {ok, FilePath} ->
    +            ok = ensure_exists(FilePath),
    +            {ok, {file, FilePath}};
    +        Else ->
    +            Else
    +    end;
    +locate(_App, {file, FilePath}) ->
    +    ok = ensure_exists(FilePath),
    +    {ok, {file, FilePath}};
    +locate(_App, Locator) ->
    +    {ok, Locator}.
    +
    +priv_path(AppName, FileName) ->
    +    case code:priv_dir(AppName) of
    +        {error, _Error} = Error ->
    +            Error;
    +        Dir ->
    +            {ok, filename:join(Dir, FileName)}
         end.
     
    -get_from_module(Module) ->
    -    Module:data().
    +ensure_exists(FilePath) ->
    +    case filelib:is_regular(FilePath) of
    +        true ->
    +            ok;
    +        false ->
    +            {error, {notfound, FilePath}}
    --- End diff --
    
    or, if you're doing it for a better name only;
    
    ```
    -spec check_exists(file()) -> boolean().
    check_exists(FilePath) ->
        ok == filelib:is_regular(FilePath).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40709729
  
    --- Diff: src/couch_epi_codechange_monitor.erl ---
    @@ -0,0 +1,63 @@
    +% Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +% use this file except in compliance with the License. You may obtain a copy of
    +% the License at
    +%
    +% http://www.apache.org/licenses/LICENSE-2.0
    +%
    +% Unless required by applicable law or agreed to in writing, software
    +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +% License for the specific language governing permissions and limitations under
    +% the License.
    +
    +-module(couch_epi_codechange_monitor).
    +
    +-behaviour(gen_server).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([start_link/1]).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    +         terminate/2, code_change/3]).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Definitions
    +%% ------------------------------------------------------------------
    +
    +start_link(Handler) ->
    +    gen_server:start_link(?MODULE, [Handler], []).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Definitions
    +%% ------------------------------------------------------------------
    +
    +init([Handler]) ->
    +    couch_epi_module_keeper:reload(Handler),
    +    {ok, Handler}.
    +
    +handle_call(_Request, _From, State) ->
    +    {reply, ok, State}.
    +
    +handle_cast(_Msg, State) ->
    +    {noreply, State}.
    +
    +handle_info(_Info, State) ->
    +    {noreply, State}.
    +
    +terminate(_Reason, _State) ->
    +    ok.
    +
    +code_change(_OldVsn, Keeper, _Extra) ->
    +    couch_epi_module_keeper:reload(Keeper),
    --- End diff --
    
    Hmm good point. We might need to add couch_epi_codechange_monitor to the list of modules in childspec. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144745626
  
    @rnewson: squashed. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40803114
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -32,13 +47,216 @@
     start_link() ->
         supervisor:start_link({local, ?MODULE}, ?MODULE, []).
     
    +plugin_childspecs(Plugin, Children) ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    plugin_childspecs(Plugin, Plugins, Children).
    +
     %% ===================================================================
     %% Supervisor callbacks
     %% ===================================================================
     
     init([]) ->
    -    Children = [
    -        ?CHILD(couch_epi_server, worker),
    -        ?SUP(couch_epi_keeper_sup, [])
    +    {ok, { {one_for_one, 5, 10}, keepers()} }.
    +
    +%% ------------------------------------------------------------------
    +%% Internal Function Definitions
    +%% ------------------------------------------------------------------
    +
    +keepers() ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    Definitions0 = couch_epi_plugin:grouped_definitions(Plugins),
    +    Definitions1 = reorder(Definitions0),
    +    Children = keeper_childspecs(Definitions1),
    +    remove_duplicates(Children).
    +
    +plugin_childspecs(Plugin, Plugins, Children) ->
    +    Definitions = couch_epi_plugin:grouped_definitions([Plugin]),
    +    ExtraChildren = couch_epi_plugin:plugin_processes(Plugin, Plugins),
    +    merge(ExtraChildren, Children) ++ childspecs(Definitions).
    +
    +childspecs(Definitions) ->
    +    lists:map(fun({{Kind, Key}, Defs}) ->
    +        CodeGen = couch_epi_plugin:codegen(Kind),
    +        Handle = CodeGen:get_handle(Key),
    +        Modules = lists:append([modules(Spec) || {_App, Spec} <- Defs]),
    +        Name = service_name(Key) ++ "|" ++ atom_to_list(Kind),
    +        code_monitor(Name, [Handle], [Handle|Modules])
    +    end, Definitions).
    +
    +%% ------------------------------------------------------------------
    +%% Helper Function Definitions
    +%% ------------------------------------------------------------------
    +
    +reorder(Definitions) ->
    +    lists:sort(fun(A, B) ->
    +       kinds_order(A) =< kinds_order(B)
    +    end, Definitions).
    +
    +kinds_order({{providers, _Key}, _Specs}) -> 3;
    +kinds_order({{services, _Key}, _Specs}) -> 1;
    +kinds_order({{data_providers, _Key}, _Specs}) -> 4;
    +kinds_order({{data_subscriptions, _Key}, _Specs}) -> 2.
    +
    +remove_duplicates(Definitions) ->
    +    remove_duplicates(Definitions, []).
    --- End diff --
    
    Actually we don't care about order anymore.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40691876
  
    --- Diff: src/couch_epi_codechange_monitor.erl ---
    @@ -0,0 +1,63 @@
    +% Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +% use this file except in compliance with the License. You may obtain a copy of
    +% the License at
    +%
    +% http://www.apache.org/licenses/LICENSE-2.0
    +%
    +% Unless required by applicable law or agreed to in writing, software
    +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +% License for the specific language governing permissions and limitations under
    +% the License.
    +
    +-module(couch_epi_codechange_monitor).
    +
    +-behaviour(gen_server).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([start_link/1]).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    +         terminate/2, code_change/3]).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Definitions
    +%% ------------------------------------------------------------------
    +
    +start_link(Handler) ->
    +    gen_server:start_link(?MODULE, [Handler], []).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Definitions
    +%% ------------------------------------------------------------------
    +
    +init([Handler]) ->
    +    couch_epi_module_keeper:reload(Handler),
    +    {ok, Handler}.
    +
    +handle_call(_Request, _From, State) ->
    +    {reply, ok, State}.
    +
    +handle_cast(_Msg, State) ->
    +    {noreply, State}.
    +
    +handle_info(_Info, State) ->
    +    {noreply, State}.
    +
    +terminate(_Reason, _State) ->
    +    ok.
    +
    +code_change(_OldVsn, Keeper, _Extra) ->
    +    couch_epi_module_keeper:reload(Keeper),
    --- End diff --
    
    Unless couch_epi_codechange_monitor module itself changes, it will not be including in an .appup file, so the code_change callback will not be called, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40776985
  
    --- Diff: test/couch_epi_tests.erl ---
    @@ -152,26 +228,26 @@ teardown(#ctx{kv = KV}) ->
         application:stop(couch_epi),
         ok.
     
    -upgrade_release(Pid, Module) ->
    +upgrade_release(Pid, Modules) ->
         sys:suspend(Pid),
    -    'ok' = sys:change_code(Pid, Module, 'undefined', []),
    +    ['ok' = sys:change_code(Pid, M, 'undefined', []) || M <- Modules],
    --- End diff --
    
    why quote the ok atom here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40776725
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -32,13 +47,216 @@
     start_link() ->
         supervisor:start_link({local, ?MODULE}, ?MODULE, []).
     
    +plugin_childspecs(Plugin, Children) ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    plugin_childspecs(Plugin, Plugins, Children).
    +
     %% ===================================================================
     %% Supervisor callbacks
     %% ===================================================================
     
     init([]) ->
    -    Children = [
    -        ?CHILD(couch_epi_server, worker),
    -        ?SUP(couch_epi_keeper_sup, [])
    +    {ok, { {one_for_one, 5, 10}, keepers()} }.
    +
    +%% ------------------------------------------------------------------
    +%% Internal Function Definitions
    +%% ------------------------------------------------------------------
    +
    +keepers() ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    Definitions0 = couch_epi_plugin:grouped_definitions(Plugins),
    +    Definitions1 = reorder(Definitions0),
    +    Children = keeper_childspecs(Definitions1),
    +    remove_duplicates(Children).
    +
    +plugin_childspecs(Plugin, Plugins, Children) ->
    +    Definitions = couch_epi_plugin:grouped_definitions([Plugin]),
    +    ExtraChildren = couch_epi_plugin:plugin_processes(Plugin, Plugins),
    +    merge(ExtraChildren, Children) ++ childspecs(Definitions).
    +
    +childspecs(Definitions) ->
    +    lists:map(fun({{Kind, Key}, Defs}) ->
    +        CodeGen = couch_epi_plugin:codegen(Kind),
    +        Handle = CodeGen:get_handle(Key),
    +        Modules = lists:append([modules(Spec) || {_App, Spec} <- Defs]),
    +        Name = service_name(Key) ++ "|" ++ atom_to_list(Kind),
    +        code_monitor(Name, [Handle], [Handle|Modules])
    +    end, Definitions).
    +
    +%% ------------------------------------------------------------------
    +%% Helper Function Definitions
    +%% ------------------------------------------------------------------
    +
    +reorder(Definitions) ->
    +    lists:sort(fun(A, B) ->
    +       kinds_order(A) =< kinds_order(B)
    +    end, Definitions).
    +
    +kinds_order({{providers, _Key}, _Specs}) -> 3;
    +kinds_order({{services, _Key}, _Specs}) -> 1;
    +kinds_order({{data_providers, _Key}, _Specs}) -> 4;
    +kinds_order({{data_subscriptions, _Key}, _Specs}) -> 2.
    +
    +remove_duplicates(Definitions) ->
    +    remove_duplicates(Definitions, []).
    --- End diff --
    
    `lists:ukeysort(1, Definitions).`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by jaydoane <gi...@git.apache.org>.
Github user jaydoane commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40637232
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -12,18 +12,33 @@
     
     -module(couch_epi_sup).
     
    +%% ----------------------------------
    +%% Important assumption
    +%% ====================
    +%% Keeper and codechange_monitor childspecs relie on underdocumented feature.
    --- End diff --
    
    s/relie/rely/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40776106
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -32,13 +47,216 @@
     start_link() ->
         supervisor:start_link({local, ?MODULE}, ?MODULE, []).
     
    +plugin_childspecs(Plugin, Children) ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    plugin_childspecs(Plugin, Plugins, Children).
    +
     %% ===================================================================
     %% Supervisor callbacks
     %% ===================================================================
     
     init([]) ->
    -    Children = [
    -        ?CHILD(couch_epi_server, worker),
    -        ?SUP(couch_epi_keeper_sup, [])
    +    {ok, { {one_for_one, 5, 10}, keepers()} }.
    +
    +%% ------------------------------------------------------------------
    +%% Internal Function Definitions
    +%% ------------------------------------------------------------------
    +
    +keepers() ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    Definitions0 = couch_epi_plugin:grouped_definitions(Plugins),
    +    Definitions1 = reorder(Definitions0),
    +    Children = keeper_childspecs(Definitions1),
    +    remove_duplicates(Children).
    +
    +plugin_childspecs(Plugin, Plugins, Children) ->
    +    Definitions = couch_epi_plugin:grouped_definitions([Plugin]),
    +    ExtraChildren = couch_epi_plugin:plugin_processes(Plugin, Plugins),
    +    merge(ExtraChildren, Children) ++ childspecs(Definitions).
    +
    +childspecs(Definitions) ->
    +    lists:map(fun({{Kind, Key}, Defs}) ->
    +        CodeGen = couch_epi_plugin:codegen(Kind),
    +        Handle = CodeGen:get_handle(Key),
    +        Modules = lists:append([modules(Spec) || {_App, Spec} <- Defs]),
    +        Name = service_name(Key) ++ "|" ++ atom_to_list(Kind),
    +        code_monitor(Name, [Handle], [Handle|Modules])
    +    end, Definitions).
    +
    +%% ------------------------------------------------------------------
    +%% Helper Function Definitions
    +%% ------------------------------------------------------------------
    +
    +reorder(Definitions) ->
    +    lists:sort(fun(A, B) ->
    +       kinds_order(A) =< kinds_order(B)
    +    end, Definitions).
    +
    +kinds_order({{providers, _Key}, _Specs}) -> 3;
    +kinds_order({{services, _Key}, _Specs}) -> 1;
    +kinds_order({{data_providers, _Key}, _Specs}) -> 4;
    +kinds_order({{data_subscriptions, _Key}, _Specs}) -> 2.
    +
    +remove_duplicates(Definitions) ->
    +    remove_duplicates(Definitions, []).
    +
    +remove_duplicates([Spec | Rest], Acc) ->
    +    case has_duplicate(Acc, Spec) of
    +        true ->
    +            remove_duplicates(Rest, Acc);
    +        false ->
    +            remove_duplicates(Rest, [Spec | Acc])
    +    end;
    +remove_duplicates([], Acc) ->
    +    Acc.
    +
    +
    +has_duplicate(Acc, {IdA, _, _, _, _, _}) ->
    +    lists:any(fun({IdB, _, _, _, _, _}) ->
    +        IdA == IdB
    +    end, Acc).
    +
    +keeper_childspecs(Definitions) ->
    +    lists:map(fun({{Kind, Key}, _Specs}) ->
    +        Name = service_name(Key) ++ "|keeper",
    +        CodeGen = couch_epi_plugin:codegen(Kind),
    +        Handle = CodeGen:get_handle(Key),
    +        keeper(Name, [provider_kind(Kind), Key, CodeGen], [Handle])
    +    end, Definitions).
    +
    +keeper(Name, Args, Modules) ->
    +    {"couch_epi|" ++ Name, {couch_epi_module_keeper, start_link,
    +        Args}, permanent, 5000, worker, Modules}.
    +
    +code_monitor(Name, Args, Modules0) ->
    +    Modules = [couch_epi_codechange_monitor | Modules0],
    +    {"couch_epi_codechange_monitor|" ++ Name,
    +        {couch_epi_codechange_monitor, start_link, Args}, permanent, 5000, worker, Modules}.
    +
    +provider_kind(services) -> providers;
    +provider_kind(data_subscriptions) -> data_providers;
    +provider_kind(Kind) -> Kind.
    +
    +service_name({ServiceId, Key}) ->
    +    atom_to_list(ServiceId) ++ ":" ++ atom_to_list(Key);
    +service_name(ServiceId) ->
    +    atom_to_list(ServiceId).
    +
    +modules(#couch_epi_spec{kind = providers, value = Module}) ->
    +    [Module];
    +modules(#couch_epi_spec{kind = services, value = Module}) ->
    +    [Module];
    +modules(#couch_epi_spec{kind = data_providers, value = Value}) ->
    +    case Value of
    +        {module, Module} -> [Module];
    +        _ -> []
    +    end;
    +modules(#couch_epi_spec{kind = data_subscriptions, behaviour = Module}) ->
    +    [Module].
    +
    +merge([], Children) ->
    +    Children;
    +merge([{Id, _, _, _, _, _} = Spec | Rest], Children) ->
    +    merge(Rest, lists:keystore(Id, 1, Children, Spec)).
    --- End diff --
    
    why not http://www.erlang.org/doc/man/lists.html#ukeymerge-3?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40800310
  
    --- Diff: src/couch_epi_sup.erl ---
    @@ -32,13 +47,216 @@
     start_link() ->
         supervisor:start_link({local, ?MODULE}, ?MODULE, []).
     
    +plugin_childspecs(Plugin, Children) ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    plugin_childspecs(Plugin, Plugins, Children).
    +
     %% ===================================================================
     %% Supervisor callbacks
     %% ===================================================================
     
     init([]) ->
    -    Children = [
    -        ?CHILD(couch_epi_server, worker),
    -        ?SUP(couch_epi_keeper_sup, [])
    +    {ok, { {one_for_one, 5, 10}, keepers()} }.
    +
    +%% ------------------------------------------------------------------
    +%% Internal Function Definitions
    +%% ------------------------------------------------------------------
    +
    +keepers() ->
    +    Plugins = application:get_env(couch_epi, plugins, []),
    +    Definitions0 = couch_epi_plugin:grouped_definitions(Plugins),
    +    Definitions1 = reorder(Definitions0),
    +    Children = keeper_childspecs(Definitions1),
    +    remove_duplicates(Children).
    +
    +plugin_childspecs(Plugin, Plugins, Children) ->
    +    Definitions = couch_epi_plugin:grouped_definitions([Plugin]),
    +    ExtraChildren = couch_epi_plugin:plugin_processes(Plugin, Plugins),
    +    merge(ExtraChildren, Children) ++ childspecs(Definitions).
    +
    +childspecs(Definitions) ->
    +    lists:map(fun({{Kind, Key}, Defs}) ->
    +        CodeGen = couch_epi_plugin:codegen(Kind),
    +        Handle = CodeGen:get_handle(Key),
    +        Modules = lists:append([modules(Spec) || {_App, Spec} <- Defs]),
    +        Name = service_name(Key) ++ "|" ++ atom_to_list(Kind),
    +        code_monitor(Name, [Handle], [Handle|Modules])
    +    end, Definitions).
    +
    +%% ------------------------------------------------------------------
    +%% Helper Function Definitions
    +%% ------------------------------------------------------------------
    +
    +reorder(Definitions) ->
    +    lists:sort(fun(A, B) ->
    +       kinds_order(A) =< kinds_order(B)
    +    end, Definitions).
    +
    +kinds_order({{providers, _Key}, _Specs}) -> 3;
    +kinds_order({{services, _Key}, _Specs}) -> 1;
    +kinds_order({{data_providers, _Key}, _Specs}) -> 4;
    +kinds_order({{data_subscriptions, _Key}, _Specs}) -> 2.
    +
    +remove_duplicates(Definitions) ->
    +    remove_duplicates(Definitions, []).
    --- End diff --
    
    We need children to be ordered in a particular way. But we probably can switch the order of operations [here](https://github.com/cloudant/couchdb-couch-epi/blob/simplify_couch_epi/src/couch_epi_sup.erl#L68:L70). I'll think about it. Thank you.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144070526
  
    there's erl_first_files in rebar specifically to help us build behaviour modules first but I can't get it to work.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144536218
  
    @rnewson ok with merge?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144014201
  
    @iilyak if I understood correct, all simplification is about that previously all the apps tried to register them selfs in couch_epi explicitly, now couch_epi tries to locate all the apps with the special modules to register. Is that correct? If so, how to register new features on fly?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144047289
  
    Can we solve this warning?
    ```
    /home/kxepal/projects/asf/couchdb/couchdb/src/chttpd/src/chttpd_epi.erl:16: Warning: behaviour couch_epi_plugin undefined
    ```
    I guess, need to change compilation order for rebar to make couch_epi before other apps, right?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144103614
  
    > @kxepal: And if it is, how to register new features on fly?
    Unfortunately there is no easy way to do it. But you can deal with it on release upgrade.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144114662
  
    I made some comments. I'm broadly +1, nice work. I'd like other people to look at it. I've checked all the branches out locally and run a dev cluster successfully.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#issuecomment-144072874
  
    @rnewson I think erl_first_files shouldn't help us here - it's about files order within the same project, while we need to fix order of compiled apps. Actually, I just moved couch_epi before chttpd in rebar.config.script and warning has gone.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40686246
  
    --- Diff: src/couch_epi_codechange_monitor.erl ---
    @@ -0,0 +1,63 @@
    +% Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +% use this file except in compliance with the License. You may obtain a copy of
    +% the License at
    +%
    +% http://www.apache.org/licenses/LICENSE-2.0
    +%
    +% Unless required by applicable law or agreed to in writing, software
    +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +% License for the specific language governing permissions and limitations under
    +% the License.
    +
    +-module(couch_epi_codechange_monitor).
    +
    +-behaviour(gen_server).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([start_link/1]).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Exports
    +%% ------------------------------------------------------------------
    +
    +-export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    +         terminate/2, code_change/3]).
    +
    +%% ------------------------------------------------------------------
    +%% API Function Definitions
    +%% ------------------------------------------------------------------
    +
    +start_link(Handler) ->
    +    gen_server:start_link(?MODULE, [Handler], []).
    +
    +%% ------------------------------------------------------------------
    +%% gen_server Function Definitions
    +%% ------------------------------------------------------------------
    +
    +init([Handler]) ->
    +    couch_epi_module_keeper:reload(Handler),
    +    {ok, Handler}.
    +
    +handle_call(_Request, _From, State) ->
    +    {reply, ok, State}.
    +
    +handle_cast(_Msg, State) ->
    +    {noreply, State}.
    +
    +handle_info(_Info, State) ->
    +    {noreply, State}.
    +
    +terminate(_Reason, _State) ->
    +    ok.
    +
    +code_change(_OldVsn, Keeper, _Extra) ->
    +    couch_epi_module_keeper:reload(Keeper),
    --- End diff --
    
    what's the purpose of this module?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40691917
  
    --- Diff: src/couch_epi_data.erl ---
    @@ -12,131 +12,103 @@
     
     -module(couch_epi_data).
     
    --behaviour(gen_server).
    +-include("couch_epi.hrl").
     
     %% ------------------------------------------------------------------
     %% API Function Exports
     %% ------------------------------------------------------------------
     
    --export([childspec/4]).
    --export([start_link/4, reload/1]).
    --export([wait/1, stop/1]).
    -
    -%% ------------------------------------------------------------------
    -%% gen_server Function Exports
    -%% ------------------------------------------------------------------
    -
    --export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    -         terminate/2, code_change/3]).
    -
    --record(state, {
    -    subscriber, module, key, hash, handle,
    -    initialized = false, pending = []}).
    +-export([interval/1, data/1]).
     
     %% ------------------------------------------------------------------
     %% API Function Definitions
     %% ------------------------------------------------------------------
     
    -childspec(Id, App, EpiKey, Module) ->
    -    {
    -        Id,
    -        {?MODULE, start_link, [
    -            App,
    -            EpiKey,
    -            Module,
    -            []
    -        ]},
    -        permanent,
    -        5000,
    -        worker,
    -        [Module]
    -    }.
    -
    -start_link(SubscriberApp, {epi_key, Key}, Module, Options) ->
    -    maybe_start_keeper(Key),
    -    gen_server:start_link(?MODULE, [SubscriberApp, Module, Key, Options], []).
    -
    -reload(Server) ->
    -    gen_server:call(Server, reload).
    -
    -wait(Server) ->
    -    gen_server:call(Server, wait).
    -
    -stop(Server) ->
    -    catch gen_server:call(Server, stop).
    +interval(Specs) ->
    +    extract_minimal_interval(Specs).
     
    -%% ------------------------------------------------------------------
    -%% gen_server Function Definitions
    -%% ------------------------------------------------------------------
    -
    -init([Subscriber, Module, Key, _Options]) ->
    -    gen_server:cast(self(), init),
    -    {ok, #state{
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key,
    -        handle = couch_epi_data_gen:get_handle(Key)}}.
    -
    -handle_call(wait, _From, #state{initialized = true} = State) ->
    -    {reply, ok, State};
    -handle_call(wait, From, #state{pending = Pending} = State) ->
    -    {noreply, State#state{pending = [From | Pending]}};
    -handle_call(reload, _From, State) ->
    -    {Res, NewState} = reload_if_updated(State),
    -    {reply, Res, NewState};
    -handle_call(stop, _From, State) ->
    -    {stop, normal, State};
    -handle_call(_Request, _From, State) ->
    -    {reply, ok, State}.
    -
    -handle_cast(init, #state{pending = Pending} = State) ->
    -    {_, NewState} = reload_if_updated(State),
    -    [gen_server:reply(Client, ok) || Client <- Pending],
    -    {noreply, NewState#state{initialized = true, pending = []}};
    -handle_cast(_Msg, State) ->
    -    {noreply, State}.
    -
    -handle_info(_Info, State) ->
    -    {noreply, State}.
    -
    -terminate(_Reason, _State) ->
    -    ok.
    -
    -code_change(_OldVsn, State, _Extra) ->
    -    {_, NewState} = reload_if_updated(State),
    -    {ok, NewState}.
    +data(Specs) ->
    +    Locators = locate_sources(Specs),
    +    case lists:foldl(fun collect_data/2, {ok, [], []}, Locators) of
    +        {ok, Hashes, Data} ->
    +            {ok, couch_epi_util:hash(Hashes), Data};
    +        Error ->
    +            Error
    +    end.
     
     %% ------------------------------------------------------------------
     %% Internal Function Definitions
     %% ------------------------------------------------------------------
     
    -reload_if_updated(#state{hash = OldHash, module = Module} = State) ->
    -    case couch_epi_functions_gen:hash([Module]) of
    -        OldHash ->
    -            {ok, State};
    -        Hash ->
    -            safe_set(Hash, State)
    +collect_data({App, Locator}, {ok, HashAcc, DataAcc}) ->
    +    case definitions(Locator) of
    +        {ok, Hash, Data} ->
    +            {ok, [Hash | HashAcc], [{App, Data} | DataAcc]};
    +        Error ->
    +            Error
    +    end;
    +collect_data({_App, _Locator}, Error) ->
    +    Error.
    +
    +extract_minimal_interval(Specs) ->
    +    lists:foldl(fun minimal_interval/2, undefined, Specs).
    +
    +minimal_interval({_App, #couch_epi_spec{options = Options}}, Min) ->
    +    case lists:keyfind(interval, 1, Options) of
    +        {interval, Interval} -> min(Interval, Min);
    +        false -> Min
         end.
     
    -safe_set(Hash, #state{} = State) ->
    -    #state{
    -        handle = Handle,
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key} = State,
    -    try
    -        Data = get_from_module(Module),
    -        OldData = couch_epi_data_gen:current_data(Handle, Subscriber),
    -        ok = couch_epi_data_gen:set(Handle, Subscriber, Data),
    -        couch_epi_server:notify(Subscriber, Key, {data, OldData}, {data, Data}),
    -        {ok, State#state{hash = Hash}}
    -    catch Class:Reason ->
    -        {{Class, Reason}, State}
    +locate_sources(Specs) ->
    +    lists:map(fun({ProviderApp, #couch_epi_spec{value = Src}}) ->
    +        {ok, Locator} = locate(ProviderApp, Src),
    +        {ProviderApp, Locator}
    +    end, Specs).
    +
    +locate(App, {priv_file, FileName}) ->
    +    case priv_path(App, FileName) of
    +        {ok, FilePath} ->
    +            ok = ensure_exists(FilePath),
    +            {ok, {file, FilePath}};
    +        Else ->
    +            Else
    +    end;
    +locate(_App, {file, FilePath}) ->
    +    ok = ensure_exists(FilePath),
    +    {ok, {file, FilePath}};
    +locate(_App, Locator) ->
    +    {ok, Locator}.
    +
    +priv_path(AppName, FileName) ->
    +    case code:priv_dir(AppName) of
    +        {error, _Error} = Error ->
    +            Error;
    +        Dir ->
    +            {ok, filename:join(Dir, FileName)}
         end.
     
    -get_from_module(Module) ->
    -    Module:data().
    +ensure_exists(FilePath) ->
    +    case filelib:is_regular(FilePath) of
    +        true ->
    +            ok;
    +        false ->
    +            {error, {notfound, FilePath}}
    --- End diff --
    
    I'll rename the function into `check_exists`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40691218
  
    --- Diff: src/couch_epi_data.erl ---
    @@ -12,131 +12,103 @@
     
     -module(couch_epi_data).
     
    --behaviour(gen_server).
    +-include("couch_epi.hrl").
     
     %% ------------------------------------------------------------------
     %% API Function Exports
     %% ------------------------------------------------------------------
     
    --export([childspec/4]).
    --export([start_link/4, reload/1]).
    --export([wait/1, stop/1]).
    -
    -%% ------------------------------------------------------------------
    -%% gen_server Function Exports
    -%% ------------------------------------------------------------------
    -
    --export([init/1, handle_call/3, handle_cast/2, handle_info/2,
    -         terminate/2, code_change/3]).
    -
    --record(state, {
    -    subscriber, module, key, hash, handle,
    -    initialized = false, pending = []}).
    +-export([interval/1, data/1]).
     
     %% ------------------------------------------------------------------
     %% API Function Definitions
     %% ------------------------------------------------------------------
     
    -childspec(Id, App, EpiKey, Module) ->
    -    {
    -        Id,
    -        {?MODULE, start_link, [
    -            App,
    -            EpiKey,
    -            Module,
    -            []
    -        ]},
    -        permanent,
    -        5000,
    -        worker,
    -        [Module]
    -    }.
    -
    -start_link(SubscriberApp, {epi_key, Key}, Module, Options) ->
    -    maybe_start_keeper(Key),
    -    gen_server:start_link(?MODULE, [SubscriberApp, Module, Key, Options], []).
    -
    -reload(Server) ->
    -    gen_server:call(Server, reload).
    -
    -wait(Server) ->
    -    gen_server:call(Server, wait).
    -
    -stop(Server) ->
    -    catch gen_server:call(Server, stop).
    +interval(Specs) ->
    +    extract_minimal_interval(Specs).
     
    -%% ------------------------------------------------------------------
    -%% gen_server Function Definitions
    -%% ------------------------------------------------------------------
    -
    -init([Subscriber, Module, Key, _Options]) ->
    -    gen_server:cast(self(), init),
    -    {ok, #state{
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key,
    -        handle = couch_epi_data_gen:get_handle(Key)}}.
    -
    -handle_call(wait, _From, #state{initialized = true} = State) ->
    -    {reply, ok, State};
    -handle_call(wait, From, #state{pending = Pending} = State) ->
    -    {noreply, State#state{pending = [From | Pending]}};
    -handle_call(reload, _From, State) ->
    -    {Res, NewState} = reload_if_updated(State),
    -    {reply, Res, NewState};
    -handle_call(stop, _From, State) ->
    -    {stop, normal, State};
    -handle_call(_Request, _From, State) ->
    -    {reply, ok, State}.
    -
    -handle_cast(init, #state{pending = Pending} = State) ->
    -    {_, NewState} = reload_if_updated(State),
    -    [gen_server:reply(Client, ok) || Client <- Pending],
    -    {noreply, NewState#state{initialized = true, pending = []}};
    -handle_cast(_Msg, State) ->
    -    {noreply, State}.
    -
    -handle_info(_Info, State) ->
    -    {noreply, State}.
    -
    -terminate(_Reason, _State) ->
    -    ok.
    -
    -code_change(_OldVsn, State, _Extra) ->
    -    {_, NewState} = reload_if_updated(State),
    -    {ok, NewState}.
    +data(Specs) ->
    +    Locators = locate_sources(Specs),
    +    case lists:foldl(fun collect_data/2, {ok, [], []}, Locators) of
    +        {ok, Hashes, Data} ->
    +            {ok, couch_epi_util:hash(Hashes), Data};
    +        Error ->
    +            Error
    +    end.
     
     %% ------------------------------------------------------------------
     %% Internal Function Definitions
     %% ------------------------------------------------------------------
     
    -reload_if_updated(#state{hash = OldHash, module = Module} = State) ->
    -    case couch_epi_functions_gen:hash([Module]) of
    -        OldHash ->
    -            {ok, State};
    -        Hash ->
    -            safe_set(Hash, State)
    +collect_data({App, Locator}, {ok, HashAcc, DataAcc}) ->
    +    case definitions(Locator) of
    +        {ok, Hash, Data} ->
    +            {ok, [Hash | HashAcc], [{App, Data} | DataAcc]};
    +        Error ->
    +            Error
    +    end;
    +collect_data({_App, _Locator}, Error) ->
    +    Error.
    +
    +extract_minimal_interval(Specs) ->
    +    lists:foldl(fun minimal_interval/2, undefined, Specs).
    +
    +minimal_interval({_App, #couch_epi_spec{options = Options}}, Min) ->
    +    case lists:keyfind(interval, 1, Options) of
    +        {interval, Interval} -> min(Interval, Min);
    +        false -> Min
         end.
     
    -safe_set(Hash, #state{} = State) ->
    -    #state{
    -        handle = Handle,
    -        subscriber = Subscriber,
    -        module = Module,
    -        key = Key} = State,
    -    try
    -        Data = get_from_module(Module),
    -        OldData = couch_epi_data_gen:current_data(Handle, Subscriber),
    -        ok = couch_epi_data_gen:set(Handle, Subscriber, Data),
    -        couch_epi_server:notify(Subscriber, Key, {data, OldData}, {data, Data}),
    -        {ok, State#state{hash = Hash}}
    -    catch Class:Reason ->
    -        {{Class, Reason}, State}
    +locate_sources(Specs) ->
    +    lists:map(fun({ProviderApp, #couch_epi_spec{value = Src}}) ->
    +        {ok, Locator} = locate(ProviderApp, Src),
    +        {ProviderApp, Locator}
    +    end, Specs).
    +
    +locate(App, {priv_file, FileName}) ->
    +    case priv_path(App, FileName) of
    +        {ok, FilePath} ->
    +            ok = ensure_exists(FilePath),
    +            {ok, {file, FilePath}};
    +        Else ->
    +            Else
    +    end;
    +locate(_App, {file, FilePath}) ->
    +    ok = ensure_exists(FilePath),
    +    {ok, {file, FilePath}};
    +locate(_App, Locator) ->
    +    {ok, Locator}.
    +
    +priv_path(AppName, FileName) ->
    +    case code:priv_dir(AppName) of
    +        {error, _Error} = Error ->
    +            Error;
    +        Dir ->
    +            {ok, filename:join(Dir, FileName)}
         end.
     
    -get_from_module(Module) ->
    -    Module:data().
    +ensure_exists(FilePath) ->
    +    case filelib:is_regular(FilePath) of
    +        true ->
    +            ok;
    +        false ->
    +            {error, {notfound, FilePath}}
    --- End diff --
    
    this function does not ensure the FilePath exists, so either the name or implementation is wrong.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-couch-epi pull request: Simplify couch epi

Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:

    https://github.com/apache/couchdb-couch-epi/pull/12#discussion_r40777079
  
    --- Diff: test/couch_epi_tests.erl ---
    @@ -152,26 +228,26 @@ teardown(#ctx{kv = KV}) ->
         application:stop(couch_epi),
         ok.
     
    -upgrade_release(Pid, Module) ->
    +upgrade_release(Pid, Modules) ->
         sys:suspend(Pid),
    -    'ok' = sys:change_code(Pid, Module, 'undefined', []),
    +    ['ok' = sys:change_code(Pid, M, 'undefined', []) || M <- Modules],
         sys:resume(Pid),
         ok.
     
     epi_config_update_test_() ->
         Funs = [
             fun ensure_notified_when_changed/2,
    -        fun ensure_not_notified_when_no_change/2,
    -        fun ensure_not_notified_when_unsubscribed/2
    +        fun ensure_not_notified_when_no_change/2%%,
    +        %%fun ensure_not_notified_when_unsubscribed/2
    --- End diff --
    
    remove commented-out code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---