Skip to content

Commit a7f89c2

Browse files
authored
refactor!: Make Role::{fixed,estimated}_replica_count standalone (#1247)
* refactor!: Make `Role::{fixed,estimated}_replica_count` standalone * changelog * Document why we use IntoIterator
1 parent 013bbf4 commit a7f89c2

2 files changed

Lines changed: 84 additions & 101 deletions

File tree

crates/stackable-operator/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
44

55
## [Unreleased]
66

7+
### Changed
8+
9+
- BREAKING: Make `Role::fixed_replica_count` and `Role::estimated_replica_count` functions standalone,
10+
so consumers don't need access to the `Role` struct ([#1247]).
11+
12+
[#1247]: https://github.com/stackabletech/operator-rs/pull/1247
13+
714
## [0.113.3] - 2026-07-07
815

916
### Fixed

crates/stackable-operator/src/role_utils.rs

Lines changed: 77 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -403,62 +403,6 @@ where
403403
}
404404
}
405405

406-
impl<Config, ConfigOverrides, RoleConfig, CommonConfig>
407-
Role<Config, ConfigOverrides, RoleConfig, CommonConfig>
408-
where
409-
RoleConfig: Default + JsonSchema + Serialize,
410-
CommonConfig: Default + JsonSchema + Serialize,
411-
ConfigOverrides: Default + JsonSchema + Serialize,
412-
{
413-
/// Returns [`Some<u32>`] in case the number of replicas is hard-coded to a certain value.
414-
///
415-
/// This is the case when all `replicas` are set to [`Some<u16>`], in which case they are simply
416-
/// summed.
417-
///
418-
/// The argument `zero_replicas_counting` is a safety mechanism, which allows the caller to
419-
/// decide if an explicit replica count of `0` should be treated as [`None`]. It also means that
420-
/// [`None`] is returned in case no roleGroups are configured at all.
421-
pub fn fixed_replica_count(&self, zero_replicas_counting: ZeroReplicasCounting) -> Option<u32> {
422-
// An empty role has no fixed replica count when zeros are treated as None.
423-
if zero_replicas_counting == ZeroReplicasCounting::TreatAsNone
424-
&& self.role_groups.is_empty()
425-
{
426-
return None;
427-
}
428-
429-
self.role_groups
430-
.values()
431-
.map(|rg| match rg.replicas {
432-
None => None,
433-
Some(0) if zero_replicas_counting == ZeroReplicasCounting::TreatAsNone => None,
434-
// The individual replicas are [`u16`]s, so a [`u32`] sum has plenty of space.
435-
Some(replicas) => Some(u32::from(replicas)),
436-
})
437-
.sum()
438-
}
439-
440-
/// Returns the estimated total number of replicas across all role groups.
441-
///
442-
/// Unlike [`Self::fixed_replica_count`], this always returns a value: a role group with an unset
443-
/// (i.e. [`None`]) replica count is assumed to run a single replica. Use this when a best-effort
444-
/// estimate is needed even though the exact number of replicas is not hard-coded.
445-
pub fn estimated_replica_count(&self) -> u32 {
446-
self.role_groups
447-
.values()
448-
.map(|rg| u32::from(rg.replicas.unwrap_or(1)))
449-
.sum()
450-
}
451-
}
452-
453-
/// How explicit zero (`0`) replicas on a role group should be counted
454-
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
455-
pub enum ZeroReplicasCounting {
456-
/// Treat them as what they are: `Some(0)`.
457-
TreatAsZero,
458-
/// Treat them as if the user configured [`None`].
459-
TreatAsNone,
460-
}
461-
462406
impl<Config, ConfigOverrides, RoleConfig>
463407
Role<Config, ConfigOverrides, RoleConfig, JavaCommonConfig>
464408
where
@@ -594,6 +538,62 @@ impl<K: Resource> Display for RoleGroupRef<K> {
594538
}
595539
}
596540

