From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pdm-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id D765A1FF1B7
	for <inbox@lore.proxmox.com>; Mon, 14 Apr 2025 14:00:59 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id E723631A01;
	Mon, 14 Apr 2025 14:00:59 +0200 (CEST)
From: Gabriel Goller <g.goller@proxmox.com>
To: pdm-devel@lists.proxmox.com
Date: Mon, 14 Apr 2025 14:00:44 +0200
Message-Id: <20250414120046.486853-3-g.goller@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250414120046.486853-1-g.goller@proxmox.com>
References: <20250414120046.486853-1-g.goller@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL -0.021 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: [pdm-devel] [PATCH proxmox 2/3] section-config: remove DerefMut and
 make underlying HashMap private
X-BeenThere: pdm-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Datacenter Manager development discussion
 <pdm-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pdm-devel>, 
 <mailto:pdm-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pdm-devel/>
List-Post: <mailto:pdm-devel@lists.proxmox.com>
List-Help: <mailto:pdm-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel>, 
 <mailto:pdm-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Datacenter Manager development discussion
 <pdm-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pdm-devel-bounces@lists.proxmox.com
Sender: "pdm-devel" <pdm-devel-bounces@lists.proxmox.com>

Currently to insert a SectionConfig-section correctly we need to insert
it into the HashMap and insert the key in the order vector as well. This
is not ensure the SectionConfig gets written in the same order as it was
read. By implementing DerefMut and making the underlying HashMap `pub`
we allow to insert sections in the HashMap without inserting the key
into the the order. This caused a whole range of bugs where sections
where inserted, but never written (because we iterate over the order).

To improve this, we add our own insert and remove functions that
automatically add and remove the keys from the order. Also delete the
DerefMut impl and make the underlying Vec and HashMap private.

Note that this is a breaking change.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-section-config/src/typed.rs | 61 +++++++++++++++++++++++++----
 1 file changed, 53 insertions(+), 8 deletions(-)

diff --git a/proxmox-section-config/src/typed.rs b/proxmox-section-config/src/typed.rs
index a25798e8a29b..14a8d87e3425 100644
--- a/proxmox-section-config/src/typed.rs
+++ b/proxmox-section-config/src/typed.rs
@@ -143,8 +143,8 @@ fn to_pair(
 /// This dereferences to the section hash for convenience.
 #[derive(Debug, Clone)]
 pub struct SectionConfigData<T> {
-    pub sections: HashMap<String, T>,
-    pub order: Vec<String>,
+    sections: HashMap<String, T>,
+    order: Vec<String>,
 }
 
 impl<T> Default for SectionConfigData<T> {
@@ -156,6 +156,57 @@ impl<T> Default for SectionConfigData<T> {
     }
 }
 
+impl<T> SectionConfigData<T> {
+    /// Return a mutable reference to the value corresponding to the key.
+    pub fn get_mut(&mut self, key: &str) -> Option<&mut T> {
+        self.sections.get_mut(key)
+    }
+
+    /// Returns an iterator over the section key-value pairs in their original order.
+    pub fn iter(&self) -> Iter<'_, T> {
+        Iter {
+            sections: &self.sections,
+            order: self.order.iter(),
+        }
+    }
+
+    /// Insert a key-value pair in the section config.
+    ///
+    /// If the key does not exist in the map, it will be added to the end of
+    /// the order vector.
+    ///
+    /// # Returns
+    ///
+    /// * Some(value) - If the key was present, returns the previous value
+    /// * None - If the key was not present
+    pub fn insert(&mut self, key: String, value: T) -> Option<T> {
+        let previous_value = self.sections.insert(key.clone(), value);
+        // If there was no previous value, this is a new key, so add it to the order
+        if previous_value.is_none() {
+            self.order.push(key);
+        }
+        previous_value
+    }
+
+    /// Remove a key-value pair from the section config.
+    ///
+    /// If the key existed and was removed, also remove the key from the order vector
+    /// to maintain ordering consistency.
+    ///
+    /// # Returns
+    ///
+    /// * `Some(value)` - If the key was present, returns the value that was removed
+    /// * `None` - If the key was not present
+    pub fn remove(&mut self, key: &str) -> Option<T> {
+        let removed_value = self.sections.remove(key);
+        // only update the order vector if we actually removed something
+        if removed_value.is_some() {
+            self.order.retain(|k| k != key);
+        }
+        removed_value
+    }
+}
+
 impl<T: ApiSectionDataEntry + DeserializeOwned> TryFrom<RawSectionConfigData>
     for SectionConfigData<T>
 {
@@ -184,12 +235,6 @@ impl<T> std::ops::Deref for SectionConfigData<T> {
     }
 }
 
-impl<T> std::ops::DerefMut for SectionConfigData<T> {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.sections
-    }
-}
-
 impl<T: Serialize + ApiSectionDataEntry> TryFrom<SectionConfigData<T>> for RawSectionConfigData {
     type Error = serde_json::Error;
 
-- 
2.39.5



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel