From 920cc72c9f9c9304e0629cd267b50def1c8dd9f9 Mon Sep 17 00:00:00 2001
From: Elliu <elliu@hashi.re>
Date: Tue, 27 May 2025 10:25:41 +0200
Subject: [PATCH] UTILS: dkmap: allow more shapes of data to be stored
Use a IntoDKMap trait to provide the interface between the stored data and
the key management
---
lektor_nkdb/src/database/pool.rs | 97 ++++++++++++------------
lektor_nkdb/src/database/update.rs | 2 +-
lektor_repo/src/lib.rs | 2 +
lektor_utils/src/dkmap.rs | 116 ++++++++++++++++++++---------
lektor_utils/tests/dkmap.rs | 35 +++++----
5 files changed, 157 insertions(+), 95 deletions(-)
diff --git a/lektor_nkdb/src/database/pool.rs b/lektor_nkdb/src/database/pool.rs
index 75de9409..51b5534a 100644
--- a/lektor_nkdb/src/database/pool.rs
+++ b/lektor_nkdb/src/database/pool.rs
@@ -5,23 +5,19 @@
//! specific things like the [u64] to [Kid] / [Kara] mappings, do that in the epoch struct.
use crate::{kara::status::KaraState, strings, EpochData, KId, Kara, KaraStatus, RemoteKId};
-use anyhow::{Context as _, Result};
+use anyhow::{anyhow, Context as _, Result};
use futures::prelude::*;
-use lektor_utils::dkmap::DKMap;
+use lektor_utils::dkmap::{DKMap, IntoDKMapping};
use serde::{Deserialize, Serialize};
-/// ID mapping from local to remote. Note that there is a one-to-one relation between
-/// [RemoteKId] and [KId], even with metadata updates or file update.
-// State of the karas: virtual or not, if physical: filesize, filehash,...
+/// ID mapping from local to remote.
+/// Note that there is a one-to-many relation between from [KId] to [RemoteKId]
+/// State of the karas: virtual or not, if physical: filesize, filehash,...
type KaraMappingValue = (KId, RemoteKId, KaraState);
type KaraMapping = DKMap<KId, RemoteKId, KaraMappingValue>;
-pub trait IntoMapping: Sized {
- fn into_mapping(self) -> KaraMappingValue;
-}
-
-impl IntoMapping for &'_ Kara {
- fn into_mapping(self) -> KaraMappingValue {
+impl IntoDKMapping<KId, RemoteKId, KaraState> for &'_ Kara {
+ fn into_keys_value(&self) -> KaraMappingValue {
(
self.id,
self.remote.clone(),
@@ -33,41 +29,26 @@ impl IntoMapping for &'_ Kara {
},
)
}
-}
-
-impl IntoMapping for (KId, RemoteKId, KaraState) {
- fn into_mapping(self) -> KaraMappingValue {
- self
- }
-}
-
-impl IntoMapping for (RemoteKId, KaraState, KId) {
- fn into_mapping(self) -> KaraMappingValue {
- (self.2, self.0, self.1)
- }
-}
-impl IntoMapping for (KaraState, KId, RemoteKId) {
- fn into_mapping(self) -> KaraMappingValue {
- (self.1, self.2, self.0)
+ fn into_keys(&self) -> (KId, RemoteKId) {
+ (self.id, self.remote.clone())
}
-}
-impl IntoMapping for (KId, KaraState, RemoteKId) {
- fn into_mapping(self) -> KaraMappingValue {
- (self.0, self.2, self.1)
+ fn into_primary_key(&self) -> KId {
+ self.id
}
-}
-impl IntoMapping for (RemoteKId, KId, KaraState) {
- fn into_mapping(self) -> KaraMappingValue {
- (self.1, self.0, self.2)
+ fn into_secondary_key(&self) -> RemoteKId {
+ self.remote.clone()
}
-}
-impl IntoMapping for (KaraState, RemoteKId, KId) {
- fn into_mapping(self) -> KaraMappingValue {
- (self.2, self.1, self.0)
+ fn into_value(&self) -> KaraState {
+ match self.kara_status {
+ KaraStatus::Virtual => KaraState::Virtual,
+ // Don't try to discriminate PhysicalUnavailable and PhysicalAvailable here because
+ // it might take some time to resolve
+ _ => KaraState::PhysicalUnavailable,
+ }
}
}
@@ -118,13 +99,13 @@ impl Default for Pool {
}
}
-impl<M: IntoMapping> FromIterator<M> for Pool {
+impl<M: IntoDKMapping<KId, RemoteKId, KaraMappingValue>> FromIterator<M> for Pool {
fn from_iter<T: IntoIterator<Item = M>>(iter: T) -> Self {
let (mappings, ids): (Vec<KaraMappingValue>, Vec<u64>) = iter
.into_iter()
.map(|kara| {
- let (KId(id), rkid, remaining_values) = kara.into_mapping();
- ((KId(id), rkid, remaining_values), id)
+ let (KId(id), rkid, state) = kara.into_value();
+ ((KId(id), rkid, state), id)
})
.unzip();
@@ -149,17 +130,39 @@ impl Pool {
KId(ret)
}
- pub(crate) fn add_mapping(&mut self, mapping: impl IntoMapping) {
- self.kara_mapping.insert_or_update(mapping.into_mapping());
+ pub(crate) fn add_mapping(
+ &mut self,
+ mapping: impl IntoDKMapping<KId, RemoteKId, KaraMappingValue>,
+ ) {
+ self.kara_mapping.insert_or_update(mapping.into_value());
}
- pub(crate) fn update_state_from_kid(&mut self, id: KId, state: KaraState) -> Result<()> {
+ pub(crate) fn update_mapping(&mut self, mapping: KaraMappingValue) -> Result<()> {
self.kara_mapping
- .update(&id, state)
- .with_context(|| format!("failed to set state for kara {id}"))?;
+ .update(mapping.into_value())
+ .with_context(|| {
+ format!(
+ "failed to set state for kara {}",
+ mapping.into_primary_key()
+ )
+ })?;
Ok(())
}
+ // FIXME: delete?
+ pub(crate) fn update_state_from_kid(&mut self, id: KId, state: KaraState) -> Result<()> {
+ if let Some(mapping) = self.kara_mapping.get_k1(&id) {
+ let mut new = mapping.into_value();
+ new.2 = state;
+ self.kara_mapping
+ .update(new)
+ .with_context(|| format!("failed to set state for kara {id}"))?;
+ Ok(())
+ } else {
+ Err(anyhow!("update_state_from_kid with unknown kid"))
+ }
+ }
+
pub(crate) fn get_state(&self, kid: KId) -> Option<KaraState> {
Some(self.kara_mapping.get_k1(&kid)?.2)
}
diff --git a/lektor_nkdb/src/database/update.rs b/lektor_nkdb/src/database/update.rs
index 69a88ca7..43325f96 100644
--- a/lektor_nkdb/src/database/update.rs
+++ b/lektor_nkdb/src/database/update.rs
@@ -163,7 +163,7 @@ impl<'a, Storage: DatabaseStorage> UpdateHandler<'a, Storage> {
self.pool
.write()
.await
- .add_mapping((rkid.clone(), kid, state));
+ .add_mapping((kid, rkid.clone(), state));
Some((kid, rkid))
};
diff --git a/lektor_repo/src/lib.rs b/lektor_repo/src/lib.rs
index d4a7f2ce..8adc3a30 100644
--- a/lektor_repo/src/lib.rs
+++ b/lektor_repo/src/lib.rs
@@ -189,6 +189,8 @@ impl Repo {
{
log::error!("error downloading karaoke {kid}: {err}")
} else {
+ // FIXME: here, also pass rkid, to use the cheaper
+ // Pool.update_mapping instead of Pool.update_state_from_kid
return handler
.set_physical_available(kid, hash, filesize)
.await
diff --git a/lektor_utils/src/dkmap.rs b/lektor_utils/src/dkmap.rs
index 0397892e..d2a9bb87 100644
--- a/lektor_utils/src/dkmap.rs
+++ b/lektor_utils/src/dkmap.rs
@@ -2,25 +2,68 @@ use anyhow::{anyhow, Context as _, Result};
use serde::{Deserialize, Deserializer, Serialize};
use std::{cmp::Eq, collections::HashMap, hash::Hash, mem};
+// Use a custom trait and not just Into<(K1, K2)>, to allow for conversion copying only needed
+// values, and not all [Value]
+pub trait IntoDKMapping<K1, K2, Value>: Sized {
+ fn into_keys_value(&self) -> (K1, K2, Value);
+ fn into_keys(&self) -> (K1, K2);
+ fn into_primary_key(&self) -> K1;
+ fn into_secondary_key(&self) -> K2;
+ fn into_value(&self) -> Value;
+}
+
+// Let's be nice and provide a built-in implementation for simple tuples
+impl<K1, K2, Value> IntoDKMapping<K1, K2, (K1, K2, Value)> for (K1, K2, Value)
+where
+ K1: Hash + Eq + Copy,
+ K2: Hash + Eq + Clone,
+ Value: Clone,
+{
+ fn into_keys_value(&self) -> (K1, K2, (K1, K2, Value)) {
+ (
+ self.0,
+ self.1.clone(),
+ (self.0, self.1.clone(), self.2.clone()),
+ )
+ }
+
+ fn into_keys(&self) -> (K1, K2) {
+ (self.0, self.1.clone())
+ }
+
+ fn into_primary_key(&self) -> K1 {
+ self.0
+ }
+
+ fn into_secondary_key(&self) -> K2 {
+ self.1.clone()
+ }
+
+ fn into_value(&self) -> (K1, K2, Value) {
+ (self.0, self.1.clone(), self.2.clone())
+ }
+}
+
#[derive(Debug, Serialize, Clone, Default)]
-pub struct DKMap<K1: Hash + Eq, K2: Hash + Eq, Values> {
+pub struct DKMap<K1: Hash + Eq, K2: Hash + Eq, Value> {
#[serde(skip)]
primary_key_mapping: HashMap<K1, usize>,
#[serde(skip)]
secondary_key_mapping: HashMap<K2, Vec<usize>>,
- values: Vec<Values>,
+ values: Vec<Value>,
}
#[derive(Debug, Deserialize, Clone)]
-struct DKMapDeserializeProxy<Values> {
- values: Vec<Values>,
+struct DKMapDeserializeProxy<Value> {
+ values: Vec<Value>,
}
-impl<K1, K2, Values> DKMap<K1, K2, (K1, K2, Values)>
+impl<K1, K2, Value> DKMap<K1, K2, Value>
where
K1: Hash + Eq + Copy,
K2: Hash + Eq + Clone,
+ Value: IntoDKMapping<K1, K2, Value>,
{
pub fn with_capacity(capacity: usize) -> Self {
Self {
@@ -30,13 +73,13 @@ where
}
}
- pub fn get_k1(&self, key: &K1) -> Option<&(K1, K2, Values)> {
+ pub fn get_k1(&self, key: &K1) -> Option<&Value> {
self.primary_key_mapping
.get(key)
.and_then(|index| self.values.get(*index))
}
- pub fn get_k2(&self, key: &K2) -> Option<Vec<&(K1, K2, Values)>> {
+ pub fn get_k2(&self, key: &K2) -> Option<Vec<&Value>> {
self.secondary_key_mapping.get(key).map(|indexes| {
let mut ret = vec![];
for index in indexes {
@@ -46,29 +89,34 @@ where
})
}
- pub fn update(&mut self, key: &K1, values: Values) -> Result<(K1, K2, Values)> {
+ pub fn update(&mut self, value: Value) -> Result<Value> {
self.primary_key_mapping
- .get(key)
+ .get(&value.into_primary_key())
.map(|index| {
- let old_values = mem::replace(&mut self.values[*index].2, values);
- let (k1, k2, _) = &self.values[*index];
- (*k1, k2.clone(), old_values)
+ if value.into_keys() != self.values[*index].into_keys() {
+ None
+ } else {
+ Some(mem::replace(&mut self.values[*index], value.into_value()))
+ }
})
- .context("Called DKMap::update() on a non existing key")
+ .context("Called DKMap::update() on a non existing key")?
+ .context("Tried to update with an object that changes primary or secondary key")
}
/// Insert OR replace the value at value.0 (K1)
///
/// Note that neither K1 nor K2 will be updated for an already existing mapping, ever.
/// Only the remaining values will be
- pub fn insert_or_update(&mut self, value: (K1, K2, Values)) -> Option<(K1, K2, Values)> {
- let (k1, k2) = (value.0, value.1.clone());
+ pub fn insert_or_update(&mut self, value: Value) -> Option<Value> {
+ let (k1, k2) = value.into_keys();
match self.primary_key_mapping.get(&k1) {
- Some(index) => Some((
- k1,
- self.values[*index].1.clone(),
- mem::replace(&mut self.values[*index].2, value.2),
- )),
+ Some(index) => {
+ if value.into_keys() != self.values[*index].into_keys() {
+ None
+ } else {
+ Some(mem::replace(&mut self.values[*index], value.into_value()))
+ }
+ }
None => {
self.values.push(value);
let index = self.values.len() - 1;
@@ -83,8 +131,8 @@ where
}
/// Insert only. If the key K1 is already present, returns an Error
- pub fn insert_only(&mut self, value: (K1, K2, Values)) -> Result<()> {
- let (k1, k2) = (value.0, value.1.clone());
+ pub fn insert_only(&mut self, value: Value) -> Result<()> {
+ let (k1, k2) = value.into_keys();
match self.primary_key_mapping.get(&k1) {
Some(_) => Err(anyhow!("K1 already present in the DKMap")),
None => {
@@ -100,35 +148,35 @@ where
}
}
- pub fn iter(&self) -> impl Iterator<Item = &(K1, K2, Values)> + '_ {
+ pub fn iter(&self) -> impl Iterator<Item = &Value> + '_ {
self.values.iter()
}
}
-impl<K1, K2, Values> FromIterator<(K1, K2, Values)> for DKMap<K1, K2, (K1, K2, Values)>
+impl<K1, K2, Value> FromIterator<Value> for DKMap<K1, K2, Value>
where
K1: Hash + Eq + Copy,
K2: Hash + Eq + Clone,
- Values: Copy,
+ Value: IntoDKMapping<K1, K2, Value> + Clone,
{
- fn from_iter<T: IntoIterator<Item = (K1, K2, Values)>>(iter: T) -> Self {
+ fn from_iter<T: IntoIterator<Item = Value>>(iter: T) -> Self {
let iter = iter.into_iter();
let (min, max) = iter.size_hint();
let init = Self::with_capacity(max.unwrap_or(min));
- iter.fold(init, |mut ret, (key1, key2, values)| {
- ret.insert_or_update((key1, key2, values));
+ iter.fold(init, |mut ret, value| {
+ ret.insert_or_update(value);
ret
})
}
}
-impl<K1, K2, Values> IntoIterator for DKMap<K1, K2, (K1, K2, Values)>
+impl<K1, K2, Value> IntoIterator for DKMap<K1, K2, Value>
where
K1: Hash + Eq + Copy,
K2: Hash + Eq + Clone,
- Values: Copy,
+ Value: IntoDKMapping<K1, K2, Value> + Clone,
{
- type Item = (K1, K2, Values);
+ type Item = Value;
type IntoIter = std::vec::IntoIter<Self::Item>;
fn into_iter(self) -> Self::IntoIter {
@@ -136,17 +184,17 @@ where
}
}
-impl<'de, K1, K2, Values> Deserialize<'de> for DKMap<K1, K2, (K1, K2, Values)>
+impl<'de, K1, K2, Value> Deserialize<'de> for DKMap<K1, K2, Value>
where
K1: Hash + Eq + Copy + Deserialize<'de>,
K2: Hash + Eq + Clone + Deserialize<'de>,
- Values: Copy + Deserialize<'de>,
+ Value: IntoDKMapping<K1, K2, Value> + Clone + Deserialize<'de>,
{
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
- let proxy = DKMapDeserializeProxy::<(K1, K2, Values)>::deserialize(deserializer)?;
+ let proxy = DKMapDeserializeProxy::<Value>::deserialize(deserializer)?;
Ok(DKMap::from_iter(proxy.values))
}
}
diff --git a/lektor_utils/tests/dkmap.rs b/lektor_utils/tests/dkmap.rs
index d0aff9be..033e8596 100644
--- a/lektor_utils/tests/dkmap.rs
+++ b/lektor_utils/tests/dkmap.rs
@@ -29,9 +29,14 @@ fn test_from_iter() {
assert_eq!(map.get_k2(&5), Some(vec![&(4, 5, 6)]));
let map = TestDKMap::from_iter(vec![(1, 2, 3), (1, 5, 6)]);
- assert_eq!(map.get_k1(&1), Some(&(1, 2, 6)));
- assert_eq!(map.get_k2(&2), Some(vec![&(1, 2, 6)]));
+ assert_eq!(map.get_k1(&1), Some(&(1, 2, 3)));
+ assert_eq!(map.get_k2(&2), Some(vec![&(1, 2, 3)]));
assert_eq!(map.get_k2(&5), None);
+
+ let map = TestDKMap::from_iter(vec![(1, 2, 3), (10, 2, 6)]);
+ assert_eq!(map.get_k1(&1), Some(&(1, 2, 3)));
+ assert_eq!(map.get_k1(&10), Some(&(10, 2, 6)));
+ assert_eq!(map.get_k2(&2), Some(vec![&(1, 2, 3), &(10, 2, 6)]));
}
fn test_functional_empty_dkmap(mut map: TestDKMap) -> Result<()> {
@@ -45,20 +50,20 @@ fn test_functional_empty_dkmap(mut map: TestDKMap) -> Result<()> {
assert_eq!(map.get_k1(&1), Some(&(1, 2, 3)));
assert_eq!(map.get_k1(&4), Some(&(4, 5, 6)));
- assert_eq!(map.insert_or_update((4, 7, 8)), Some((4, 5, 6)));
+ assert_eq!(map.insert_or_update((4, 7, 8)), None);
// Make sure that primary key and secondary key are not updated
- assert_eq!(map.get_k1(&4), Some(&(4, 5, 8)));
+ assert_eq!(map.get_k1(&4), Some(&(4, 5, 6)));
let old_prim = 10;
let old = (old_prim, 11, 12);
assert_eq!(map.insert_or_update(old), None);
- assert_eq!(map.insert_or_update((old_prim, 20, 21)), Some(old));
+ assert_eq!(map.insert_or_update((old_prim, 20, 21)), None);
- assert_ok!(map.update(&1, 5));
- assert_eq!(map.get_k1(&1), Some(&(1, 2, 5)));
+ assert_err!(map.update((1, 99, 99)));
+ assert_eq!(map.get_k1(&1), Some(&(1, 2, 3)));
- assert_err!(map.update(&100, 5));
+ assert_err!(map.update((0, 0, 0)));
assert_eq!(map.get_k1(&101), None);
assert_eq!(map.insert_or_update((101, 102, 103)), None);
@@ -88,10 +93,14 @@ fn test_update() {
let mut map = TestDKMap::from_iter(vec![(1, 2, 3)]);
assert_eq!(map.get_k1(&1), Some(&(1, 2, 3)));
- assert_ok!(map.update(&1, 5));
+ assert_err!(map.update((99, 99, 99)));
+ assert_eq!(map.get_k1(&1), Some(&(1, 2, 3)));
+
+ let old = map.update((1, 2, 5));
+ assert_eq!(old.unwrap(), (1, 2, 3));
assert_eq!(map.get_k1(&1), Some(&(1, 2, 5)));
- assert_err!(map.update(&2, 5));
+ assert_err!(map.update((100, 100, 100)));
}
#[test]
@@ -101,8 +110,8 @@ fn test_get() {
assert_eq!(map.get_k1(&4), Some(&(4, 2, 6)));
assert_eq!(map.get_k2(&2), Some(vec![&(1, 2, 3), &(4, 2, 6)]));
- assert_ok!(map.update(&1, 10));
- assert_eq!(map.get_k2(&2), Some(vec![&(1, 2, 10), &(4, 2, 6)]));
+ assert_err!(map.update((1, 99, 10)));
+ assert_eq!(map.get_k2(&2), Some(vec![&(1, 2, 3), &(4, 2, 6)]));
}
#[test]
@@ -130,7 +139,7 @@ fn test_into_iter() {
assert_eq!(map_iter.next(), Some((4, 5, 6)));
let mut map_iter = TestDKMap::from_iter(vec![(1, 2, 3), (1, 5, 6)]).into_iter();
- assert_eq!(map_iter.next(), Some((1, 2, 6)));
+ assert_eq!(map_iter.next(), Some((1, 2, 3)));
assert_eq!(map_iter.next(), None);
}
--
GitLab