Stop using the deprecated GetNtpServers* methods

src/clients/ntp-client_proposal.rb

@@ -491,15 +491,20 @@ def configured_ntp_items
     # @return [Array<Yast::Term>] ntp address Item
     def timezone_ntp_items
       timezone_country = Timezone.GetCountryForTimezone(Timezone.timezone)
-      NtpClient.GetNtpServersByCountry(timezone_country, true)
+      servers = NtpClient.country_ntp_servers(timezone_country)
+      # Select the first occurrence of pool.ntp.org as the default option (bnc#940881)
+      selected = servers.find { |s| s.hostname.end_with?("pool.ntp.org") }
+      servers.map do |server|
+        Item(Id(server.hostname), server.hostname, server.hostname == selected)
+      end
     end
 
     # List of dhcp ntp servers Yast::Term items with the ntp address ID and
     # label
     #
     # @return [Array<Yast::Term>] ntp address table Item
     def dhcp_ntp_items
-      NtpClient.dhcp_ntp_servers.map { |s| Item(Id(s), s) }
+      NtpClient.dhcp_ntp_servers.map { |s| Item(Id(s.hostname), s.hostname) }
     end
 
     # List of ntp servers Yast::Term items with the ntp address ID and label

src/lib/y2ntp_client/widgets/pool_widgets.rb

@@ -308,8 +308,15 @@ def help
 
     private
 
+      # Returns the country for the given address
+      #
+      # FIXME: if the UI used a proper object instead of just a string, this
+      # method will not be needed.
+      #
+      # @param address [String] Server address
       def country_for(address)
-        Yast::NtpClient.GetNtpServers.fetch(address, {})["country"]
+        server = Yast::NtpClient.public_ntp_servers.find { |s| s.hostname == address }
+        server ? server.country : nil
       end
     end
 
@@ -382,7 +389,7 @@ def label
 
       # macro seeItemsSelection
       def items
-        ntp_servers.map { |s, v| [s, "#{s} (#{v["country"]})"] }
+        ntp_servers.map { |s| [s.hostname, "#{s.hostname} (#{s.country})"] }
       end
 
       def refresh(country)
@@ -393,10 +400,10 @@ def refresh(country)
     private
 
       def ntp_servers
-        servers = Yast::NtpClient.GetNtpServers
+        servers = Yast::NtpClient.public_ntp_servers
         return servers if @country.to_s.empty?
 
-        servers.find_all { |_s, v| v["country"] == @country }
+        servers.select { |s| s.country == @country }
       end
     end
   end

src/modules/NtpClient.rb

@@ -624,7 +624,7 @@ def AutoPackages
     # Convenience method to obtain the list of ntp servers proposed by DHCP
     # @see https://www.rubydoc.info/github/yast/yast-network/Yast/LanClass:${0}
     def dhcp_ntp_servers
-      Yast::Lan.dhcp_ntp_servers
+      Yast::Lan.dhcp_ntp_servers.map { |s| Y2Network::NtpServer.new(s) }
     end
 
     publish variable: :AbortFunction, type: "boolean ()"

test/ntp_client_proposal_test.rb

@@ -13,6 +13,51 @@
     client
   end
 
+  describe "#MakeProposal" do
+    let(:dhcp_ntp_servers) { [] }
+
+    before do
+      allow(Yast::Lan).to receive(:dhcp_ntp_servers)
+        .and_return(dhcp_ntp_servers)
+     allow(Yast::Directory).to receive(:find_data_file).and_call_original
+     allow(Yast::Directory).to receive(:find_data_file).with("ntp_servers.yml")
+       .and_return(DATA_PATH.join("ntp_servers_sample.yml").to_s)
+     allow(Yast::Timezone).to receive(:timezone).and_return("Europe/Berlin")
+     allow(Yast::Timezone).to receive(:GetCountryForTimezone)
+       .with("Europe/Berlin").and_return("de")
+    end
+
+    context "when NTP servers were found via DHCP" do
+      let(:dhcp_ntp_servers) { ["test.example.net"] }
+
+      it "proposes only the found servers" do
+        expect(Yast::UI).to receive(:ChangeWidget) do |*args|
+          items = args.last
+          hostnames = items.map { |i| i[1] }
+          expect(hostnames).to eq(
+            ["test.example.net"]
+          )
+        end
+        subject.MakeProposal
+      end
+    end
+
+    context "when no NTP server were found via DHCP" do
+      let(:dhcp_ntp_servers) { [] }
+
+      it "proposes the known public servers for the current timezone" do
+        expect(Yast::UI).to receive(:ChangeWidget) do |*args|
+          items = args.last
+          hostnames = items.map { |i| i[1] }
+          expect(hostnames).to eq(
+            ["tick.fh-augsburg.de", "de.pool.ntp.org"]
+          )
+        end
+        subject.MakeProposal
+      end
+    end
+  end
+
   describe "#Write" do
     let(:ntp_server) { "fake.pool.ntp.org" }
     let(:write_only) { false }

test/test_helper.rb

@@ -22,12 +22,13 @@
 
 # stub module to prevent its Import
 # Useful for modules from different yast packages, to avoid build dependencies
-def stub_module(name)
-  Yast.const_set(name.to_sym, Class.new { def self.fake_method; end })
+def stub_module(name, fake_class = nil)
+  fake_class = Class.new { def self.fake_method; end } if fake_class.nil?
+  Yast.const_set name.to_sym, fake_class
 end
 
 # stub classes from other modules to speed up a build
-stub_module("Lan")
+stub_module("Lan", Class.new { def dhcp_ntp_servers; []; end })
 stub_module("Language")
 stub_module("Pkg")
 stub_module("PackageCallbacks")