fix: Scale rhs in math ops

Author: Techassi

crates/stackable-operator/src/quantity/cpu.rs

@@ -1,14 +1,11 @@
 use std::{
-    ops::{Deref, DerefMut},
-    str::FromStr,
+    iter::Sum,
+    ops::{Add, Deref},
 };
 
 use k8s_openapi::apimachinery::pkg::api::resource::Quantity as K8sQuantity;
 
-use crate::quantity::{
-    macros::{forward_from_impls, forward_op_impls},
-    DecimalByteMultiple, ParseQuantityError, Quantity, Suffix,
-};
+use crate::quantity::{macros::forward_quantity_impls, DecimalMultiple, Quantity, Suffix};
 
 #[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
 pub struct CpuQuantity(Quantity);
@@ -21,44 +18,35 @@ impl Deref for CpuQuantity {
     }
 }
 
-impl DerefMut for CpuQuantity {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.0
-    }
-}
-
-impl FromStr for CpuQuantity {
-    type Err = ParseQuantityError;
-
-    fn from_str(input: &str) -> Result<Self, Self::Err> {
-        let quantity = Quantity::from_str(input)?;
-        Ok(Self(quantity))
-    }
-}
-
 impl From<CpuQuantity> for K8sQuantity {
     fn from(value: CpuQuantity) -> Self {
         K8sQuantity(value.to_string())
     }
 }
 
-forward_from_impls!(Quantity, K8sQuantity, CpuQuantity);
-forward_op_impls!(
-    CpuQuantity(Quantity {
-        value: 0.0,
-        suffix: None,
-    }),
-    CpuQuantity,
-    usize,
-    f32,
-    f64
-);
+impl Sum for CpuQuantity {
+    fn sum<I: Iterator<Item = Self>>(iter: I) -> Self {
+        iter.fold(
+            CpuQuantity(Quantity {
+                value: 0.0,
+                suffix: Suffix::DecimalMultiple(DecimalMultiple::Empty),
+            }),
+            CpuQuantity::add,
+        )
+    }
+}
+
+forward_quantity_impls!(CpuQuantity, K8sQuantity, usize, f32, f64);
 
 impl CpuQuantity {
     pub fn from_millis(value: u32) -> Self {
         CpuQuantity(Quantity {
-            suffix: Some(Suffix::DecimalByteMultiple(DecimalByteMultiple::Milli)),
+            suffix: Suffix::DecimalMultiple(DecimalMultiple::Milli),
             value: value.into(),
         })
     }
+
+    pub fn scale_to(self, suffix: Suffix) -> Self {
+        Self(self.0.scale_to(suffix))
+    }
 }

crates/stackable-operator/src/quantity/macros.rs

