[service] Improve feedback while reading software repositories

service/lib/dinstaller/software/manager.rb

@@ -88,18 +88,22 @@ def select_product(name)
       def probe
         logger.info "Probing software"
 
-        store_original_repos
-
         # as we use liveDVD with normal like ENV, lets temporary switch to normal to use its repos
         Yast::Stage.Set("normal")
 
-        start_progress(3)
-        Yast::PackageCallbacks.InitPackageCallbacks(logger)
-        progress.step("Initialize target repositories") { initialize_target_repos }
-        progress.step("Initialize sources") { add_base_repos }
-        progress.step("Making the initial proposal") { propose }
+        if repositories.empty?
+          start_progress(4)
+          store_original_repos
+          Yast::PackageCallbacks.InitPackageCallbacks(logger)
+          progress.step("Initializing target repositories") { initialize_target_repos }
+          progress.step("Initializing sources") { add_base_repos }
+        else
+          start_progress(2)
+        end
+
+        progress.step("Refreshing repositories metadata") { repositories.load }
+        progress.step("Calculating the software proposal") { propose }
 
-        @probed = true
         Yast::Stage.Set("initial")
       end
 
@@ -108,9 +112,8 @@ def initialize_target_repos
         import_gpg_keys
       end
 
+      # Updates the software proposal
       def propose
-        return unless repositories.available?
-
         proposal.base_product = @product
         proposal.languages = languages
         select_resolvables
@@ -119,10 +122,18 @@ def propose
         result
       end
 
+      # Returns the errors related to the software proposal
+      #
+      # * Repositories that could not be probed are reported as errors
+      # * If none of the repositories could be probed, do not report missing patterns and packages
+      #   problems as it does not make sense.
       def validate
-        return [ValidationError.new("No repositories are available")] unless repositories.available?
+        errors = repositories.disabled.map do |repo|
+          ValidationError.new("Could not read the repository #{repo.name}")
+        end
+        return errors if repositories.enabled.empty?
 
-        proposal.errors
+        errors + proposal.errors
       end
 
       def install
@@ -213,10 +224,8 @@ def add_base_repos
           else
             url = repo
           end
-          # TODO: report failing repositories
           repositories.add(url)
         end
-        repositories.refresh_all
       end
 
       REPOS_BACKUP = "/etc/zypp/repos.d.dinstaller.backup"

service/lib/dinstaller/software/proposal.rb

@@ -135,8 +135,10 @@ def select_base_product
 
       # Returns the errors from the attempt to create a proposal
       #
-      # Apart from extracting the errors from the result, it checks the packaging
-      # system for errors of the last solver execution
+      # It collects errors from:
+      #
+      # * The proposal result
+      # * The last solver execution
       #
       # @param proposal_result [Hash] Proposal result; it might contain a "warning" key with warning
       #   messages.

service/lib/dinstaller/software/repositories_manager.rb

@@ -34,22 +34,45 @@ def initialize
         @repositories = []
       end
 
+      # Adds a new repository
+      #
       # @param url [String] Repository URL
       def add(url)
         repositories << Repository.create(name: url, url: url)
       end
 
-      # Refreshes all the repositories
-      def refresh_all
-        repositories.each(&:refresh)
+      # Determines if there are registered repositories
+      #
+      # @return [Boolean] true if there are not repositories; false otherwise
+      def empty?
+        repositories.empty?
+      end
+
+      # Returns the enabled repositories
+      #
+      # @return [Array<Repository>]
+      def enabled
+        repositories.select(&:enabled?)
+      end
+
+      # Returns the disabled repositories
+      #
+      # @return [Array<Repository>]
+      def disabled
+        repositories.reject(&:enabled?)
       end
 
-      # Determines whether some repository is available
+      # Loads the repository metadata
       #
-      # @return [Boolean] true if at least one repository is available; false otherwise
-      # @see Repository#available?
-      def available?
-        repositories.any?(&:available?)
+      # As a side effect, it disables those repositories that cannot be read.
+      # The intentation is to prevent the proposal from trying to read them
+      # again.
+      def load
+        repositories.each(&:enable!)
+        repositories.each do |repo|
+          repo.disable! unless repo.refresh
+        end
+        Yast::Pkg.SourceLoad
       end
     end
   end

service/lib/dinstaller/software/repository.rb

@@ -31,16 +31,17 @@ module Software
     #
     # @see RepositoriesManager
     class Repository < Y2Packager::Repository
-      # Refreshes the repository metadata
-      def refresh
-        @available = Yast::Pkg.SourceRefreshNow(repo_id)
-      end
-
       # Determines whether a repository is available or not
       #
       # A repository is available when it is possible to read its metadata.
-      def available?
-        !!@available
+      def probed?
+        data = Yast::Pkg.SourceGeneralData(repo_id)
+        @probed = data["type"] != "NONE"
+      end
+
+      # @return [Boolean]
+      def refresh
+        Yast::Pkg.SourceRefreshNow(repo_id)
       end
     end
   end

service/test/dinstaller/software/manager_test.rb

