Y2Packager::License clean-up and update

library/packages/src/lib/y2packager/license.rb

@@ -19,6 +19,7 @@ module Y2Packager
   #
   # This class represents a license.
   class License
+    # Default language for licenses.
     DEFAULT_LANG = "en_US".freeze
 
     # @return [Boolean] whether the license has been accepted or not
@@ -27,47 +28,67 @@ class License
     # @return [Hash<String, String>] language -> content
     attr_reader :translations
 
-    # @return [Y2Packager::LicensesFetchers::Base] License fetcher object
-    attr_reader :fetcher
-
     alias_method :accepted?, :accepted
 
     class << self
       # Find a license for a given product
       #
-      # @param product_name [String]    Product's name
-      # @param source       [:rpm,:url] Source to get the license from. For the time being,
-      #   only :rpm is really supported.
+      # This method uses a cache to return the same license if it was already
+      # used for another product.
+      #
+      # @param product_name [String]   Product's name
+      # @param source       [:rpm,nil] Source to get the license from. For the time being,
+      #   only :rpm is supported.
+      # @param content      [String]   License content. If this argument is given, this
+      #   string is used as the license's content (and `source` is ignored).
       # @return [License]
-      def find(product_name, source, options = {})
+      def find(product_name, source: nil, content: nil)
         return cache[product_name] if cache[product_name]
 
+        fetcher = source ? LicensesFetchers.for(source, product_name) : nil
         # This could be done in the constructor.
-        fetcher = LicensesFetchers.for(source, product_name, options)
-        new_license = License.new(fetcher)
+        new_license = License.new(fetcher: fetcher, content: content)
         return unless new_license.id
 
         eq_license = cache.values.find { |l| l.id == new_license.id }
         license = eq_license || new_license
         cache[product_name] = license
       end
 
+      # Clean licenses cache
       def clear_cache
         @cache = nil
       end
 
-      private def cache
+    private
+
+      # Licenses cache
+      #
+      # @return [Hash<String,License>]
+      def cache
         @cache ||= {}
       end
     end
 
     # Constructor
     #
-    # @param options [Hash<String, String>]
-    def initialize(fetcher)
+    # This class should be able to use the proper fetcher (see Y2Packager::LicensesFetchers)
+    # in order to retrieve license content (including translations). However, for compatibility
+    # reasons, the constructor can receive a `content` that will be used as licence's
+    # content. The reason is that, in some parts of YaST, the license content/translations
+    # is retrieved in different ways. We might need to unify them.
+    #
+    # @param fetcher [:rpm,nil] Fetcher to retrieve licenses information.
+    # @param content [String]   License content. If this argument is given, this
+    #   string is used as the license's content (and `source` is ignored).
+    def initialize(fetcher: nil, content: nil)
       @accepted = false
-      @fetcher = fetcher
       @translations = {}
+      if content
+        add_content_for(DEFAULT_LANG, content)
+      else
+        @fetcher = fetcher
+      end
     end
 
     # License unique identifier
@@ -79,39 +100,32 @@ def id
       return @id if @id
       content = content_for(DEFAULT_LANG)
       return unless content
-      @id = Digest::MD5.hexdigest(content_for(DEFAULT_LANG))
+      @id = Digest::MD5.hexdigest(content)
     end
 
     # Return the license translated content for the given language
     #
-    # It will return the empty string ("") if the license does not exist
-    #
-    # @param [String] language
+    # @param lang [String] Contents' language
     # @return [String,nil] the license translated content or nil if not found
     def content_for(lang = DEFAULT_LANG)
       return @translations[lang] if @translations[lang]
+      return nil unless fetcher
       content = fetcher.license_content(lang)
       return add_content_for(lang, content) if content
-      content_for(DEFAULT_LANG) unless lang == DEFAULT_LANG
     end
 
-    # FIXME: Probably the locales should be obtained through the licenses
-    # translations, and probably could be initialized the first time a license
-    # is instantiated.
+    # Return license's available locales
+    #
+    # @return [String] List of available locales
     def locales
       fetcher.license_locales
     end
 
-    def license_confirmation_required?
-      # FIXME
-      true
-    end
-
     # Add the license translated content for the given language
     #
-    # @param lang [String]
-    # @param content [String]
-    # @return [String,nil] the license translated content or nil if not found
+    # @param lang    [String] Language to add the translation to
+    # @param content [String] Content to add
+    # @return [String] the license translated content
     def add_content_for(lang, content)
       @translations[lang] = content
     end
@@ -125,5 +139,9 @@ def accept!
     def reject!
       @accepted = false
     end
+
+  private
+    # @return [Y2Packager::LicensesFetchers::Base] License fetcher object
+    attr_reader :fetcher
   end
 end

library/packages/src/lib/y2packager/licenses_fetchers.rb

@@ -20,9 +20,9 @@ module LicensesFetchers
     # Return the licenses proper fetcher for a given source
     #
     # @param source [Symbol] :rpm or :url
-    def self.for(source, product_name, options = {})
+    def self.for(source, product_name)
       klass = const_get(source.to_s.capitalize)
-      klass.new(product_name, options)
+      klass.new(product_name)
     end
   end
 end