@@ -1,31 +1,37 @@
-/// This macro is intended to be used to implement the From and TryFrom traits on specialized
-/// quantities.
-///
-/// Currently two specialized quantities exist: [`MemoryQuantity`][1] and [`CpuQuantity`][2].
-/// The traits are implemented by forwarding to the inner [`Quantity`][3] implementation. Both
-/// specialized quantities are just newtypes / wrappers around [`Quantity`][3].
-///
-/// [1]: super::MemoryQuantity
-/// [2]: super::CpuQuantity
-/// [3]: super::Quantity
+macro_rules! forward_quantity_impls {
+    ($for:ty, $kq:ty, $($on:ty),+) => {
+        $crate::quantity::macros::forward_from_impls!($for, $kq);
+        $crate::quantity::macros::forward_op_impls!($for, $($on),*);
+    };
+}
+
 macro_rules! forward_from_impls {
-    ($q:ty, $kq:ty, $for:ty) => {
-        impl From<$q> for $for {
-            fn from(quantity: $q) -> Self {
+    ($for:ty, $kq:ty) => {
+        impl ::std::str::FromStr for $for {
+            type Err = $crate::quantity::ParseQuantityError;
+
+            fn from_str(input: &str) -> Result<Self, Self::Err> {
+                let quantity = $crate::quantity::Quantity::from_str(input)?;
+                Ok(Self(quantity))
+            }
+        }
+
+        impl From<$crate::quantity::Quantity> for $for {
+            fn from(quantity: $crate::quantity::Quantity) -> Self {
                 Self(quantity)
             }
         }
 
         impl TryFrom<$kq> for $for {
-            type Error = ParseQuantityError;
+            type Error = $crate::quantity::ParseQuantityError;
 
             fn try_from(value: $kq) -> Result<Self, Self::Error> {
-                Ok(Self(Quantity::try_from(value)?))
+                Ok(Self($crate::quantity::Quantity::try_from(value)?))
             }
         }
 
         impl TryFrom<&$kq> for $for {
-            type Error = ParseQuantityError;
+            type Error = $crate::quantity::ParseQuantityError;
 
             fn try_from(value: &$kq) -> Result<Self, Self::Error> {
                 Ok(Self(Quantity::try_from(value)?))
@@ -35,7 +41,7 @@ macro_rules! forward_from_impls {
 }
 
 macro_rules! forward_op_impls {
-    ($acc:expr, $for:ty, $($on:ty),+) => {
+    ($for:ty, $($on:ty),+) => {
         impl ::std::ops::Add for $for {
             type Output = $for;
 
@@ -72,15 +78,6 @@ macro_rules! forward_op_impls {
             }
         }
 
-        impl ::std::iter::Sum for $for {
-            fn sum<I: Iterator<Item = Self>>(iter: I) -> Self {
-                iter.fold(
-                    $acc,
-                    <$for as ::std::ops::Add>::add,
-                )
-            }
-        }
-
         $(
             impl ::std::ops::Mul<$on> for $for {
                 type Output = $for;
@@ -99,6 +96,7 @@ macro_rules! forward_op_impls {
     };
 }
 
-/// HACK: Make the macros only available in this crate.
+// HACK: Make the macros only available in this crate.
 pub(crate) use forward_from_impls;
 pub(crate) use forward_op_impls;
+pub(crate) use forward_quantity_impls;

crates/stackable-operator/src/quantity/memory.rs

@@ -1,22 +1,11 @@
 use std::{
-    ops::{Deref, DerefMut},
-    str::FromStr,
+    iter::Sum,
+    ops::{Add, Deref},
 };
 
 use k8s_openapi::apimachinery::pkg::api::resource::Quantity as K8sQuantity;
 
-use crate::quantity::{
-    macros::{forward_from_impls, forward_op_impls},
-    BinaryByteMultiple, ParseQuantityError, Quantity, Suffix,
-};
-
-pub trait JavaHeap {
-    // TODO (@Techassi): Add proper error type
-    /// Formats the [`MemoryQuantity`] so that it can be used as a Java heap value.
-    ///
-    /// This function can fail, because the [`Quantity`] has to be scaled down to at most
-    fn to_java_heap_string(&self) -> Result<String, String>;
-}
+use crate::quantity::{macros::forward_quantity_impls, BinaryMultiple, Quantity, Suffix};
 
 #[derive(Clone, Copy, Debug, PartialEq, PartialOrd)]
 pub struct MemoryQuantity(Quantity);
@@ -29,53 +18,57 @@ impl Deref for MemoryQuantity {
     }
 }
 
-impl DerefMut for MemoryQuantity {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.0
-    }
-}
-
-impl FromStr for MemoryQuantity {
-    type Err = ParseQuantityError;
-
-    fn from_str(input: &str) -> Result<Self, Self::Err> {
-        let quantity = Quantity::from_str(input)?;
-        Ok(Self(quantity))
-    }
-}
-
 impl From<MemoryQuantity> for K8sQuantity {
     fn from(value: MemoryQuantity) -> Self {
         K8sQuantity(value.to_string())
     }
 }
 
-forward_from_impls!(Quantity, K8sQuantity, MemoryQuantity);
-forward_op_impls!(
-    MemoryQuantity(Quantity {
-        value: 0.0,
-        // TODO (@Techassi): This needs to be talked about. The previous implementation used Kibi
-        // here. Code which relies on that fact (for later scaling) will thus break.
-        suffix: None,
-    }),
-    MemoryQuantity,
-    usize,
-    f32,
-    f64
-);
+impl Sum for MemoryQuantity {
+    fn sum<I: Iterator<Item = Self>>(iter: I) -> Self {
+        iter.fold(
+            MemoryQuantity(Quantity {
+                value: 0.0,
+                suffix: Suffix::BinaryMultiple(BinaryMultiple::Kibi),
+            }),
+            MemoryQuantity::add,
+        )
+    }
+}
+
+forward_quantity_impls!(MemoryQuantity, K8sQuantity, usize, f32, f64);
 
 impl MemoryQuantity {
     pub const fn from_gibi(value: f64) -> Self {
         MemoryQuantity(Quantity {
-            suffix: Some(Suffix::BinaryByteMultiple(BinaryByteMultiple::Gibi)),
+            suffix: Suffix::BinaryMultiple(BinaryMultiple::Gibi),
             value,
         })
     }
 
     pub const fn from_mebi(value: f64) -> Self {
         MemoryQuantity(Quantity {
-            suffix: Some(Suffix::BinaryByteMultiple(BinaryByteMultiple::Mebi)),
+            suffix: Suffix::BinaryMultiple(BinaryMultiple::Mebi),
             value,
         })
     }
+
+    pub fn scale_to(self, suffix: Suffix) -> Self {
+        Self(self.0.scale_to(suffix))
+    }
+
+    pub fn ceil(self) -> Self {
+        Self(Quantity {
+            value: self.value.ceil(),
+            suffix: self.suffix,
+        })
+    }
+}
+
+pub trait JavaHeap {
+    // TODO (@Techassi): Add proper error type
+    /// Formats the [`MemoryQuantity`] so that it can be used as a Java heap value.
+    ///
+    /// This function can fail, because the [`Quantity`] has to be scaled down to at most
+    fn to_java_heap_string(&self) -> Result<String, String>;
 }

crates/stackable-operator/src/quantity/ops.rs

@@ -1,14 +1,13 @@
-use std::{
-    iter::Sum,
-    ops::{Add, AddAssign, Div, Mul, MulAssign, Sub, SubAssign},
-};
+use std::ops::{Add, AddAssign, Div, Mul, MulAssign, Sub, SubAssign};
 
 use crate::quantity::Quantity;
 
 impl Add for Quantity {
     type Output = Quantity;
 
     fn add(self, rhs: Quantity) -> Self::Output {
+        let rhs = rhs.scale_to(self.suffix);
+
         Self {
             value: self.value + rhs.value,
             ..self
@@ -18,14 +17,16 @@ impl Add for Quantity {
 
 impl AddAssign for Quantity {
     fn add_assign(&mut self, rhs: Quantity) {
-        self.value += rhs.value
+        *self = self.add(rhs)
     }
 }
 
 impl Sub for Quantity {
     type Output = Quantity;
 
     fn sub(self, rhs: Quantity) -> Self::Output {
+        let rhs = rhs.scale_to(self.suffix);
+
         Self {
             value: self.value - rhs.value,
             ..self
@@ -35,7 +36,7 @@ impl Sub for Quantity {
 
 impl SubAssign for Quantity {
     fn sub_assign(&mut self, rhs: Self) {
-        self.value -= rhs.value
+        *self = self.sub(rhs)
     }
 }
 
@@ -52,7 +53,7 @@ impl Mul<usize> for Quantity {
 
 impl MulAssign<usize> for Quantity {
     fn mul_assign(&mut self, rhs: usize) {
-        self.value *= rhs as f64
+        *self = self.mul(rhs)
     }
 }
 
@@ -69,7 +70,7 @@ impl Mul<f32> for Quantity {
 
 impl MulAssign<f32> for Quantity {
     fn mul_assign(&mut self, rhs: f32) {
-        self.value *= rhs as f64
+        *self = self.mul(rhs)
     }
 }
 
@@ -86,26 +87,15 @@ impl Mul<f64> for Quantity {
 
 impl MulAssign<f64> for Quantity {
     fn mul_assign(&mut self, rhs: f64) {
-        self.value *= rhs
+        *self = self.mul(rhs)
     }
 }
 
 impl Div for Quantity {
     type Output = f64;
 
     fn div(self, rhs: Self) -> Self::Output {
+        let rhs = rhs.scale_to(self.suffix);
         self.value / rhs.value
     }
 }
-
-impl Sum for Quantity {
-    fn sum<I: Iterator<Item = Self>>(iter: I) -> Self {
-        iter.fold(
-            Quantity {
-                value: 0.0,
-                suffix: None,
-            },
-            Quantity::add,
-        )
-    }
-}