Skip to content

Commit

Permalink
Fix arithmetic side effects (#55)
Browse files Browse the repository at this point in the history 
* use saturating operations

* use explicit non-overflowing ops everywhere
  • Loading branch information
@remkop22
remkop22 authored Jul 11, 2023
1 parent e9f6a33 commit fefa6dc
Show file tree
Hide file tree
Showing 9 changed files with 202 additions and 390 deletions.
1 change: 0 additions & 1 deletion .cargo/config.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ rustflags = [
"-Wtrivial_numeric_casts",
"-Wunsafe_code",
"-Wunused_import_braces",
"-Wunused_qualifications",

# https://rust-lang.github.io/rust-clippy/master/index.html
"-Aclippy::doc_markdown",
Expand Down
5 changes: 2 additions & 3 deletions cli/src/cli.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::{
fmt::Display,
fs::File,
io::{stdin, Read},
ops::Mul,
path::PathBuf,
process::exit,
};
Expand All @@ -28,7 +27,7 @@ use ocpi_tariffs::{
tariff::{CompatibilityVat, OcpiTariff},
v211,
},
pricer::{DimensionReport, Pricer, Report},
pricer::{Dimension, DimensionReport, Pricer, Report},
types::{
electricity::Kwh,
money::{Money, Price},
Expand Down Expand Up @@ -427,7 +426,7 @@ impl<V: Display> PeriodTable<V> {

pub fn row<T>(&mut self, dim: &DimensionReport<T>, time: DateTime<Tz>)
where
T: Into<V> + Mul<Money, Output = Money> + Copy,
T: Into<V> + Dimension,
{
let cost = dim.cost();
self.rows.push(PeriodComponent {
Expand Down
145 changes: 103 additions & 42 deletions ocpi-tariffs/src/pricer.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::ops::Mul;

use crate::{
ocpi::{
cdr::Cdr,
Expand Down Expand Up @@ -81,14 +79,18 @@ impl Pricer {

let dimensions = Dimensions::new(components, &period.period_data);

total_charging_time += dimensions.time.volume.unwrap_or_else(HoursDecimal::zero);
total_charging_time = total_charging_time
.saturating_add(dimensions.time.volume.unwrap_or_else(HoursDecimal::zero));

total_energy += dimensions.energy.volume.unwrap_or_else(Kwh::zero);
total_energy =
total_energy.saturating_add(dimensions.energy.volume.unwrap_or_else(Kwh::zero));

total_parking_time += dimensions
.parking_time
.volume
.unwrap_or_else(HoursDecimal::zero);
total_parking_time = total_parking_time.saturating_add(
dimensions
.parking_time
.volume
.unwrap_or_else(HoursDecimal::zero),
);

periods.push(PeriodReport::new(period, dimensions));
}
Expand All @@ -97,38 +99,57 @@ impl Pricer {
let billed_energy = step_size.apply_energy(&mut periods, total_energy);
let billed_parking_time = step_size.apply_parking_time(&mut periods, total_parking_time);

let mut total_energy_cost = None;
let mut total_time_cost = None;
let mut total_parking_cost = None;
let mut total_fixed_cost = None;
let mut total_energy_cost: Option<Price> = None;
let mut total_time_cost: Option<Price> = None;
let mut total_parking_cost: Option<Price> = None;
let mut total_fixed_cost: Option<Price> = None;

for period in &periods {
let dimensions = &period.dimensions;

total_energy_cost = match (total_energy_cost, dimensions.energy.cost()) {
(None, None) => None,
(total, period) => Some(total.unwrap_or_default() + period.unwrap_or_default()),
(total, period) => Some(
total
.unwrap_or_default()
.saturating_add(period.unwrap_or_default()),
),
};

total_time_cost = match (total_time_cost, dimensions.time.cost()) {
(None, None) => None,
(total, period) => Some(total.unwrap_or_default() + period.unwrap_or_default()),
(total, period) => Some(
total
.unwrap_or_default()
.saturating_add(period.unwrap_or_default()),
),
};

total_parking_cost = match (total_parking_cost, dimensions.parking_time.cost()) {
(None, None) => None,
(total, period) => Some(total.unwrap_or_default() + period.unwrap_or_default()),
(total, period) => Some(
total
.unwrap_or_default()
.saturating_add(period.unwrap_or_default()),
),
};

total_fixed_cost = match (total_fixed_cost, dimensions.flat.cost()) {
(None, None) => None,
(total, period) => Some(total.unwrap_or_default() + period.unwrap_or_default()),
(total, period) => Some(
total
.unwrap_or_default()
.saturating_add(period.unwrap_or_default()),
),
};
}

let total_time = if let Some(first) = periods.first() {
let last = periods.last().unwrap();
(last.end_date_time - first.start_date_time).into()
(last
.end_date_time
.signed_duration_since(first.start_date_time))
.into()
} else {
HoursDecimal::zero()
};
Expand All @@ -140,9 +161,13 @@ impl Pricer {
total_energy_cost,
]
.into_iter()
.fold(None, |accum, next| match (accum, next) {
.fold(None, |accum: Option<Price>, next| match (accum, next) {
(None, None) => None,
_ => Some(accum.unwrap_or_default() + next.unwrap_or_default()),
_ => Some(
accum
.unwrap_or_default()
.saturating_add(next.unwrap_or_default()),
),
});

let report = Report {
Expand Down Expand Up @@ -203,23 +228,28 @@ impl StepSize {
}

fn duration_step_size(
total: HoursDecimal,
billed_volume: &mut HoursDecimal,
total_volume: HoursDecimal,
period_billed_volume: &mut HoursDecimal,
step_size: u64,
) -> HoursDecimal {
if step_size > 0 {
let total_seconds = total.as_num_seconds_decimal();
let total_seconds = total_volume.as_num_seconds_decimal();
let step_size = Number::from(step_size);

let priced_total_seconds = (total_seconds / step_size).ceil() * step_size;
let priced_total =
HoursDecimal::from_seconds_decimal(priced_total_seconds).expect("overflow");
let total_billed_volume = HoursDecimal::from_seconds_decimal(
total_seconds
.checked_div(step_size)
.ceil()
.saturating_mul(step_size),
)
.expect("overflow");

*billed_volume += priced_total - total;
let period_delta_volume = total_billed_volume.saturating_sub(total_volume);
*period_billed_volume = period_billed_volume.saturating_add(period_delta_volume);

priced_total
total_billed_volume
} else {
total
total_volume
}
}

Expand Down Expand Up @@ -259,29 +289,35 @@ impl StepSize {
}
}

fn apply_energy(&self, periods: &mut [PeriodReport], total: Kwh) -> Kwh {
fn apply_energy(&self, periods: &mut [PeriodReport], total_volume: Kwh) -> Kwh {
if let Some((energy_index, price)) = &self.energy {
if price.step_size > 0 {
let period = &mut periods[*energy_index];
let step_size = Number::from(price.step_size);

let volume = period
let period_billed_volume = period
.dimensions
.energy
.billed_volume
.as_mut()
.expect("dimension should have a volume");

let billed =
Kwh::from_watt_hours((total.watt_hours() / step_size).ceil() * step_size);
let total_billed_volume = Kwh::from_watt_hours(
total_volume
.watt_hours()
.checked_div(step_size)
.ceil()
.saturating_mul(step_size),
);

*volume += billed - total;
let period_delta_volume = total_billed_volume.saturating_sub(total_volume);
*period_billed_volume = period_billed_volume.saturating_add(period_delta_volume);

return billed;
return total_billed_volume;
}
}

total
total_volume
}
}

Expand Down Expand Up @@ -354,7 +390,11 @@ impl PeriodReport {
if accum.is_none() && next.is_none() {
None
} else {
Some(accum.unwrap_or_default() + next.unwrap_or_default())
Some(
accum
.unwrap_or_default()
.saturating_add(next.unwrap_or_default()),
)
}
})
}
Expand Down Expand Up @@ -421,17 +461,14 @@ where
}
}

impl<V> DimensionReport<V>
where
V: Mul<Money, Output = Money> + Copy,
{
impl<V: Dimension> DimensionReport<V> {
/// The total cost of this dimension during a period.
pub fn cost(&self) -> Option<Price> {
if let (Some(volume), Some(price)) = (self.billed_volume, self.price) {
let excl_vat = volume * price.price;
let excl_vat = volume.cost(price.price);

let incl_vat = match price.vat {
CompatibilityVat::Vat(Some(vat)) => Some(excl_vat * vat),
CompatibilityVat::Vat(Some(vat)) => Some(excl_vat.apply_vat(vat)),
CompatibilityVat::Vat(None) => Some(excl_vat),
CompatibilityVat::Unknown => None,
};
Expand All @@ -442,3 +479,27 @@ where
}
}
}

/// An OCPI tariff dimension
pub trait Dimension: Copy {
/// The cost of this dimension at a certain price.
fn cost(&self, price: Money) -> Money;
}

impl Dimension for Kwh {
fn cost(&self, price: Money) -> Money {
price.kwh_cost(*self)
}
}

impl Dimension for () {
fn cost(&self, price: Money) -> Money {
price
}
}

impl Dimension for HoursDecimal {
fn cost(&self, price: Money) -> Money {
price.time_cost(*self)
}
}
15 changes: 12 additions & 3 deletions ocpi-tariffs/src/session.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,15 +118,24 @@ impl InstantData {
fn next(&self, state: &PeriodData, date_time: DateTime) -> Self {
let mut next = self.clone();

next.total_duration = next.total_duration + (date_time - next.date_time);
let duration = date_time.signed_duration_since(next.date_time);

next.total_duration = next
.total_duration
.checked_add(&duration)
.unwrap_or_else(Duration::max_value);

next.date_time = date_time;

if let Some(duration) = state.charging_duration {
next.total_charging_duration = next.total_charging_duration + duration;
next.total_charging_duration = next
.total_charging_duration
.checked_add(&duration)
.unwrap_or_else(Duration::max_value);
}

if let Some(energy) = state.energy {
next.total_energy += energy;
next.total_energy = next.total_energy.saturating_add(energy);
}

next
Expand Down
50 changes: 13 additions & 37 deletions ocpi-tariffs/src/types/electricity.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
use std::{
fmt::Display,
ops::{Add, AddAssign, Mul, Sub},
};
use std::fmt::Display;

use rust_decimal_macros::dec;
use serde::{Deserialize, Serialize};

use super::number::Number;
Expand All @@ -18,12 +14,22 @@ impl Kwh {
Self(Number::default())
}

/// Saturating addition
pub fn saturating_add(self, other: Self) -> Self {
Self(self.0.saturating_add(other.0))
}

/// Saturating subtraction
pub fn saturating_sub(self, other: Self) -> Self {
Self(self.0.saturating_sub(other.0))
}

pub(crate) fn watt_hours(self) -> Number {
self.0 * Number::from(dec!(1000.0))
self.0.saturating_mul(Number::from(1000))
}

pub(crate) fn from_watt_hours(num: Number) -> Self {
Self(num / dec!(1000.0).into())
Self(num.checked_div(Number::from(1000)))
}

/// Round this number to the OCPI specified amount of decimals.
Expand All @@ -44,36 +50,6 @@ impl Display for Kwh {
}
}

impl Add for Kwh {
type Output = Self;

fn add(self, rhs: Self) -> Self::Output {
Self(self.0 + rhs.0)
}
}

impl AddAssign for Kwh {
fn add_assign(&mut self, rhs: Self) {
self.0 = self.0 + rhs.0;
}
}

impl Sub for Kwh {
type Output = Self;

fn sub(self, rhs: Self) -> Self::Output {
Self(self.0 - rhs.0)
}
}

impl Mul<Number> for Kwh {
type Output = Self;

fn mul(self, rhs: Number) -> Self::Output {
Self(self.0 * rhs)
}
}

/// A value of kilo watts.
#[derive(Debug, Deserialize, Serialize, PartialEq, Eq, Clone, Copy, PartialOrd, Ord)]
#[serde(transparent)]
Expand Down
Loading

0 comments on commit fefa6dc

Please sign in to comment.