Merge pull request #888 from yast/use_connection_config_in_builder

Use InterfaceType everywhere

src/include/network/lan/complex.rb

@@ -535,9 +535,7 @@ def input_done?(ret)
       true
     end
 
-    def MainDialog(init_tab, builder:)
-      @builder = builder
-
+    def MainDialog(init_tab)
       caption = _("Network Settings")
       widget_descr = {
         "tab" => CWMTab.CreateWidget(
@@ -588,8 +586,6 @@ def MainDialog(init_tab, builder:)
         break if input_done?(ret)
       end
 
-      @builder = nil
-
       ret
     end
 

src/include/network/lan/wizards.rb

@@ -119,10 +119,9 @@ def LanAutoSequence(mode)
     end
 
     def MainSequence(mode)
-      iface_builder = Y2Network::InterfaceConfigBuilder.new
       aliases = {
-        "global"   => -> { MainDialog("global", builder: iface_builder) },
-        "overview" => -> { MainDialog("overview", builder: iface_builder) }
+        "global"   => -> { MainDialog("global") },
+        "overview" => -> { MainDialog("overview") }
       }
 
       start = "overview"

src/lib/y2network/dialogs/add_interface.rb

@@ -53,7 +53,7 @@ def run
         end
 
         # TODO: use factory to get proper builder
-        builder = InterfaceConfigBuilder.for(@type_widget.result)
+        builder = InterfaceConfigBuilder.for(InterfaceType.from_short_name(@type_widget.result))
         proposed_name = Yast::LanItems.new_type_devices(@type_widget.result, 1).first
         builder.name = proposed_name
         Yast::NetworkInterfaces.Name = proposed_name

src/lib/y2network/interface.rb

@@ -27,7 +27,7 @@ class Interface
     attr_accessor :name
     # @return [String] Interface description
     attr_accessor :description
-    # @return [Symbol] Interface type
+    # @return [InterfaceType] Interface type
     attr_accessor :type
     # @return [Boolean]
     attr_reader :configured

src/lib/y2network/interface_config_builder.rb

@@ -33,11 +33,13 @@ class InterfaceConfigBuilder
 
     # Load fresh instance of interface config builder for given type.
     # It can be specialized type or generic, depending if specialized is needed.
-    # @param type [String] type of device
-    # TODO: it would be nice to have type of device as Enum and not pure string
+    # @param type [Y2Network::InterfaceType,String] type of device or its short name
     def self.for(type)
-      require "y2network/interface_config_builders/#{type}"
-      InterfaceConfigBuilders.const_get(type.to_s.capitalize).new
+      if !type.is_a?(InterfaceType)
+        type = InterfaceType.from_short_name(type) or raise "Unknown type #{type.inspect}"
+      end
+      require "y2network/interface_config_builders/#{type.file_name}"
+      InterfaceConfigBuilders.const_get(type.class_name).new
     rescue LoadError => e
       log.info "Specialed builder for #{type} not found. Fallbacking to default. #{e.inspect}"
       new(type: type)
@@ -51,7 +53,8 @@ def self.for(type)
     # Constructor
     #
     # Load with reasonable defaults
-    def initialize(type: nil)
+    # @param type [Y2Network::InterfaceType] type of device
+    def initialize(type:)
       @type = type
       @config = init_device_config({})
       @s390_config = init_device_s390_config({})
@@ -95,7 +98,7 @@ def save
     # do not modify anything
     # @return [Array<String>]
     def proposed_names
-      Yast::LanItems.new_type_devices(type, NEW_DEVICES_COUNT)
+      Yast::LanItems.new_type_devices(type.short_name, NEW_DEVICES_COUNT)
     end
 
     def valid_name?(name)
@@ -175,13 +178,15 @@ def device_sysconfig
       config = @config.dup
 
       # filter out options which are not needed
-      config.delete_if { |k, _| k =~ /WIRELESS.*/ } if type != "wlan"
-      config.delete_if { |k, _| k =~ /BONDING.*/ } if type != "bond"
-      config.delete_if { |k, _| k =~ /BRIDGE.*/ } if type != "br"
-      config.delete_if { |k, _| k =~ /TUNNEL.*/ } if !["tun", "tap"].include?(type)
-      config.delete_if { |k, _| k == "VLAN_ID" || k == "ETHERDEVICE" } if type != "vlan"
-      config.delete_if { |k, _| k == "IPOIB_MODE" } if type != "ib"
-      config.delete_if { |k, _| k == "INTERFACE" } if type != "dummy"
+      config.delete_if { |k, _| k =~ /WIRELESS.*/ } if type != InterfaceType::WIRELESS
+      config.delete_if { |k, _| k =~ /BONDING.*/ } if type != InterfaceType::BONDING
+      config.delete_if { |k, _| k =~ /BRIDGE.*/ } if type != InterfaceType::BRIDGE
+      if ![InterfaceType::TUN, InterfaceType::TAP].include?(type)
+        config.delete_if { |k, _| k =~ /TUNNEL.*/ }
+      end
+      config.delete_if { |k, _| k == "VLAN_ID" || k == "ETHERDEVICE" } if type != InterfaceType::VLAN
+      config.delete_if { |k, _| k == "IPOIB_MODE" } if type != InterfaceType::INFINIBAND
+      config.delete_if { |k, _| k == "INTERFACE" } if type != InterfaceType::DUMMY
       config.delete_if { |k, _| k == "IFPLUGD_PRIORITY" } if config["STARTMODE"] != "ifplugd"
 
       config.merge("_aliases" => lan_items_format_aliases)

src/lib/y2network/interface_config_builders/bond.rb → src/lib/y2network/interface_config_builders/bonding.rb

@@ -4,11 +4,11 @@
 
 module Y2Network
   module InterfaceConfigBuilders
-    class Bond < InterfaceConfigBuilder
+    class Bonding < InterfaceConfigBuilder
       include Yast::Logger
 
       def initialize
-        super(type: "bond")
+        super(type: InterfaceType::BONDING)
 
         # fill mandatory bond option
         @config["BOND_SLAVES"] = []

src/lib/y2network/interface_config_builders/br.rb → src/lib/y2network/interface_config_builders/bridge.rb

@@ -6,11 +6,11 @@
 
 module Y2Network
   module InterfaceConfigBuilders
-    class Br < InterfaceConfigBuilder
+    class Bridge < InterfaceConfigBuilder
       include Yast::Logger
 
       def initialize
-        super(type: "br")
+        super(type: InterfaceType::BRIDGE)
       end
 
       def already_configured?(devices)
@@ -31,7 +31,12 @@ def interfaces
         Config.find(:yast).interfaces
       end
 
-      NONBRIDGEABLE_TYPES = ["br", "tun", "usb", "wlan"].freeze
+      NONBRIDGEABLE_TYPES = [
+        InterfaceType::BRIDGE,
+        InterfaceType::TUN,
+        InterfaceType::USB,
+        InterfaceType::WIRELESS
+      ].freeze
       NONBRIDGEABLE_STARTMODE = ["nfsroot", "ifplugd"].freeze
 
       # Checks whether an interface can be bridged in particular bridge
@@ -56,8 +61,8 @@ def bridgeable?(iface)
         end
 
         # exclude interfaces of type unusable for bridge
-        if NONBRIDGEABLE_TYPES.include?(iface.type.short_name)
-          log.debug("Excluding (#{iface.name}) - is #{iface.type.short_name}")
+        if NONBRIDGEABLE_TYPES.include?(iface.type)
+          log.debug("Excluding (#{iface.name}) - is #{iface.type.name}")
           return false
         end
 

src/lib/y2network/interface_config_builders/dummy.rb

@@ -5,7 +5,7 @@ module Y2Network
   module InterfaceConfigBuilders
     class Dummy < InterfaceConfigBuilder
       def initialize
-        super(type: "dummy")
+        super(type: InterfaceType::DUMMY)
       end
 
       # It does all operations needed for sucessfull configuration export.

src/lib/y2network/interface_config_builders/ib.rb → src/lib/y2network/interface_config_builders/infiniband.rb

@@ -5,9 +5,9 @@
 
 module Y2Network
   module InterfaceConfigBuilders
-    class Ib < InterfaceConfigBuilder
+    class Infiniband < InterfaceConfigBuilder
       def initialize
-        super(type: "ib")
+        super(type: InterfaceType::INFINIBAND)
       end
 
       attr_writer :ipoib_mode

src/lib/y2network/interface_config_builders/vlan.rb

@@ -8,7 +8,7 @@ module Y2Network
   module InterfaceConfigBuilders
     class Vlan < InterfaceConfigBuilder
       def initialize
-        super(type: "vlan")
+        super(type: InterfaceType::VLAN)
       end
 
       def etherdevice

src/lib/y2network/interface_type.rb

@@ -28,10 +28,10 @@ class InterfaceType
     include Yast::I18n
 
     class << self
-      # @param const_name [String] Constant name
       # @param name       [String] Type name ("Ethernet", "Wireless", etc.)
       # @param short_name [String] Short name used in legacy code
-      def define_type(const_name, name, short_name)
+      def define_type(name, short_name)
+        const_name = name.upcase
         const_set(const_name, new(name, short_name))
         all << const_get(const_name)
       end
@@ -73,21 +73,33 @@ def to_human_string
       _(name)
     end
 
+    # Returns name for specialized class for this type e.g. for reader, write or builder
+    # @return [String]
+    def class_name
+      name.capitalize
+    end
+
+    # Returns name for file without suffix for this type e.g. for reader, write or builder
+    # @return [String]
+    def file_name
+      name.downcase
+    end
+
     # Define types constants
-    define_type "ETHERNET", N_("Ethernet"), "eth"
-    define_type "WIRELESS", N_("Wireless"), "wlan"
-    define_type "INFINIBAND", N_("Infiniband"), "ib"
-    define_type "BONDING", N_("Bonding"), "bond"
-    define_type "BRIDGE", N_("Bridge"), "br"
-    define_type "DUMMY", N_("Dummy"), "dummy"
-    define_type "VLAN", N_("VLAN"), "vlan"
-    define_type "TUN", N_("TUN"), "tun"
-    define_type "TAP", N_("TAP"), "tap"
-    define_type "USB", N_("USB"), "usb"
+    define_type N_("Ethernet"), "eth"
+    define_type N_("Wireless"), "wlan"
+    define_type N_("Infiniband"), "ib"
+    define_type N_("Bonding"), "bond"
+    define_type N_("Bridge"), "br"
+    define_type N_("Dummy"), "dummy"
+    define_type N_("VLAN"), "vlan"
+    define_type N_("TUN"), "tun"
+    define_type N_("TAP"), "tap"
+    define_type N_("USB"), "usb"
     # s390
-    define_type "QETH", N_("QETH"), "qeth"
-    define_type "LCS", N_("LCS"), "lcs"
-    define_type "HIPERSOCKETS", N_("HiperSockets"), "hsi"
-    define_type "FICON", N_("FICON"), "ficon"
+    define_type N_("QETH"), "qeth"
+    define_type N_("LCS"), "lcs"
+    define_type N_("HiperSockets"), "hsi"
+    define_type N_("FICON"), "ficon"
   end
 end

src/lib/y2network/sysconfig/connection_config_reader.rb

@@ -28,7 +28,7 @@ class ConnectionConfigReader
       # Constructor
       #
       # @param name [String] Interface name
-      # @param type [Symbol,nil] Interface type (:eth, :wlan, etc.); if the type is unknown,
+      # @param type [InterfaceType, string, nil] Interface type; if the type is unknown,
       #   `nil` can be used and it will be guessed from the configuration file is possible.
       #
       # @return [Y2Network::ConnectionConfig::Base]
@@ -43,11 +43,17 @@ def read(name, type)
 
       # Returns the class to handle a given interface type
       #
-      # @param type [Symbol]
+      # @param type [String, InterfaceType, nil] interface type or its short name
       # @return [Class] A class which belongs to the ConnectionConfigReaders module
       def find_handler_class(type)
-        require "y2network/sysconfig/connection_config_readers/#{type}"
-        ConnectionConfigReaders.const_get(type.to_s.capitalize)
+        return unless type
+
+        if !type.is_a?(InterfaceType)
+          t = InterfaceType.from_short_name(type) or raise "Unknown type #{type.inspect} #{type.class.inspect}"
+          type = t
+        end
+        require "y2network/sysconfig/connection_config_readers/#{type.file_name}"
+        ConnectionConfigReaders.const_get(type.class_name)
       rescue LoadError, NameError => e
         log.info "Unknown connection type: '#{type}'. " \
                  "Connection handler could not be loaded: #{e.message}"

src/lib/y2network/sysconfig/connection_config_readers/eth.rb → src/lib/y2network/sysconfig/connection_config_readers/ethernet.rb

@@ -24,7 +24,7 @@ module Sysconfig
     module ConnectionConfigReaders
       # This class is able to build a ConnectionConfig::Ethernet object given a
       # Sysconfig::InterfaceFile object.
-      class Eth
+      class Ethernet
         # @return [Y2Network::Sysconfig::InterfaceFile]
         attr_reader :file
 

src/lib/y2network/sysconfig/connection_config_readers/wlan.rb → src/lib/y2network/sysconfig/connection_config_readers/wireless.rb

@@ -24,7 +24,7 @@ module Sysconfig
     module ConnectionConfigReaders
       # This class is able to build a ConnectionConfig::Wireless object given a
       # Sysconfig::InterfaceFile object.
-      class Wlan
+      class Wireless
         # @return [Y2Network::Sysconfig::InterfaceFile]
         attr_reader :file
 

src/lib/y2network/sysconfig/interface_file.rb

@@ -158,9 +158,9 @@ def fetch(key)
       #
       # @todo Borrow logic from https://github.com/yast/yast-yast2/blob/6f7a789d00cd03adf62e00da34720f326f0e0633/library/network/src/modules/NetworkInterfaces.rb#L291
       #
-      # @return [Symbol] Interface's type depending on the file values
+      # @return [String] Interface's type depending on the file values
       def type
-        :eth
+        "eth"
       end
 
     private

src/lib/y2network/sysconfig/interfaces_reader.rb

@@ -19,6 +19,7 @@
 
 require "yast"
 require "y2network/interface"
+require "y2network/interface_type"
 require "y2network/virtual_interface"
 require "y2network/physical_interface"
 require "y2network/fake_interface"
@@ -107,7 +108,13 @@ def build_physical_interface(data)
         Y2Network::PhysicalInterface.new(data["dev_name"]).tap do |iface|
           iface.description = data["name"]
           type = data["type"] || Yast::NetworkInterfaces.GetTypeFromSysfs(iface.name)
-          iface.type = type.nil? ? :eth : type.to_sym
+          iface.type = case type
+          when nil then InterfaceType::ETHERNET
+          when ::String then InterfaceType.from_short_name(type)
+          when InterfaceType then type
+          else
+            raise "Unexpected value in interface type #{type.class.inspect}:#{type.inspect}"
+          end
         end
       end
 

src/lib/y2network/widgets/boot_protocol.rb

@@ -1,6 +1,8 @@
 require "yast"
 require "cwm/custom_widget"
 
+require "y2network/interface_type"
+
 Yast.import "DNS"
 Yast.import "Hostname"
 Yast.import "IP"
@@ -87,7 +89,7 @@ def contents
 
       def ibft_available?
         # IBFT only for eth, is it correct?
-        @settings.type == "eth"
+        @settings.type == Y2Network::InterfaceType::ETHERNET
       end
 
       def init

test/bond_test.rb

@@ -65,7 +65,7 @@
     # for selecting bridgable devices but imports interfaces
     # from LanItems internally
     let(:config) { Y2Network::Config.new(source: :test) }
-    let(:builder) { Y2Network::InterfaceConfigBuilder.for("bond") }
+    let(:builder) { Y2Network::InterfaceConfigBuilder.for(Y2Network::InterfaceType::BONDING) }
 
     before do
       allow(Y2Network::Config)

test/bridge_test.rb

@@ -103,7 +103,7 @@
     # for selecting bridgable devices but imports interfaces
     # from LanItems internally
     let(:config) { Y2Network::Config.new(source: :test) }
-    let(:builder) { Y2Network::InterfaceConfigBuilder.for("br") }
+    let(:builder) { Y2Network::InterfaceConfigBuilder.for(Y2Network::InterfaceType::BRIDGE) }
 
     it "returns list of slave candidates" do
       allow(Y2Network::Config)

test/cmdline_test.rb

@@ -86,6 +86,7 @@ def initialize
 
     before do
       allow(Yast::LanItems).to receive(:Items).and_return(items)
+      allow(Yast::LanItems).to receive(:GetCurrentType).and_return("eth")
       richtext = "test<br><ul><li>item1</li></ul>"
       allow(subject).to receive(:getConfigList).and_return(["0" => { "rich_descr" => richtext }])
     end