library/packages/test/y2packager/license_test.rb

@@ -16,7 +16,7 @@
 require "y2packager/license"
 
 describe Y2Packager::License do
-  subject(:license) { Y2Packager::License.new(fetcher) }
+  subject(:license) { Y2Packager::License.new(fetcher: fetcher) }
 
   let(:fetcher) do
     instance_double(Y2Packager::LicensesFetchers::Rpm, license_content: "license") 
@@ -28,15 +28,26 @@
   end
 
   describe ".find" do
-    it "returns a license for the given product" do
-      license = described_class.find("SLES", :rpm)
-      expect(license).to be_a(Y2Packager::License)
+    context "when a source is given" do
+      it "uses a fetcher to build the license" do
+        expect(Y2Packager::License).to receive(:new).with(fetcher: fetcher, content: nil)
+          .and_return(license)
+        expect(described_class.find("SLES", source: :rpm)).to be(license)
+      end
+    end
+
+    context "when some content is given" do
+      it "uses the content as license's content" do
+        expect(Y2Packager::License).to receive(:new).with(fetcher: nil, content: "Some content")
+          .and_return(license)
+        expect(described_class.find("SLES", content: "Some content")).to be(license)
+      end
     end
 
     context "when a license with the same ID was already retrieved" do
       it "returns the already retrieved instance" do
-        sles_license = described_class.find("SLES", :rpm)
-        sled_license = described_class.find("SLED", :rpm)
+        sles_license = described_class.find("SLES", source: :rpm)
+        sled_license = described_class.find("SLED", source: :rpm)
         expect(sles_license).to be(sled_license)
       end
     end
@@ -47,8 +58,8 @@
       end
 
       it "returns a new license instance" do
-        sles_license = described_class.find("SLES", :rpm)
-        sled_license = described_class.find("SLED", :rpm)
+        sles_license = described_class.find("SLES", source: :rpm)
+        sled_license = described_class.find("SLED", source: :rpm)
         expect(sles_license).to_not be(sled_license)
         expect(sles_license.id).to_not eq(sled_license.id)
       end
@@ -78,41 +89,67 @@
   describe "#content_for" do
     let(:lang) { "es_ES" }
 
-    context "when no language is given" do
-      it "returns the license content for the default language" do
-        expect(fetcher).to receive(:license_content).with(described_class::DEFAULT_LANG)
-          .and_return("dummy content")
-        expect(license.content_for).to eq("dummy content")
+    context "when some content was given in the constructor" do
+      subject(:license) { Y2Packager::License.new(content: "Some content") }
+
+      context "and no language is specified" do
+        it "returns the given content" do
+          expect(license.content_for).to eq("Some content")
+        end
+      end
+
+      context "and the default language is specified" do
+        it "returns the given content" do
+          expect(license.content_for()).to eq("Some content")
+        end
       end
-    end
 
-    it "returns the license content for the given language" do
-      expect(fetcher).to receive(:license_content).with(lang)
-        .and_return("dummy content")
-      expect(license.content_for(lang)).to eq("dummy content")
+      context "and a language different from default is specified" do
+        it "returns nil" do
+          expect(license.content_for("es_ES")).to be_nil
+        end
+      end
     end
 
-    context "when there is no translation for the given language" do
-      before do
-        allow(fetcher).to receive(:license_content).with(lang).and_return(nil)
-        allow(fetcher).to receive(:license_content).with(described_class::DEFAULT_LANG)
-          .and_return("dummy content")
+    context "when a fetcher was give in the constructor" do
+      subject(:license) { Y2Packager::License.new(fetcher: fetcher) }
+
+      context "when no language is given" do
+        it "returns the license content for the default language" do
+          expect(fetcher).to receive(:license_content).with(described_class::DEFAULT_LANG)
+            .and_return("dummy content")
+          expect(license.content_for).to eq("dummy content")
+        end
       end
 
-      it "returns the license content for the default language" do
+      it "returns the license content for the given language" do
+        expect(fetcher).to receive(:license_content).with(lang)
+          .and_return("dummy content")
         expect(license.content_for(lang)).to eq("dummy content")
       end
-    end
 
-    context "license content for the given languages was already retrieved" do
-      before do
-        allow(fetcher).to receive(:license_content).with(lang).and_return("content")
-        license.content_for(lang)
+      context "when there is no translation for the given language" do
+        before do
+          allow(fetcher).to receive(:license_content).with(lang).and_return(nil)
+          allow(fetcher).to receive(:license_content).with(described_class::DEFAULT_LANG)
+            .and_return("dummy content")
+        end
+
+        it "returns nil" do
+          expect(license.content_for(lang)).to be_nil
+        end
       end
 
-      it "returns cached content" do
-        expect(fetcher).to_not receive(:license_content)
-        expect(license.content_for(lang)).to eq("content")
+      context "license content for the given languages was already retrieved" do
+        before do
+          allow(fetcher).to receive(:license_content).with(lang).and_return("content")
+          license.content_for(lang)
+        end
+
+        it "returns cached content" do
+          expect(fetcher).to_not receive(:license_content)
+          expect(license.content_for(lang)).to eq("content")
+        end
       end
     end
   end