Order peers in pick_peer callbacks

The order of peers presented to a diameter_app(3) pick_peer callback has
previously not been documented, but there are use cases that are
simplified by an ordering. For example, consider preferring a direct
connection to a specified Destination-Host/Realm to any host in the
realm. The implementation previously treated this as a special case by
placing matching hosts at the head of the peers list, but the
documentation made no guarantees. Now present peers in match-order, so
that the desired sorting is the result of the following filter.

  {any, [{all, [host, realm]}, realm]}

The implementation is not backwards compatible in the sense that a realm
filter alone is no longer equivalent in this case. However, as stated,
the documentation never made any guarantees regarding the sorting.

2 files changed, 48 insertions, 29 deletions

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index ab9ad25a3a..00b54ffbc4 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -500,6 +500,18 @@ Matches only those peers matched by each filter in the specified list.</p>
 <p>
 Matches only those peers matched by at least one filter in the
 specified list.</p>
+
+<p>
+The resulting peer list will be in match order, peers matching the
+first filter of the list sorting before those matched by the second,
+and so on.
+For example, the following filter causes peers matching both the host
+and realm filters to be presented before those matching only the realm
+filter.</p>
+
+<pre>
+{any, [{all, [host, realm]}, realm]}
+</pre>
 </item>
 
 </taglist>
diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index ab56ca9cef..76b05a2ad4 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -1460,42 +1460,52 @@ pick_peer(Local,
 peers(Alias, RH, Filter, Peers) ->
     case ?Dict:find(Alias, Peers) of
         {ok, L} ->
-            ps(L, RH, Filter, {[],[]});
+            filter(L, RH, Filter);
         error ->
             []
     end.
 
-%% Place a peer whose Destination-Host/Realm matches those of the
-%% request at the front of the result list. Could add some sort of
-%% 'sort' option to allow more control.
-
-ps([], _, _, {Ys, Ns}) ->
-    lists:reverse(Ys, Ns);
-ps([{_TPid, #diameter_caps{} = Caps} = TC | Rest], RH, Filter, Acc) ->
-    ps(Rest, RH, Filter, pacc(caps_filter(Caps, RH, Filter),
-                              caps_filter(Caps, RH, {all, [host, realm]}),
-                              TC,
-                              Acc)).
-
-pacc(true, true, Peer, {Ts, Fs}) ->
-    {[Peer|Ts], Fs};
-pacc(true, false, Peer, {Ts, Fs}) ->
-    {Ts, [Peer|Fs]};
-pacc(_, _, _, Acc) ->
-    Acc.
+%% filter/3
+%%
+%% Return peers in match order.
 
-%% caps_filter/3
+filter(Peers, RH, Filter) ->
+    {Ts, _} = fltr(Peers, RH, Filter),
+    Ts.
+
+%% fltr/4
 
-caps_filter(C, RH, {neg, F}) ->
-    not caps_filter(C, RH, F);
+fltr(Peers, _, none) ->
+    {Peers, []};
 
-caps_filter(C, RH, {all, L})
+fltr(Peers, RH, {neg, F}) ->
+    {Ts, Fs} = fltr(Peers, RH, F),
+    {Fs, Ts};
+
+fltr(Peers, RH, {all, L})
   when is_list(L) ->
-    lists:all(fun(F) -> caps_filter(C, RH, F) end, L);
+    lists:foldl(fun(F,A) -> fltr_all(F, A, RH) end,
+                {Peers, []},
+                L);
 
-caps_filter(C, RH, {any, L})
+fltr(Peers, RH, {any, L})
   when is_list(L) ->
-    lists:any(fun(F) -> caps_filter(C, RH, F) end, L);
+    lists:foldl(fun(F,A) -> fltr_any(F, A, RH) end,
+                {[], Peers},
+                L);
+
+fltr(Peers, RH, F) ->
+    lists:partition(fun({_,C}) -> caps_filter(C, RH, F) end, Peers).
+
+fltr_all(F, {Ts0, Fs0}, RH) ->
+    {Ts1, Fs1} = fltr(Ts0, RH, F),
+    {Ts1, Fs0 ++ Fs1}.
+
+fltr_any(F, {Ts0, Fs0}, RH) ->
+    {Ts1, Fs1} = fltr(Fs0, RH, F),
+    {Ts0 ++ Ts1, Fs1}.
+
+%% caps_filter/3
 
 caps_filter(#diameter_caps{origin_host = {_,OH}}, [_,DH], host) ->
     eq(undefined, DH, OH);
@@ -1508,9 +1518,6 @@ caps_filter(C, _, Filter) ->
 
 %% caps_filter/2
 
-caps_filter(_, none) ->
-    true;
-
 caps_filter(#diameter_caps{origin_host = {_,OH}}, {host, H}) ->
     eq(any, H, OH);