Merge pull request #259 from tjsousa/secondary-preferred-fix

Respect secondary_preferred read preference by falling back to primary

lib/mongo/topology_description.ex

@@ -92,32 +92,39 @@ defmodule Mongo.TopologyDescription do
 
   ## Private Functions
 
-  defp select_replica_set_server(topology, mode, read_preference)
-      when mode in [:primary, :primary_preferred] do
-    primary = Enum.filter(topology.servers, fn {_, server} ->
+  defp select_replica_set_server(topology, :primary, read_preference) do
+    Enum.filter(topology.servers, fn {_, server} ->
       server.type == :rs_primary
     end)
+  end
 
-    if mode == :primary_preferred && Enum.empty? primary do
+  defp select_replica_set_server(topology, :primary_preferred, read_preference) do
+    preferred = select_replica_set_server(topology, :primary, read_preference)
+
+    if Enum.empty?(preferred) do
       select_replica_set_server(topology, :secondary, read_preference)
     else
-      primary
+      preferred
     end
   end
 
-  defp select_replica_set_server(topology, mode, read_preference)
-         when mode in [:secondary, :secondary_preferred, :nearest] do
-    initial = if mode in [:secondary, :secondary_preferred] do
-      topology.servers
-      |> Enum.filter(fn {_, server} ->
-        server.type == :rs_secondary
-      end)
-      |> Enum.into(%{})
+  defp select_replica_set_server(topology, :secondary_preferred, read_preference) do
+    preferred = select_replica_set_server(topology, :secondary, read_preference)
+
+    if Enum.empty?(preferred) do
+      select_replica_set_server(topology, :primary, read_preference)
     else
-      topology.servers
+      preferred
     end
+  end
 
-    initial
+  defp select_replica_set_server(topology, mode, read_preference)
+    when mode in [:secondary, :nearest] do
+    topology.servers
+    |> Enum.filter(fn {_, server} ->
+        server.type == :rs_secondary || mode == :nearest
+    end)
+    |> Enum.into(%{})
     |> filter_out_stale(topology, read_preference.max_staleness_ms)
     |> select_tag_sets(read_preference.tag_sets)
     |> filter_latency_window(topology.local_threshold_ms)

test/mongo/topology_description_test.exs

@@ -38,6 +38,19 @@ defmodule Mongo.TopologyDescriptionTest do
     assert {:ok, [master], true, false} ==
            TopologyDescription.select_servers(repl_set_with_master(), :read, opts)
 
+    opts = [
+      read_preference: ReadPreference.defaults(%{mode: :primary_preferred})
+    ]
+    assert {:ok, [master], true, false} ==
+      TopologyDescription.select_servers(repl_set_with_master(), :read, opts)
+
+    opts = [
+      read_preference: ReadPreference.defaults(%{mode: :primary_preferred})
+    ]
+    assert {:ok, List.delete(all_hosts, master), true, false} ==
+      TopologyDescription.select_servers(repl_set_no_master(), :read, opts)
+
+
     opts = [
       read_preference: ReadPreference.defaults(%{mode: :nearest})
     ]
@@ -50,8 +63,17 @@ defmodule Mongo.TopologyDescriptionTest do
     assert {:ok, List.delete(all_hosts, master), true, false} ==
            TopologyDescription.select_servers(repl_set_no_master(), :read, opts)
 
-    assert {:ok, [], false, false} ==
-           TopologyDescription.select_servers(repl_set_no_master(), :write)
+    opts = [
+      read_preference: ReadPreference.defaults(%{mode: :secondary_preferred})
+    ]
+    assert {:ok, List.delete(all_hosts, master), true, false} ==
+      TopologyDescription.select_servers(repl_set_with_master(), :read, opts)
+
+    assert {:ok, [master], true, false} ==
+      TopologyDescription.select_servers(repl_set_only_master(), :read, opts)
+
+    assert {:ok, List.delete(all_hosts, master), true, false} ==
+           TopologyDescription.select_servers(repl_set_no_master(), :read, opts)
 
     opts = [
       read_preference: ReadPreference.defaults(%{mode: :nearest})

test/support/topology_test_data.ex

@@ -191,4 +191,35 @@ defmodule Mongo.TopologyTestData do
       }
     }
   }
+  def repl_set_only_master, do: %{
+    compatibility_error: nil,
+    compatible: true,
+    local_threshold_ms: 15,
+    set_name: "replset1",
+    type: :replica_set_with_primary,
+    max_election_id: nil,
+    max_set_version: 3,
+    servers: %{
+      "localhost:27018" => %{
+        address: "localhost:27018",
+        arbiters: [],
+        election_id: nil,
+        error: nil,
+        last_update_time: nil,
+        last_write_date: nil,
+        max_wire_version: 0,
+        me: nil,
+        min_wire_version: 0,
+        op_time: nil,
+        passives: [],
+        primary: nil,
+        round_trip_time: 15,
+        set_name: nil,
+        set_version: nil,
+        tag_set: %{},
+        type: :rs_primary,
+        hosts: ["localhost:27018"]
+      }
+    }
+  }
 end