Refactor Wicked::ConfigReader so it can be used as base class

src/lib/y2network/config.rb

@@ -264,6 +264,31 @@ def backend=(id)
       @backend = Y2Network::Backend.all.find { |b| b.id == id }
     end
 
+    # Updates configuration section
+    #
+    # This method returns a new instance of Config, leaving the received
+    # as it is.
+    #
+    # @example Update interfaces list
+    #   config.update(interfaces: InterfacesCollection.new)
+    #
+    # @param changes [Hash<Symbol,Object>] A hash where the keys are the
+    #   sections to update and the values are the new values
+    # @return [Y2Network::Config]
+    def update(changes = {})
+      self.class.new(
+        interfaces:   changes[:interfaces] || interfaces,
+        connections:  changes[:connections] || connections,
+        s390_devices: changes[:s390_devices] || s390_devices,
+        drivers:      changes[:drivers] || drivers,
+        routing:      changes[:routing] || routing,
+        dns:          changes[:dns] || dns,
+        hostname:     changes[:hostname] || hostname,
+        source:       changes[:source] || source,
+        backend:      changes[:backend] || backend
+      )
+    end
+
     alias_method :eql?, :==
 
   private

src/lib/y2network/config_reader.rb

@@ -16,18 +16,41 @@
 #
 # To contact SUSE LLC about this file by physical or electronic mail, you may
 # find current contact information at www.suse.com.
+
+require "yast"
+
 module Y2Network
-  module ConfigReader
-    # Config reader for a given source
+  # This class is responsible for reading the configuration from the system
+  #
+  # It implements a {#config} method which returns a configuration object
+  # containing the information from the corresponding backend.
+  #
+  # It is expect that a configuration reader exists for each supported backend
+  # by inheriting from this class.
+  class ConfigReader
+    include Yast::Logger
+
+    class << self
+      # Returns a configuration reader for a given source
+      #
+      # @param source [Symbol] Source name (e.g., :wicked)
+      # @param opts  [Array<Object>] Reader options
+      # @return [Y2Network::Autoinst::ConfigReader,Y2Network::Wicked::ConfigReader]
+      def for(source, *opts)
+        require "y2network/#{source}/config_reader"
+        modname = source.to_s.split("_").map(&:capitalize).join
+        klass = Y2Network.const_get("#{modname}::ConfigReader")
+        klass.new(*opts)
+      end
+    end
+
+    def initialize(_opts = {}); end
+
+    # Returns the configuration from the given backend
     #
-    # @param source [Symbol] Source name (e.g., :wicked)
-    # @param opts  [Array<Object>] Reader options
-    # @return [Y2Network::Autoinst::ConfigReader,Y2Network::Wicked::ConfigReader]
-    def self.for(source, *opts)
-      require "y2network/#{source}/config_reader"
-      modname = source.to_s.split("_").map(&:capitalize).join
-      klass = Y2Network.const_get("#{modname}::ConfigReader")
-      klass.new(*opts)
+    # @return [Y2Network::Config] Network configuration
+    def config
+      Y2Network::Config.new
     end
   end
 end

src/lib/y2network/interfaces_collection.rb

@@ -197,5 +197,13 @@ def free_names(prefix, count)
         return result if result.size == count
       end
     end
+
+    # Returns a new collection including elements from both collections
+    #
+    # @param other [InterfacesCollection] Other interfaces collection
+    # @return [InterfacesCollection] New interfaces collection
+    def +(other)
+      self.class.new(to_a + other.to_a)
+    end
   end
 end

src/lib/y2network/wicked/config_reader.rb

@@ -18,7 +18,7 @@
 # find current contact information at www.suse.com.
 require "yast"
 require "cfa/sysctl_config"
-require "y2network/config"
+require "y2network/config_reader"
 require "y2network/interface"
 require "y2network/routing"
 require "y2network/routing_table"
@@ -34,42 +34,93 @@
 module Y2Network
   module Wicked
     # This class reads the current configuration from `/etc/sysconfig` files
-    class ConfigReader
+    class ConfigReader < Y2Network::ConfigReader
       include Yast::Logger
 
