Merge 0012756 into 770c44c

package/yast2-ntp-client.changes

@@ -1,3 +1,10 @@
+-------------------------------------------------------------------
+Mon Aug 19 12:04:19 UTC 2019 - José Iván López González <jlopez@suse.com>
+
+- AutoYaST: improve importing when a comment contains empty lines.
+- This also fixes bsc#1142026.
+- 3.2.20
+
 -------------------------------------------------------------------
 Fri Aug 02 15:16:17 CEST 2019 - aschnell@suse.com
 

package/yast2-ntp-client.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-ntp-client
-Version:        3.2.19
+Version:        3.2.20
 Release:        0
 Summary:        YaST2 - NTP Client Configuration
 License:        GPL-2.0+

src/lib/cfa/ntp_conf.rb

@@ -98,7 +98,7 @@ def save
         matcher = Matcher.new(key: r.augeas[:key], value_matcher: r.augeas[:value])
         placer = BeforePlacer.new(matcher)
         comments.each do |comment|
-          data.add("#comment[]", comment.strip, placer)
+          data.add("#comment[]", comment, placer)
         end
       end
       super

src/modules/NtpClient.rb

@@ -1018,24 +1018,65 @@ def AutoPackages
 
   private
 
-    # Remove blank spaces in values
+    # Sanitizes values of a record (mainly, by removing blank spaces)
     #
-    # @note to avoid augeas parsing errors, comments should be sanitized by
-    #   removing blank spaces at the beginning and adding line break. Further
-    #   sanitation is done in NtpConf.save.
+    # @param record [Hash]
+    # @return [Hash]
     def sanitize_record(record)
       sanitized = record.dup
+
       sanitized.each do |key, value|
-        if key.include?("comment")
-          value.sub!(/^ */, "")
-          value << "\n" unless value.include?("\n")
-        elsif value.respond_to?(:strip!)
-          value.strip!
-        end
+        sanitized[key] = sanitize_attribute(key, value)
       end
+
       sanitized
     end
 
+    # Sanitizes the value of an attribute
+    #
+    # @note To avoid augeas parsing errors, each comment line should be sanitized by removing blank
+    #   spaces at the beginning and ensuring a line break, see {#sanitize_comment}.
+    #
+    # @param attribute [String]
+    # @param value [String]
+    #
+    # @return [String] sanitized value
+    def sanitize_attribute(attribute, value)
+      if attribute.include?("comment")
+        sanitize_comment(value)
+      elsif value.respond_to?(:strip)
+        value.strip
+      else
+        value
+      end
+    end
+
+    # Sanitizes each line from a comment
+    #
+    # @param comment [String]
+    # @return [String] sanitized comment
+    def sanitize_comment(comment)
+      lines = comment.split("\n")
+
+      sanitized_lines = lines.map { |l| sanitize_comment_line(l) }
+
+      sanitized_lines.compact.join
+    end
+
+    # Sanitizes a a comment line by removing blank spaces at the beginning and ensuring a line break
+    #
+    # @param line [String]
+    # @return [String, nil] sanitized line. It returns nil if the line only contains blank spaces.
+    def sanitize_comment_line(line)
+      sanitized_line = line.lstrip
+
+      return nil if sanitized_line.empty?
+
+      sanitized_line << "\n" unless sanitized_line.include?("\n")
+
+      sanitized_line
+    end
+
     # Set @ntp_policy according to NETCONFIG_NTP_POLICY value found in
     # /etc/sysconfig/network/config or with "auto" if not found
     #

test/cfa/ntp_conf_test.rb

@@ -94,19 +94,6 @@ def ntp_conf(file)
         expect(file.content.lines).to include("#test comment 2\n")
         expect(file.content.lines).to include("#test comment3\n")
       end
-
-      # see bsc #1142026
-      it "can write multi lines comments with unstripped whitespaces from autoyast profiles" do
-        record = CFA::NtpConf::ServerRecord.new
-        record.value = "4.pool.ntp.org"
-        record.comment = " test comment 4 \n \n test comment 5 "
-        ntp.records << record
-        expect(record.comment).to eq " test comment 4 \n \n test comment 5 "
-        ntp.save
-        expect(file.content.lines).to include("#test comment 4\n")
-        expect(file.content.lines).to include("#test comment 5\n")
-        expect(file.content.lines).to include("server 4.pool.ntp.org\n")
-      end
     end
 
     context "when a record is deleted" do

test/fixtures/autoyast/autoinst.xml

@@ -13,7 +13,11 @@
       </peer>
       <peer>
         <address>1.opensuse.pool.ntp.org</address>
-        <comment/>
+        <comment> # a comment with spaces
+
+          # and blank lines
+
+        </comment>
         <options>iburst</options>
         <type>server</type>
       </peer>

test/ntp_client_test.rb

@@ -61,6 +61,12 @@
           expect(record["comment"]).to eq "# a comment with spaces \n"
         end
 
+        # see bsc #1142026
+        it "sanitizes comments by removing blank lines" do
+          record = subject.ntp_records[1]
+          expect(record["comment"]).to eq "# a comment with spaces\n# and blank lines\n"
+        end
+
         it "reads the list of peers" do
           expect(subject.ntp_records.size).to eq 5
         end
@@ -147,7 +153,16 @@
 
       it "produces an output equivalent to #Import" do
         subject.Import(ntp_client_section)
-        expect(subject.Export()).to eq ntp_client_section
+
+        result = subject.Export
+
+        expect(result["synchronize_time"]).to eq(ntp_client_section["synchronize_time"])
+        expect(result["sync_interval"]).to eq(ntp_client_section["sync_interval"])
+        expect(result["start_at_boot"]).to eq(ntp_client_section["start_at_boot"])
+        expect(result["start_in_chroot"]).to eq(ntp_client_section["start_in_chroot"])
+        expect(result["ntp_policy"]).to eq(ntp_client_section["ntp_policy"])
+        expect(result["peers"].size).to eq(ntp_client_section["peers"].size)
+        expect(result["restricts"].size).to eq(ntp_client_section["restricts"].size)
       end
 
       it "clones without encountering a CFA object" do