Merge pull request #164 from yast/fix-1st-stage-support-simple

Fix services management AutoYaST support

package/yast2-services-manager.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Wed Aug  1 10:50:29 UTC 2018 - igonzalezsosa@suse.com
+
+- Fix support to handle services during early 1st stage
+  (related to fate#319428).
+- 4.0.6
+
 -------------------------------------------------------------------
 Wed Jul 25 12:17:42 UTC 2018 - jlopez@suse.com
 

package/yast2-services-manager.spec

@@ -24,16 +24,16 @@
 ######################################################################
 
 Name:           yast2-services-manager
-Version:        4.0.5
+Version:        4.0.6
 Release:        0
 BuildArch:      noarch
 
 BuildRoot:      %{_tmppath}/%{name}-build
 Source0:        %{name}-%{version}.tar.bz2
 
 Requires:       ruby
-# Yast2::SystemService class
-Requires:       yast2 >= 4.0.80
+# Yast2::SystemService#build
+Requires:       yast2 >= 4.0.82
 Requires:       yast2-ruby-bindings >= 1.2.0
 # need new enough installation for its inst clients
 Conflicts:      yast2-installation < 3.1.32
@@ -44,8 +44,8 @@ Conflicts:      yast2-runlevel
 BuildRequires:  ruby
 BuildRequires:  update-desktop-files
 BuildRequires:  yast2-ruby-bindings >= 1.2.0
-# Yast2::SystemService class
-BuildRequires:  yast2 >= 4.0.80
+# Yast2::SystemService#build
+BuildRequires:  yast2 >= 4.0.82
 # Support for 'data' directory in rake install task
 BuildRequires:  rubygem(yast-rake) >= 0.1.7
 BuildRequires:  rubygem(rspec)

src/modules/services_manager.rb

@@ -54,11 +54,13 @@ def import data
     def reset
       ServicesManagerTarget.reset
       ServicesManagerService.reset
+      nil
     end
 
     def read
       ServicesManagerTarget.read
       ServicesManagerService.read
+      nil
     end
 
     # Errors are delegated to ServiceManagerService

src/modules/services_manager_service.rb

@@ -20,9 +20,14 @@ class ServicesManagerServiceClass < Module
       manual:    N_('Manually')
     }.freeze
 
+    # @!attribute [w] modified
+    #   @return [Boolean] Whether the module has been modified
+    attr_writer :modified
+
     # @return [Hash{String => Yast2::SystemService}]
     def services
-      @services ||= read
+      read if @services.nil?
+      @services
     end
 
     attr_writer :services
@@ -32,15 +37,18 @@ def services
     def initialize
       textdomain 'services-manager'
       @modified = false
-      @services = nil
     end
 
     # Finds a service
     #
-    # @param service [String] service name
+    # @param name [String] service name
     # @return [Yast2::SystemService, nil]
-    def find(service)
-      services[service]
+    def find(name)
+      return services[name] unless Stage.initial
+
+      # We are in inst-sys. So we cannot check for installed services but generate entries
+      # for these services if they do not exist yet.
+      services[name] = Yast2::SystemService.build(name)
     end
 
     # Sets whether service should be running after writing the configuration
@@ -143,17 +151,20 @@ def modified_services
       services.values.select(&:changed?)
     end
 
-    # Reloads services list
+    # Reloads the service list
+    #
+    # @return [Hash{String => Yast2::SystemService}]
+    # @see #read
     def reload
-      self.services = Y2ServicesManager::ServiceLoader.new.read
+      @services = nil
+      read
     end
 
     # Reads all services' data
     #
-    # @return [Hash{String => Settings}]
-    #   like "foo" => { enabled: false, loaded: true, ..., description: "Features OO" }
+    # @return [Hash{String => Yast2::SystemService}]
     def read
-      Y2ServicesManager::ServiceLoader.new.read
+      @services ||= Y2ServicesManager::ServiceLoader.new.read
     end
 
     # Resets the global status of the object
@@ -210,6 +221,8 @@ def import(profile)
     def save
       log.info "Saving systemd services..."
 
+      refresh_services if Stage.initial
+
       if modified_services.empty?
         log.info "No service has been changed, nothing to do..."
         return true
@@ -307,7 +320,7 @@ def start_mode_to_human(mode)
     #
     # @see Yast2::SystemService#changed?
     def modified
-      modified_services.any?
+      @modified || modified_services.any?
     end
 
     alias_method :modified?, :modified
@@ -323,12 +336,6 @@ def modified
     # @return [Boolean] false if the service does not exist,
     #   otherwise what the block returned
     def exists?(name)
-      if Stage.initial && !find(name)
-        # We are in inst-sys. So we cannot check for installed services but generate entries
-        # for these services if they still not exists.
-        services[name] = Y2ServicesManager::ServiceLoader::DEFAULT_SERVICE_SETTINGS.clone
-      end
-
       service = find(name)
       if service && block_given?
         yield service
@@ -409,6 +416,14 @@ def enable_or_disable(services)
       end
     end
 