@@ -37,9 +37,9 @@
   let(:repositories) do
     instance_double(
       DInstaller::Software::RepositoriesManager,
-      add:         nil,
-      refresh_all: nil,
-      available?:  true
+      add:    nil,
+      load:   nil,
+      empty?: true
     )
   end
   let(:proposal) do
@@ -48,7 +48,8 @@
       :base_product= => nil,
       calculate:        nil,
       :languages= =>    nil,
-      set_resolvables:  nil
+      set_resolvables:  nil,
+      packages_count:   "500 MB"
     )
   end
 
@@ -114,7 +115,7 @@
 
     it "registers the repository from config" do
       expect(repositories).to receive(:add).with(/tumbleweed/)
-      expect(repositories).to receive(:refresh_all)
+      expect(repositories).to receive(:load)
       subject.probe
     end
   end

service/test/dinstaller/software/repository_manager_test.rb

@@ -23,7 +23,15 @@
 require "dinstaller/software/repositories_manager"
 
 describe DInstaller::Software::RepositoriesManager do
-  let(:repo) { instance_double(DInstaller::Software::Repository) }
+  let(:repo) do
+    instance_double(
+      DInstaller::Software::Repository, enable!: nil, refresh: true, enabled?: true
+    )
+  end
+
+  let(:disabled_repo) do
+    instance_double(DInstaller::Software::Repository, enable!: nil, enabled?: false)
+  end
 
   describe "#add" do
     it "registers the repository in the packaging system" do
@@ -36,44 +44,55 @@
     end
   end
 
-  describe "#refresh_all" do
+  describe "#load" do
+    let(:repo1) do
+      instance_double(
+        DInstaller::Software::Repository, enable!: nil, disable!: false, refresh: false
+      )
+    end
+
     before do
       subject.repositories << repo
+      subject.repositories << repo1
+      allow(Yast::Pkg).to receive(:SourceLoad)
     end
 
     it "refreshes all the repositories" do
       expect(repo).to receive(:refresh)
-      subject.refresh_all
+      expect(repo1).to receive(:refresh)
+      subject.load
     end
-  end
 
-  describe "#available?" do
-    let(:repo1) do
-      instance_double(DInstaller::Software::Repository, available?: false)
+    it "disables the repositories that cannot be read" do
+      expect(repo1).to receive(:disable!)
+      subject.load
     end
-    let(:repo2) do
-      instance_double(DInstaller::Software::Repository, available?: false)
+
+    it "loads the repositories" do
+      expect(Yast::Pkg).to receive(:SourceLoad)
+      subject.load
     end
+  end
 
+  describe "#enabled" do
     before do
-      subject.repositories << repo1
-      subject.repositories << repo2
+      subject.repositories << repo
+      subject.repositories << disabled_repo
     end
 
-    context "when there no repositories available" do
-      it "returns false" do
-        expect(subject.available?).to eq(false)
-      end
+    it "returns the enabled repositories" do
+      expect(subject.enabled).to eq([repo])
     end
+  end
 
-    context "when at least one repository is available" do
-      let(:repo1) do
-        instance_double(DInstaller::Software::Repository, available?: true)
-      end
+  describe "#disabled" do
+    before do
+      subject.repositories << repo
+      subject.repositories << disabled_repo
+    end
 
-      it "returns true" do
-        expect(subject.available?).to eq(true)
-      end
+    it "returns the enabled repositories" do
+      expect(subject.disabled).to eq([disabled_repo])
     end
   end
 end

service/test/dinstaller/software/repository_test.rb

@@ -37,37 +37,4 @@
       subject.refresh
     end
   end
-
-  describe "#available?" do
-    let(:refresh_result) { true }
-
-    before do
-      allow(Yast::Pkg).to receive(:SourceRefreshNow).with(1)
-        .and_return(refresh_result)
-    end
-
-    context "when the metadata was not read" do
-      it "returns false" do
-        expect(subject.available?).to eq(false)
-      end
-    end
-
-    context "when the metadata was successfully read" do
-      before { subject.refresh }
-
-      it "returns true" do
-        expect(subject.available?).to eq(true)
-      end
-    end
-
-    context "when it was not possible to read the metadata" do
-      let(:refresh_result) { false }
-
-      before { subject.refresh }
-
-      it "returns false" do
-        expect(subject.available?).to eq(false)
-      end
-    end
-  end
 end

web/src/components/core/ProgressText.jsx

@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) [2023] SUSE LLC
+ *
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2 of the GNU General Public License as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, contact SUSE LLC.
+ *
+ * To contact SUSE LLC about this file by physical or electronic mail, you may
+ * find current contact information at www.suse.com.
+ */
+
+// @ts-check
+
+import React from "react";
+import { Spinner, Text } from "@patternfly/react-core";
+
+/**
+ * Progress description
+ *
+ * @component
+ *
+ * @param {object} props
+ * @param {string} [props.message] Progress message
+ * @param {number} [props.current] Current step
+ * @param {number} [props.total] Number of steps
+ */
+export default function ProgressText({ message, current, total }) {
+  const text = (current === 0) ? message : `${message} (${current}/${total})`;
+  return (
+    <div className="split">
+      <Spinner size="md" aria-label="Progress indicator" />
+      <Text>
+        {text}
+      </Text>
+    </div>
+  );
+}