Skip to content

Commit

Permalink
zulip: reduce code duplication
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidtwco
davidtwco committed Nov 11, 2024
1 parent 94fe475 commit d912ce0
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 135 deletions.
209 changes: 94 additions & 115 deletions src/schema.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,49 @@ impl Team {
Ok(lists)
}

fn expand_zulip_membership(
&self,
data: &Data,
common: &RawZulipCommon,
on_exclude_not_included: impl Fn(&str) -> Error,
) -> Result<Vec<ZulipMember>, Error> {
let mut members = if common.include_team_members {
self.members(data)?
} else {
HashSet::new()
};
for person in &common.extra_people {
members.insert(person.as_str());
}
for team in &common.extra_teams {
let team = data
.team(team)
.ok_or_else(|| format_err!("team {} is missing", team))?;
members.extend(team.members(data)?);
}
for excluded in &common.excluded_people {
if !members.remove(excluded.as_str()) {
return Err(on_exclude_not_included(excluded));
}
}

let mut final_members = Vec::new();
for member in members.iter() {
let member = data
.person(member)
.ok_or_else(|| format_err!("{} does not have a person configuration", member))?;
let member = match (member.github.clone(), member.zulip_id) {
(github, Some(zulip_id)) => ZulipMember::MemberWithId { github, zulip_id },
(github, _) => ZulipMember::MemberWithoutId { github },
};
final_members.push(member);
}
for &extra in &common.extra_zulip_ids {
final_members.push(ZulipMember::JustId(extra));
}
Ok(final_members)
}

pub(crate) fn raw_zulip_groups(&self) -> &[RawZulipGroup] {
&self.zulip_groups
}
Expand All @@ -379,46 +422,17 @@ impl Team {
let zulip_groups = &self.zulip_groups;

for raw_group in zulip_groups {
let mut group = ZulipGroup {
name: raw_group.name.clone(),
includes_team_members: raw_group.include_team_members,
members: Vec::new(),
};

let mut members = if raw_group.include_team_members {
self.members(data)?
} else {
HashSet::new()
};
for person in &raw_group.extra_people {
members.insert(person.as_str());
}
for team in &raw_group.extra_teams {
let team = data
.team(team)
.ok_or_else(|| format_err!("team {} is missing", team))?;
members.extend(team.members(data)?);
}
for excluded in &raw_group.excluded_people {
if !members.remove(excluded.as_str()) {
bail!("'{excluded}' was specifically excluded from the Zulip group '{}' but they were already not included", raw_group.name);
}
}

for member in members.iter() {
let member = data.person(member).ok_or_else(|| {
format_err!("{} does not have a person configuration", member)
})?;
let member = match (member.github.clone(), member.zulip_id) {
(github, Some(zulip_id)) => ZulipGroupMember::MemberWithId { github, zulip_id },
(github, _) => ZulipGroupMember::MemberWithoutId { github },
};
group.members.push(member);
}
for &extra in &raw_group.extra_zulip_ids {
group.members.push(ZulipGroupMember::JustId(extra));
}
groups.push(group);
groups.push(ZulipGroup(ZulipCommon {
name: raw_group.common.name.clone(),
includes_team_members: raw_group.common.include_team_members,
members: self.expand_zulip_membership(
data,
&raw_group.common,
|excluded| {
format_err!("'{excluded}' was specifically excluded from the Zulip group '{}' but they were already not included", raw_group.common.name)
},
)?,
}));
}
Ok(groups)
}
Expand All @@ -432,47 +446,17 @@ impl Team {
let zulip_streams = self.raw_zulip_streams();

for raw_stream in zulip_streams {
let mut stream = ZulipStream {
name: raw_stream.name.clone(),
members: Vec::new(),
};

let mut members = if raw_stream.include_team_members {
self.members(data)?
} else {
HashSet::new()
};
for person in &raw_stream.extra_people {
members.insert(person.as_str());
}
for team in &raw_stream.extra_teams {
let team = data
.team(team)
.ok_or_else(|| format_err!("team {} is missing", team))?;
members.extend(team.members(data)?);
}
for excluded in &raw_stream.excluded_people {
if !members.remove(excluded.as_str()) {
bail!("'{excluded}' was specifically excluded from the Zulip stream '{}' but they were already not included", raw_stream.name);
}
}

for member in members.iter() {
let member = data.person(member).ok_or_else(|| {
format_err!("{} does not have a person configuration", member)
})?;
let member = match (member.github.clone(), member.zulip_id) {
(github, Some(zulip_id)) => {
ZulipStreamMember::MemberWithId { github, zulip_id }
}
(github, _) => ZulipStreamMember::MemberWithoutId { github },
};
stream.members.push(member);
}
for &extra in &raw_stream.extra_zulip_ids {
stream.members.push(ZulipStreamMember::JustId(extra));
}
streams.push(stream);
streams.push(ZulipStream(ZulipCommon {
name: raw_stream.common.name.clone(),
includes_team_members: raw_stream.common.include_team_members,
members: self.expand_zulip_membership(
data,
&raw_stream.common,
|excluded| {
format_err!("'{excluded}' was specifically excluded from the Zulip stream '{}' but they were already not included", raw_stream.common.name)
},
)?,
}));
}
Ok(streams)
}
Expand Down Expand Up @@ -733,7 +717,7 @@ pub(crate) struct TeamList {

#[derive(serde_derive::Deserialize, Debug)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub(crate) struct RawZulipGroup {
pub(crate) struct RawZulipCommon {
pub(crate) name: String,
#[serde(default = "default_true")]
pub(crate) include_team_members: bool,
Expand All @@ -747,20 +731,18 @@ pub(crate) struct RawZulipGroup {
pub(crate) excluded_people: Vec<String>,
}

#[derive(serde_derive::Deserialize, Debug)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub(crate) struct RawZulipGroup {
#[serde(flatten)]
pub(crate) common: RawZulipCommon,
}

#[derive(serde_derive::Deserialize, Debug)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub(crate) struct RawZulipStream {
pub(crate) name: String,
#[serde(default = "default_true")]
pub(crate) include_team_members: bool,
#[serde(default)]
pub(crate) extra_people: Vec<String>,
#[serde(default)]
pub(crate) extra_zulip_ids: Vec<u64>,
#[serde(default)]
pub(crate) extra_teams: Vec<String>,
#[serde(default)]
pub(crate) excluded_people: Vec<String>,
#[serde(flatten)]
pub(crate) common: RawZulipCommon,
}