+    # Refresh the services information
+    #
+    # This is vitally important during 1st stage, where services information was read
+    # too early (from the instsys and not from the installed system).
+    def refresh_services
+      services.values.each(&:refresh)
+    end
+
     publish({:function => :active,         :type => "boolean ()"              })
     publish({:function => :activate,       :type => "string (boolean)"        })
     publish({:function => :all,            :type => "map <string, map> ()"    })

test/services_manager_service_test.rb

@@ -22,7 +22,7 @@
 
 require_relative "test_helper"
 
-Yast.import "ServicesManagerService"
+Yast.import "ServicesManager"
 
 describe Yast::ServicesManagerServiceClass do
   subject { Yast::ServicesManagerServiceClass.new }
@@ -31,14 +31,14 @@
     instance_double(
       Yast2::SystemService, name: "cups", description: "CUPS", start: true, stop: true,
       state: "active", substate: "running", changed?: false, start_mode: :on_boot,
-      save: nil, errors: {}
+      save: nil, refresh: nil, errors: {}
     )
   end
 
   let(:dbus) do
     instance_double(
       Yast2::SystemService, name: "dbus", changed?: true, active?: true,
-      running?: true, save: nil, errors: {}
+      running?: true, refresh: nil, save: nil, errors: {}
     )
   end
 
@@ -56,7 +56,11 @@
   end
 
   describe "#services" do
-    it "returns the list of services from ServiceLoader" do
+    before do
+      allow(subject).to receive(:read).and_call_original
+    end
+
+    it "returns the list of services" do
       expect(subject.services).to eq(services)
     end
   end
@@ -251,9 +255,41 @@
     end
   end
 
-  describe "#reload"
+  describe "#read" do
+    it "loads the list of services from ServiceLoader" do
+      expect(loader).to receive(:read)
+      subject.read
+    end
+
+    context "when services are already read" do
+      before do
+        subject.read
+      end
+
+      it "does not try to read them again" do
+        expect(loader).to_not receive(:read)
+        subject.read
+      end
+    end
+  end
+
+  describe "#reload" do
+    it "loads the list of services from ServiceLoader" do
+      expect(loader).to receive(:read)
+      subject.reload
+    end
 
-  describe "#read"
+    context "when services are already read" do
+      before do
+        subject.reload
+      end
+
+      it "reads them again" do
+        expect(loader).to receive(:read)
+        subject.reload
+      end
+    end
+  end
 
   describe "#reset" do
     it "resets all services" do
@@ -523,6 +559,13 @@
       subject.save
     end
 
+    it "does not refresh services" do
+      expect(dbus).to_not receive(:refresh)
+      expect(cups).to_not receive(:refresh)
+      subject.save
+    end
+
+
     context "when a service registers an error" do
       before do
         allow(cups).to receive(:errors).and_return({activate: true})
@@ -536,6 +579,12 @@
     context "on 1st stage" do
       let(:initial) { true }
 
+      it "refresh services before saving them" do
+        expect(dbus).to receive(:refresh).ordered
+        expect(dbus).to receive(:save).ordered
+        subject.save
+      end
+
       it "saves all services not modifying the current status" do
         expect(dbus).to receive(:save).with(keep_state: true)
         subject.save
@@ -667,4 +716,32 @@
       end
     end
   end
+
+  describe "#modified" do
+    let(:services) { { "cups" => cups } }
+
+    context "when it has been marked as modified" do
+      before do
+        subject.modified = true
+      end
+
+      it "returns true" do
+        expect(subject.modified).to eq(true)
+      end
+    end
+
+    context "when a service has been changed" do
+      let(:services) { { "dbus" => dbus } }
+
+      it "returns true" do
+        expect(subject.modified).to eq(true)
+      end
+    end
+
+    context "when it has not been marked as modified or no service has been changed" do
+      it "returns false" do
+        expect(subject.modified).to eq(false)
+      end
+    end
+  end
 end

test/services_manager_test.rb

@@ -220,6 +220,34 @@ module Yast
         end
       end
     end
+
+    describe "#read" do
+      it "reads services and targets" do
+        expect(Yast::ServicesManagerService).to receive(:read)
+        expect(Yast::ServicesManagerTarget).to receive(:read)
+        subject.read
+      end
+
+      it "returns nil" do
+        allow(Yast::ServicesManagerService).to receive(:read)
+        allow(Yast::ServicesManagerTarget).to receive(:read)
+        expect(subject.read).to eq(nil)
+      end
+    end
+
+    describe "#reset" do
+      it "reads services and targets" do
+        expect(Yast::ServicesManagerService).to receive(:reset)
+        expect(Yast::ServicesManagerTarget).to receive(:reset)
+        subject.reset
+      end
+
+      it "returns nil" do
+        allow(Yast::ServicesManagerService).to receive(:reset)
+        allow(Yast::ServicesManagerTarget).to receive(:reset)
+        expect(subject.reset).to eq(nil)
+      end
+    end
   end
 
   context "Global public API" do