fix: remove option part for properties writer (#1218)

* fix: remove option part for properties writer (now takes <String,String> instead of <String,<Option<String>>

* fix: precommit

Author: maltesander

crates/stackable-operator/crds/Scaler.yaml

@@ -33,8 +33,8 @@ spec:
 
                   Upstream issues:
 
-                  - https://github.com/kubernetes/kubernetes/issues/105533
-                  - https://github.com/Arnavion/k8s-openapi/issues/136
+                  - <https://github.com/kubernetes/kubernetes/issues/105533>
+                  - <https://github.com/Arnavion/k8s-openapi/issues/136>
                 format: uint16
                 maximum: 65535.0
                 minimum: 0.0

crates/stackable-operator/src/v2/config_file_writer.rs

@@ -19,43 +19,37 @@ pub enum PropertiesWriterError {
 /// `property_1=value_1\nproperty_2=value_2\n`.
 pub fn to_java_properties_string<'a, T>(properties: T) -> Result<String, PropertiesWriterError>
 where
-    T: Iterator<Item = (&'a String, &'a Option<String>)>,
+    T: Iterator<Item = (&'a String, &'a String)>,
 {
     let mut output = Vec::new();
     write_java_properties(&mut output, properties)?;
     String::from_utf8(output).context(FromUtf8Snafu)
 }
 
-/// Writes Java properties to the given writer. A `None` value is written as an
-/// empty value (`key=`).
+/// Writes Java properties to the given writer.
 fn write_java_properties<'a, W, T>(writer: W, properties: T) -> Result<(), PropertiesWriterError>
 where
     W: Write,
-    T: Iterator<Item = (&'a String, &'a Option<String>)>,
+    T: Iterator<Item = (&'a String, &'a String)>,
 {
     let mut writer = PropertiesWriter::new(writer);
     for (k, v) in properties {
-        let property_value = v.as_deref().unwrap_or_default();
-        writer.write(k, property_value).context(PropertiesSnafu)?;
+        writer.write(k, v).context(PropertiesSnafu)?;
     }
     writer.flush().context(PropertiesSnafu)?;
     Ok(())
 }
 
 /// Converts properties into a Hadoop configuration XML, including the wrapping
-/// `<configuration>...</configuration>` elements. Properties with a `None` value
-/// are skipped. Keys and values are XML-escaped.
+/// `<configuration>...</configuration>` elements.
 pub fn to_hadoop_xml<'a, T>(properties: T) -> String
 where
-    T: Iterator<Item = (&'a String, &'a Option<String>)>,
+    T: Iterator<Item = (&'a String, &'a String)>,
 {
     let mut snippet = String::new();
     for (k, v) in properties {
-        let escaped_value = match v {
-            Some(value) => escape_str_attribute(value),
-            None => continue,
-        };
         let escaped_key = escape_str_attribute(k);
+        let escaped_value = escape_str_attribute(v);
         write!(
             snippet,
             "  <property>\n    <name>{escaped_key}</name>\n    <value>{escaped_value}</value>\n  </property>\n"
@@ -71,18 +65,18 @@ mod tests {
 
     use super::*;
 
-    fn xml(pairs: &[(&str, Option<&str>)]) -> String {
-        let map: BTreeMap<String, Option<String>> = pairs
+    fn xml(pairs: &[(&str, &str)]) -> String {
+        let map: BTreeMap<String, String> = pairs
             .iter()
-            .map(|(k, v)| (k.to_string(), v.map(str::to_string)))
+            .map(|(k, v)| (k.to_string(), v.to_string()))
             .collect();
         to_hadoop_xml(map.iter())
     }
 
-    fn props(pairs: &[(&str, Option<&str>)]) -> String {
-        let map: BTreeMap<String, Option<String>> = pairs
+    fn props(pairs: &[(&str, &str)]) -> String {
+        let map: BTreeMap<String, String> = pairs
             .iter()
-            .map(|(k, v)| (k.to_string(), v.map(str::to_string)))
+            .map(|(k, v)| (k.to_string(), v.to_string()))
             .collect();
         to_java_properties_string(map.iter()).unwrap()
     }
@@ -98,26 +92,16 @@ mod tests {
     #[test]
     fn hadoop_xml_renders_single_property() {
         assert_eq!(
-            xml(&[("fs.defaultFS", Some("hdfs://hdfs/"))]),
+            xml(&[("fs.defaultFS", "hdfs://hdfs/")]),
             "<?xml version=\"1.0\"?>\n<configuration>\n  \
              <property>\n    <name>fs.defaultFS</name>\n    \
              <value>hdfs://hdfs/</value>\n  </property>\n</configuration>"
         );
     }
 
-    #[test]
-    fn hadoop_xml_skips_none_values() {
-        assert_eq!(
-            xml(&[("kept", Some("1")), ("dropped", None)]),
-            "<?xml version=\"1.0\"?>\n<configuration>\n  \
-             <property>\n    <name>kept</name>\n    \
-             <value>1</value>\n  </property>\n</configuration>"
-        );
-    }
-
     #[test]
     fn hadoop_xml_escapes_special_characters() {
-        let rendered = xml(&[("k", Some("<a>&b"))]);
+        let rendered = xml(&[("k", "<a>&b")]);
         assert!(
             rendered.contains("<value>&lt;a&gt;&amp;b</value>"),
             "{rendered}"
@@ -126,18 +110,18 @@ mod tests {
 
     #[test]
     fn java_properties_renders_key_value() {
-        assert_eq!(props(&[("a", Some("1")), ("b", Some("2"))]), "a=1\nb=2\n");
+        assert_eq!(props(&[("a", "1"), ("b", "2")]), "a=1\nb=2\n");
     }
 
     #[test]
-    fn java_properties_renders_none_as_empty() {
-        assert_eq!(props(&[("none", None)]), "none=\n");
+    fn java_properties_renders_empty() {
+        assert_eq!(props(&[("empty", "")]), "empty=\n");
     }
 
     #[test]
     fn java_properties_escapes_colon_in_value() {
         assert_eq!(
-            props(&[("url", Some("file://this/location/file.abc"))]),
+            props(&[("url", "file://this/location/file.abc")]),
             "url=file\\://this/location/file.abc\n"
         );
     }