-      def initialize(_opts = {}); end
+      SECTIONS = [
+        :interfaces, :connections, :drivers, :routing, :dns, :hostname
+      ].freeze
 
       # @return [Y2Network::Config] Network configuration
       def config
         # NOTE: This code might be moved outside of the Sysconfig namespace, as it is generic.
         # NOTE: /etc/hosts cache - nothing to do with /etc/hostname
         Yast::Host.Read
 
-        interfaces = interfaces_reader.interfaces
-        s390_devices = interfaces_reader.s390_devices
-        connections = connections_configs_reader.connections(interfaces)
-        add_missing_interfaces(interfaces, connections)
+        initial_config = Config.new(source: :wicked)
 
-        routing_tables = find_routing_tables(interfaces)
+        result = SECTIONS.reduce(initial_config) do |current_config, section|
+          send("read_#{section}", current_config)
+        end
+
+        log.info "Sysconfig reader result: #{result.inspect}"
+        result
+      end
+
+    protected
+
+      # Reads the network interfaces
+      #
+      # @param config [Y2Network::Config] Initial configuration object
+      # @return [Y2Network::Config] A new configuration object including the interfaces
+      def read_interfaces(config)
+        config.update(
+          interfaces:   interfaces_reader.interfaces,
+          s390_devices: interfaces_reader.s390_devices
+        )
+      end
+
+      # Reads the connections
+      #
+      # @param config [Y2Network::Config] Initial configuration object
+      # @return [Y2Network::Config] A new configuration object including the connections
+      def read_connections(config)
+        connections = connection_configs_reader.connections(config.interfaces)
+        missing_interfaces = find_missing_interfaces(
+          connections, config.interfaces
+        )
+        config.update(
+          interfaces:  config.interfaces + missing_interfaces,
+          connections: connections
+        )
+      end
+
+      # Reads the drivers
+      #
+      # @param config [Y2Network::Config] Initial configuration object
+      # @return [Y2Network::Config] A new configuration object including the connections
+      def read_drivers(config)
+        config.update(drivers: interfaces_reader.drivers)
+      end
+
+      # Reads the routing information
+      #
+      # @param config [Y2Network::Config] Initial configuration object
+      # @return [Y2Network::Config] A new configuration object including the routing information
+      def read_routing(config)
+        routing_tables = find_routing_tables(config.interfaces)
         routing = Routing.new(
           tables:       routing_tables,
           forward_ipv4: sysctl_config_file.forward_ipv4,
           forward_ipv6: sysctl_config_file.forward_ipv6
         )
+        config.update(routing: routing)
+      end
 
-        result = Config.new(
-          interfaces:   interfaces,
-          connections:  connections,
-          s390_devices: s390_devices,
-          drivers:      interfaces_reader.drivers,
-          routing:      routing,
-          dns:          dns,
-          hostname:     hostname,
-          source:       :wicked
-        )
+      # Reads the DNS information
+      #
+      # @param config [Y2Network::Config] Initial configuration object
+      # @return [Y2Network::Config] A new configuration object including the DNS configuration
+      def read_dns(config)
+        config.update(dns: Y2Network::Wicked::DNSReader.new.config)
+      end
 
-        log.info "Sysconfig reader result: #{result.inspect}"
-        result
+      # Returns the Hostname configuration
+      #
+      # @param config [Y2Network::Config] Initial configuration object
+      # @return [Y2Network::Config] A new configuration object including the hostname information
+      def read_hostname(config)
+        config.update(hostname: Y2Network::Wicked::HostnameReader.new.config)
       end
 
     private
@@ -83,16 +134,17 @@ def interfaces_reader
 
       # @param interfaces [Y2Network::InterfacesCollection] Known interfaces
       # @return [Y2Network::ConnectionConfigsCollection] Connection configurations collection
-      def connections_configs_reader
+      def connection_configs_reader
         @connection_configs_reader ||= Y2Network::Wicked::ConnectionConfigsReader.new
       end
 
       # Adds missing interfaces from connections
       #
       # @param interfaces [Y2Network::InterfacesCollection] Known interfaces
       # @param interfaces [Y2Network::ConnectionConfigsCollection] Known interfaces