541+
/// Returns [`Some<u32>`] in case the number of replicas is hard-coded to a certain value.
542+
///
543+
/// This is the case when all `replicas` are set to [`Some<u16>`], in which case they are simply
544+
/// summed.
545+
///
546+
/// The argument `zero_replicas_counting` is a safety mechanism, which allows the caller to decide
547+
/// if an explicit replica count of `0` should be treated as [`None`]. It also means that [`None`]
548+
/// is returned in case no roleGroups are configured at all.
549+
//
550+
// Note: We are using a [`IntoIterator`] combined with `.peekable()` over [`ExactSizeIterator`] to
551+
// have minimal bound requirements on the caller.
552+
pub fn fixed_replica_count<I: IntoIterator<Item = Option<u16>>>(
553+
replicas: I,
554+
zero_replicas_counting: ZeroReplicasCounting,
555+
) -> Option<u32> {
556+
let mut replicas = replicas.into_iter().peekable();
557+
558+
// An empty role has no fixed replica count when zeros are treated as None.
559+
if zero_replicas_counting == ZeroReplicasCounting::TreatAsNone && replicas.peek().is_none() {
560+
return None;
561+
}
562+
563+
replicas
564+
.map(|replicas| match replicas {
565+
None => None,
566+
Some(0) if zero_replicas_counting == ZeroReplicasCounting::TreatAsNone => None,
567+
// The individual replicas are [`u16`]s, so a [`u32`] sum has plenty of space.
568+
Some(replicas) => Some(u32::from(replicas)),
569+
})
570+
.sum()
571+
}
572+
573+
/// Returns the estimated total number of replicas across all role groups.
574+
///
575+
/// Unlike [`fixed_replica_count`], this always returns a value: a role group with an unset (i.e.
576+
/// [`None`]) replica count is assumed to run a single replica. Use this when a best-effort estimate
577+
/// is needed even though the exact number of replicas is not hard-coded.
578+
//
579+
// Note: We are using a [`IntoIterator`] combined with `.peekable()` over [`ExactSizeIterator`] to
580+
// have minimal bound requirements on the caller.
581+
pub fn estimated_replica_count<I: IntoIterator<Item = Option<u16>>>(replicas: I) -> u32 {
582+
replicas
583+
.into_iter()
584+
.map(|replicas| u32::from(replicas.unwrap_or(1)))
585+
.sum()
586+
}
587+
588+
/// How explicit zero (`0`) replicas on a role group should be counted
589+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
590+
pub enum ZeroReplicasCounting {
591+
/// Treat them as what they are: `Some(0)`.
592+
TreatAsZero,
593+
/// Treat them as if the user configured [`None`].
594+
TreatAsNone,
595+
}
596+
597597
#[cfg(test)]
598598
mod tests {
599599
use std::collections::HashSet;
@@ -713,101 +713,77 @@ mod tests {
713713

714714
#[test]
715715
fn replica_counts_with_all_replicas_set() {
716-
let role = construct_role_with_replicas([Some(3), Some(2), Some(5)]);
716+
let replicas = [Some(3), Some(2), Some(5)];
717717

718718
assert_eq!(
719-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsZero),
719+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsZero),
720720
Some(10)
721721
);
722722
assert_eq!(
723-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsNone),
723+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsNone),
724724
Some(10)
725725
);
726-
assert_eq!(role.estimated_replica_count(), 10);
726+
assert_eq!(estimated_replica_count(replicas), 10);
727727
}
728728

729729
#[test]
730730
fn replica_counts_with_one_replica_unset() {
731-
let role = construct_role_with_replicas([Some(3), None, Some(2)]);
731+
let replicas = [Some(3), None, Some(2)];
732732

733733
assert_eq!(
734-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsZero),
734+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsZero),
735735
None
736736
);
737737
assert_eq!(
738-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsNone),
738+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsNone),
739739
None
740740
);
741-
assert_eq!(role.estimated_replica_count(), 6);
741+
assert_eq!(estimated_replica_count(replicas), 6);
742742
}
743743

744744
#[test]
745745
fn replica_counts_with_a_zero_replica() {
746-
let role = construct_role_with_replicas([Some(3), Some(0)]);
746+
let replicas = [Some(3), Some(0)];
747747

748748
assert_eq!(
749-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsZero),
749+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsZero),
750750
Some(3)
751751
);
752752
// With treat_zero_as_none the zero turns the whole count into None.
753753
assert_eq!(
754-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsNone),
754+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsNone),
755755
None
756756
);
757-
assert_eq!(role.estimated_replica_count(), 3);
757+
assert_eq!(estimated_replica_count(replicas), 3);
758758
}
759759

760760
#[test]
761-
fn replica_counts_with_a_single_zero_role_group() {
762-
let role = construct_role_with_replicas([Some(0)]);
761+
fn replica_counts_with_a_single_zero_role_groups_group() {
762+
let replicas = [Some(0)];
763763

764764
assert_eq!(
765-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsZero),
765+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsZero),
766766
Some(0)
767767
);
768768
assert_eq!(
769-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsNone),
769+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsNone),
770770
None
771771
);
772-
assert_eq!(role.estimated_replica_count(), 0);
772+
assert_eq!(estimated_replica_count(replicas), 0);
773773
}
774774

775775
#[test]
776776
fn replica_counts_without_role_groups() {
777-
let role = construct_role_with_replicas(vec![]);
777+
let replicas = [];
778778

779779
assert_eq!(
780-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsZero),
780+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsZero),
781781
Some(0)
782782
);
783783
assert_eq!(
784-
role.fixed_replica_count(ZeroReplicasCounting::TreatAsNone),
784+
fixed_replica_count(replicas, ZeroReplicasCounting::TreatAsNone),
785785
None
786786
);
787-
assert_eq!(role.estimated_replica_count(), 0);
788-
}
789-
790-
/// Builds a [`Role`] with one role group per passed `replicas` entry, so tests only need to
791-
/// care about the replica counts that [`Role::fixed_replica_count`] operates on.
792-
fn construct_role_with_replicas(
793-
replicas: impl IntoIterator<Item = Option<u16>>,
794-
) -> Role<(), EmptyConfigOverrides, GenericRoleConfig, GenericCommonConfig> {
795-
Role {
796-
config: CommonConfiguration::default(),
797-
role_config: GenericRoleConfig::default(),
798-
role_groups: replicas
799-
.into_iter()
800-
.enumerate()
801-
.map(|(index, replicas)| {
802-
(
803-
format!("role-group-{index}"),
804-
RoleGroup {
805-
config: CommonConfiguration::default(),
806-
replicas,
807-
},
808-
)
809-
})
810-
.collect(),
811-
}
787+
assert_eq!(estimated_replica_count(replicas), 0);
812788
}
813789
}

0 commit comments

Comments
 (0)