#[derive(Debug)]
Expand All @@ -780,52 +762,49 @@ impl List {
}

#[derive(Debug)]
pub(crate) struct ZulipGroup {
pub(crate) struct ZulipCommon {
name: String,
includes_team_members: bool,
members: Vec<ZulipGroupMember>,
members: Vec<ZulipMember>,
}

impl ZulipGroup {
impl ZulipCommon {
pub(crate) fn name(&self) -> &str {
&self.name
}

/// Whether the group includes the members of the team its associated
/// Whether the group/stream includes the members of the associated team?
pub(crate) fn includes_team_members(&self) -> bool {
self.includes_team_members
}

pub(crate) fn members(&self) -> &[ZulipGroupMember] {
pub(crate) fn members(&self) -> &[ZulipMember] {
&self.members
}
}

#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
pub(crate) enum ZulipGroupMember {
MemberWithId { github: String, zulip_id: u64 },
JustId(u64),
MemberWithoutId { github: String },
}

#[derive(Debug)]
pub(crate) struct ZulipStream {
name: String,
members: Vec<ZulipStreamMember>,
}
pub(crate) struct ZulipGroup(ZulipCommon);

impl ZulipStream {
pub(crate) fn name(&self) -> &str {
&self.name
impl std::ops::Deref for ZulipGroup {
type Target = ZulipCommon;
fn deref(&self) -> &Self::Target {
&self.0
}
}

pub(crate) fn members(&self) -> &[ZulipStreamMember] {
&self.members
#[derive(Debug)]
pub(crate) struct ZulipStream(ZulipCommon);

impl std::ops::Deref for ZulipStream {
type Target = ZulipCommon;
fn deref(&self) -> &Self::Target {
&self.0
}
}

#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
pub(crate) enum ZulipStreamMember {
pub(crate) enum ZulipMember {
MemberWithId { github: String, zulip_id: u64 },
JustId(u64),
MemberWithoutId { github: String },
Expand Down
16 changes: 7 additions & 9 deletions src/static_api.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use crate::data::Data;
use crate::schema::{
Bot, Email, Permissions, RepoPermission, TeamKind, ZulipGroupMember, ZulipStreamMember,
};
use crate::schema::{Bot, Email, Permissions, RepoPermission, TeamKind, ZulipMember};
use anyhow::{ensure, Context as _, Error};
use indexmap::IndexMap;
use log::info;
Expand Down Expand Up @@ -279,13 +277,13 @@ impl<'a> Generator<'a> {
members: members
.into_iter()
.filter_map(|m| match m {
ZulipGroupMember::MemberWithId { zulip_id, .. } => {
ZulipMember::MemberWithId { zulip_id, .. } => {
Some(v1::ZulipGroupMember::Id(zulip_id))
}
ZulipGroupMember::JustId(zulip_id) => {
ZulipMember::JustId(zulip_id) => {
Some(v1::ZulipGroupMember::Id(zulip_id))
}
ZulipGroupMember::MemberWithoutId { .. } => None,
ZulipMember::MemberWithoutId { .. } => None,
})
.collect(),
},
Expand All @@ -310,13 +308,13 @@ impl<'a> Generator<'a> {
members: members
.into_iter()
.filter_map(|m| match m {
ZulipStreamMember::MemberWithId { zulip_id, .. } => {
ZulipMember::MemberWithId { zulip_id, .. } => {
Some(v1::ZulipStreamMember::Id(zulip_id))
}
ZulipStreamMember::JustId(zulip_id) => {
ZulipMember::JustId(zulip_id) => {
Some(v1::ZulipStreamMember::Id(zulip_id))
}
ZulipStreamMember::MemberWithoutId { .. } => None,
ZulipMember::MemberWithoutId { .. } => None,
})
.collect(),
},
Expand Down
Loading

0 comments on commit d912ce0

Please sign in to comment.