-      def add_missing_interfaces(interfaces, connections)
-        connections.each do |conn|
+      def find_missing_interfaces(connections, interfaces)
+        empty_collection = Y2Network::InterfacesCollection.new
+        connections.to_a.each_with_object(empty_collection) do |conn, all|
           interface = interfaces.by_name(conn.interface)
           next if interface
 
@@ -102,7 +154,7 @@ def add_missing_interfaces(interfaces, connections)
             else
               PhysicalInterface.new(conn.name, hardware: Hwinfo.for(conn.name))
             end
-          interfaces << missing_interface
+          all << missing_interface
         end
       end
 
@@ -153,20 +205,6 @@ def link_routes_to_interfaces(routes, interfaces)
         end
       end
 
-      # Returns the DNS configuration
-      #
-      # @return [Y2Network::DNS]
-      def dns
-        Y2Network::Wicked::DNSReader.new.config
-      end
-
-      # Returns the Hostname configuration
-      #
-      # @return [Y2Network::Hostname]
-      def hostname
-        Y2Network::Wicked::HostnameReader.new.config
-      end
-
       # Returns the Sysctl_Config file class
       #
       # @return [CFA::SysctlConfig]

test/y2network/config_test.rb

@@ -583,4 +583,22 @@
       end
     end
   end
+
+  describe "#update" do
+    let(:wlan0) { Y2Network::PhysicalInterface.new("wlan0") }
+    let(:wlan0_conn) { Y2Network::ConnectionConfig::Wireless.new }
+    let(:updated_interfaces) { Y2Network::InterfacesCollection.new([wlan0]) }
+    let(:updated_connections) { Y2Network::ConnectionConfigsCollection.new([wlan0_conn]) }
+
+    it "returns an updated configuration" do
+      new_config = config.update(
+        interfaces:  updated_interfaces,
+        connections: updated_connections,
+        source:      :network_manager
+      )
+      expect(config.interfaces).to eq(interfaces)
+      expect(new_config.interfaces).to eq(updated_interfaces)
+      expect(new_config.connections).to eq(updated_connections)
+    end
+  end
 end

test/y2network/interfaces_collection_test.rb

@@ -148,4 +148,14 @@
       expect(collection.free_names("eth", 3)).to eq(["eth1", "eth2", "eth3"])
     end
   end
+
+  describe "#+" do
+    let(:interfaces) { [eth0] }
+    let(:other) { Y2Network::InterfacesCollection.new([br0]) }
+
+    it "returns a collection containing all the objects" do
+      new_collection = collection + other
+      expect(new_collection.to_a).to eq([eth0, br0])
+    end
+  end
 end

test/y2network/wicked/config_reader_test.rb

@@ -19,6 +19,9 @@
 require_relative "../../test_helper"
 require "y2network/wicked/config_reader"
 require "y2network/wicked/dns_reader"
+require "y2network/connection_configs_collection"
+require "y2network/interfaces_collection"
+require "y2network/s390_group_devices_collection"
 
 describe Y2Network::Wicked::ConfigReader do
   subject(:reader) { described_class.new }
@@ -32,8 +35,8 @@
       conn.name = "eth0"
     end
   end
-  let(:connections) { [eth0_conn] }
-  let(:s390_devices) { [] }
+  let(:connections) { Y2Network::ConnectionConfigsCollection.new([eth0_conn]) }
+  let(:s390_devices) { Y2Network::S390GroupDevicesCollection.new([]) }
   let(:drivers) { Y2Network::Driver.new("virtio_net", "") }
   let(:routes_file) { instance_double(CFA::RoutesFile, load: nil, routes: []) }
   let(:dns_reader) { instance_double(Y2Network::Wicked::DNSReader, config: dns) }
@@ -74,11 +77,13 @@
 
     it "returns a configuration including connection configurations" do
       config = reader.config
-      expect(config.connections).to eq([eth0_conn])
+      expect(config.connections.to_a).to eq([eth0_conn])
     end
 
     context "when a connection for a not existing device is found" do
-      let(:connections) { [eth0_conn, extra_config] }
+      let(:connections) do
+        Y2Network::ConnectionConfigsCollection.new([eth0_conn, extra_config])
+      end
 
       context "and it is a virtual connection" do
         let(:extra_config) do