GitRust GitRust
Connexion

eric / rustwarden

0033-fediverse-delete-actor-hooks-unification.md 16019 octets

RFC 0033 — Unifier l'API apply_delete_actor* derrière DeleteActorHooks

  • Auteur : EBO
  • Date : 2026-05-17
  • Statut : Implemented (branche fedivers, 2026-05-17)
  • Crate cible : rustwarden-fediverse
  • Type : refactor additif (zero breaking change pour les consommateurs existants)
  • RFC associée : suite directe de RFC 0032

Summary

Introduire une struct DeleteActorHooks<'a> et un point d'entrée canonique FediverseSignalService::apply_delete_actor_with_hooks qui accepte simultanément un observer (RFC 0032) et un ActiveTokenIterator (RFC 0018), et faire des trois variantes existantes des wrappers fins (~3 lignes chacun) qui délèguent à cet entrypoint.

Aucun consommateur n'a à modifier son code : les trois signatures actuelles (apply_delete_actor, apply_delete_actor_observed, apply_delete_actor_with_iterator) sont préservées à l'octet près.

Motivation

Le problème laissé par RFC 0032

RFC 0032 a livré apply_delete_actor_observed(db, ap_id, observer) pour permettre aux consommateurs de cascader le cleanup local après un Delete{Actor} distant. Côté JWT, RFC 0018 avait déjà livré apply_delete_actor_with_iterator(db, ap_id, iterator) pour blacklister les JWT actifs d'une remote identity supprimée.

Les deux ne se composent pas. Un consommateur qui veut et purger ses artefacts locaux (SSH keys, PATs, etc.) et blacklister les JWT distribués à cette identity doit aujourd'hui :

// Option 1 — double appel, double SELECT, semantics dupliquée
FediverseSignalService::apply_delete_actor_observed(db, ap_id, observer).await?;
FediverseSignalService::apply_delete_actor_with_iterator(db, ap_id, iterator).await?;
// → la deuxième appel voit `disabled_at IS NOT NULL` et retourne rows_disabled=0,
//   donc le iterator n'est jamais consulté. Cassé.

// Option 2 — réimplémenter à la main côté consommateur. Anti-pattern.

Il n'existe aucun chemin correct sans modifier le framework. La RFC 0032 a explicitement reconnu cette dette dans sa section Drawbacks :

Variante double apply_delete_actor / apply_delete_actor_observed : duplique la surface. À débattre upstream.

Pourquoi maintenant

  • Gitrust va consommer les deux capacités dès le Lot 6 (cleanup SSH + invalidation JWT). Sans cette RFC, gitrust devra copier-coller la logique d'upsert/idempotence côté consommateur.
  • La crate fediverse est en experimental-fediverse, donc on peut encore restructurer les exports sans bump majeur.
  • La fenêtre est étroite : chaque nouveau consommateur qui se cale sur l'API actuelle ancrera la duplication.

État actuel de l'API service

FonctionObserver ?Iterator ?Idempotent ?Sémantique
apply_delete_actor❌❌✅Flip disabled_at
apply_delete_actor_observed (RFC 0032)✅❌✅+ on_delete_actor
apply_delete_actor_with_iterator (RFC 0018)❌✅✅+ blacklist JWTs
(manquant)✅✅✅tout combiné

La 4ᵉ ligne est ce qui manque. La création d'une fonction apply_delete_actor_with_iterator_and_observer (variante combinée) est la solution naïve — refusée plus bas.

API proposée

1. Nouvelle struct DeleteActorHooks

/// Hooks branchables sur une suppression d'actor distante.
///
/// Tous les champs sont `Option` avec un default `None` (no-op). Le
/// pattern recommandé d'instanciation est le struct literal avec
/// `..Default::default()` pour garantir la rétro-compatibilité quand
/// de nouveaux hooks seront ajoutés à cette struct dans des RFCs
/// futures :
///
/// ```rust,ignore
/// let hooks = DeleteActorHooks {
///     observer: Some(&my_observer),
///     ..Default::default()
/// };
/// ```
///
/// La struct **n'est pas** `#[non_exhaustive]` : la pattern
/// `..Default::default()` suffit à protéger les call-sites contre
/// l'ajout de nouveaux champs, sans bloquer la construction par struct
/// literal dans les crates consommatrices.
#[derive(Default)]
pub struct DeleteActorHooks<'a> {
    /// Observer notifié post-commit (cf. RFC 0032). `None` ⇒ aucun
    /// hook ne tire.
    pub observer: Option<&'a dyn FediverseEventObserver>,
    /// Énumérateur des JWT actifs à blacklister (cf. RFC 0018).
    /// `None` ⇒ `jwts_blacklisted = 0`.
    pub iterator: Option<&'a dyn ActiveTokenIterator>,
}

2. Nouveau point d'entrée canonique

impl FediverseSignalService {
    /// Entrypoint canonique pour appliquer un `Delete{Actor}` distant.
    /// Combine la sémantique de toutes les variantes historiques :
    ///
    /// 1. Flip `remote_identities.disabled_at` → `NOW()` (idempotent).
    /// 2. Si `hooks.iterator.is_some()` : blacklist tous les JWT actifs
    ///    rapportés par l'iterator pour la `remote_identity_id`
    ///    déterminée par le flip.
    /// 3. Si `hooks.observer.is_some()` ET `rows_disabled == 1` : fire
    ///    `on_delete_actor(domain, ap_id, remote_identity_id)`
    ///    **après** que l'étape 2 ait été commitée.
    ///
    /// Idempotence : les étapes 2 et 3 ne s'exécutent que sur une
    /// transition active → désactivée. Une re-livraison du même
    /// `Delete{Actor}` ne re-blackliste pas et ne re-fire pas le
    /// hook.
    pub async fn apply_delete_actor_with_hooks(
        db: &DatabaseConnection,
        ap_id: &str,
        hooks: DeleteActorHooks<'_>,
    ) -> Result<DeleteOutcome, AppError> {
        // ... voir Implementation Sketch ...
    }
}

3. Wrappers de compatibilité (signatures inchangées)

impl FediverseSignalService {
    /// Wrapper de compatibilité. Préférer
    /// [`Self::apply_delete_actor_with_hooks`] pour le code neuf.
    pub async fn apply_delete_actor(
        db: &DatabaseConnection,
        ap_id: &str,
    ) -> Result<DeleteOutcome, AppError> {
        Self::apply_delete_actor_with_hooks(db, ap_id, DeleteActorHooks::default()).await
    }

    /// Wrapper de compatibilité. Préférer
    /// [`Self::apply_delete_actor_with_hooks`] pour le code neuf.
    pub async fn apply_delete_actor_observed(
        db: &DatabaseConnection,
        ap_id: &str,
        observer: &dyn FediverseEventObserver,
    ) -> Result<DeleteOutcome, AppError> {
        Self::apply_delete_actor_with_hooks(
            db,
            ap_id,
            DeleteActorHooks { observer: Some(observer), ..Default::default() },
        )
        .await
    }

    /// Wrapper de compatibilité. Préférer
    /// [`Self::apply_delete_actor_with_hooks`] pour le code neuf.
    pub async fn apply_delete_actor_with_iterator(
        db: &DatabaseConnection,
        ap_id: &str,
        iterator: &dyn ActiveTokenIterator,
    ) -> Result<DeleteOutcome, AppError> {
        Self::apply_delete_actor_with_hooks(
            db,
            ap_id,
            DeleteActorHooks { iterator: Some(iterator), ..Default::default() },
        )
        .await
    }
}

4. Migration des call-sites internes

Le dispatcher inbox (crates/rustwarden-fediverse/src/inbox.rs:202) passe au nouvel entrypoint :

// Avant
FediverseSignalService::apply_delete_actor_observed(db, &ap_id, observer).await?;

// Après
FediverseSignalService::apply_delete_actor_with_hooks(
    db,
    &ap_id,
    DeleteActorHooks { observer: Some(observer), ..Default::default() },
).await?;

L'iterator n'est pas câblé au handler inbox dans cette RFC (relève d'une RFC dédiée pour exposer l'ActiveTokenIterator via Extension ou builder).

Implementation sketch

Le corps unifié reprend la logique existante de apply_delete_actor_observed, en intercalant l'étape iterator :

pub async fn apply_delete_actor_with_hooks(
    db: &DatabaseConnection,
    ap_id: &str,
    hooks: DeleteActorHooks<'_>,
) -> Result<DeleteOutcome, AppError> {
    let Some(row) = remote_identity::Entity::find()
        .filter(remote_identity::Column::ApId.eq(ap_id))
        .one(db)
        .await?
    else {
        return Ok(DeleteOutcome::default());
    };

    if row.disabled_at.is_some() {
        return Ok(DeleteOutcome { rows_disabled: 0, jwts_blacklisted: 0 });
    }

    let domain = row.domain.clone();
    let remote_identity_id = row.id;

    let mut active: remote_identity::ActiveModel = row.into();
    active.disabled_at = Set(Some(Utc::now()));
    active.update(db).await?;

    let jwts_blacklisted = if let Some(it) = hooks.iterator {
        JwtBlacklistService::blacklist_by_remote_identity_id(db, remote_identity_id, it).await?
    } else {
        0
    };

    if let Some(observer) = hooks.observer {
        observer.on_delete_actor(&domain, ap_id, remote_identity_id).await;
    }

    Ok(DeleteOutcome {
        rows_disabled: 1,
        jwts_blacklisted,
    })
}

~25 lignes au total. Les trois wrappers font ~5 lignes chacun. Le test suite RFC 0032 + RFC 0018 s'applique inchangé puisque les wrappers délèguent. 2 tests nouveaux suffisent à couvrir le chemin combiné.

Analyse de compatibilité

SurfaceAvantAprèsBreaking ?
apply_delete_actor(db, ap_id)ExisteIdentique (wrapper)❌
apply_delete_actor_observed(db, ap_id, observer)ExisteIdentique (wrapper)❌
apply_delete_actor_with_iterator(db, ap_id, iter)ExisteIdentique (wrapper)❌
apply_delete_actor_with_hooks(...)N'existe pasNouveau➕ additif
DeleteActorHooks<'a>N'existe pasNouvelle struct publique➕ additif
Sémantique (idempotence, ordering, no-fire on no-row)RFC 0032Inchangée❌

Aucun changement de comportement observable pour les call-sites existants. Le hash MD5 du DeleteOutcome retourné pour un input donné est identique avant/après RFC 0033.

Testing plan

Tests reconduits (inchangés)

Les 3 tests apply_delete_actor_observed_* + le test apply_delete_actor_with_noop_iterator_keeps_jwts_blacklisted_zero continuent de passer puisque les wrappers délèguent au nouveau core.

Tests nouveaux

  1. apply_delete_actor_with_hooks_combines_observer_and_iterator Iterator fournit 2 JTI, observer compte 1 call. Assert rows_disabled=1, jwts_blacklisted=2, observer reçoit le (domain, ap_id, id) correct.

  2. apply_delete_actor_with_hooks_skips_observer_and_iterator_when_already_disabled Row déjà disabled : ni observer ni iterator ne sont consultés.

  3. delete_actor_hooks_default_is_all_none Trivial : DeleteActorHooks::default() donne observer=None, iterator=None.

Tests à supprimer

Aucun. Les anciens tests valident toujours le contrat public des wrappers.

Non-goals

  • Ne touche pas au trait ActiveTokenIterator ni à sa sémantique.
  • Ne touche pas au trait FediverseEventObserver ni à ses 7 méthodes.
  • Ne câble pas l'iterator au handler inbox /inbox — c'est une décision orthogonale (qui exposerait l'ActiveTokenIterator via builder / Extension). Sujet de RFC séparée.
  • Pas de deprecation des trois wrappers : ils restent first-class. Un #[deprecated] éventuel relèvera d'une RFC future après période d'observation.
  • Pas de builder pattern : la struct literal avec ..Default::default() est suffisamment ergonomique pour le besoin actuel. Un builder peut être ajouté ultérieurement comme convenience additive.

Alternatives considérées

Alternative A — 4ᵉ variante apply_delete_actor_with_iterator_and_observer

Ajouter une fonction nommée par concaténation des hooks activés.

  • Coût : 1 nouvelle fonction de ~30 lignes.
  • Problème : combinatoire. Si une RFC future ajoute un 3ᵉ hook (audit log, métriques), on a 2³ = 8 variantes possibles à nommer/maintenir.

→ Rejetée. Ne scale pas.

Alternative B — Builder pattern fluide

DeleteActorHooks::new()
    .with_observer(&my_obs)
    .with_iterator(&my_it)
  • Coût : ~4 méthodes supplémentaires.
  • Avantage : un peu plus self-documenting.
  • Inconvénient : double l'API surface sans apporter d'expressivité décisive vs le struct literal.

→ Reportée. Peut être ajoutée plus tard comme couche additive sans cassure.

Alternative C — Remplacer les 3 wrappers par #[deprecated] pointant vers _with_hooks

  • Avantage : pousse activement les consommateurs vers la nouvelle API ; surface canonique unique.
  • Inconvénient : génère des warnings dans tous les consommateurs existants (gitrust en tête) dès cette RFC, alors qu'aucune fonctionnalité ne le justifie. Force une migration synchrone côté consommateurs sur leur prochain cargo update.

→ Reportée. La deprecation pourra être faite dans une RFC ultérieure après que le _with_hooks ait fait ses preuves côté consommateurs.

Alternative D — Ne rien faire (statu quo post-RFC 0032)

  • Coût immédiat : 0.
  • Coût caché : chaque consommateur qui veut observer + iterator dupliquera la logique de flip/idempotence, perdant l'atomicité de la transition et risquant des bugs subtils (double-blacklist, race conditions).

→ Rejetée. La dette est connue et chaque jour qui passe alourdit la migration future.

Score auto-évalué

Grille docs/RUSTWARDEN_EVOLUTION_PROCESS.md §4 :

CritèreNoteJustification
Généralité5/5Le struct DeleteActorHooks accommode tout futur hook sans nouvelle variante de fonction.
Stabilité API5/5Strictement additif. Les 3 signatures publiques existantes sont préservées octet pour octet.
Performance5/5Aucun overhead : les Option<&dyn Trait> sont des pointers nullables, branchés sans allocation.
Sécurité4/5Ferme un trou de composabilité qui forçait certains consommateurs à dupliquer la logique d'idempotence → risque de bug réduit. Pas de nouvelle surface d'attaque.
Complexité d'implémentation5/5~50 lignes net (core + 3 wrappers + struct). Pas d'unsafe, pas de macro custom.
Charge de maintenance5/5Une seule impl à maintenir. Les wrappers sont des 1-liners qui n'ont rien à diverger.
Documentation5/5Doc complète sur la struct, l'entrypoint, et chaque wrapper avec pointeur vers le canonical.

Score : 34/35 — au-dessus du seuil de 25/35 pour acceptation directe.

Open questions

  1. Faut-il rendre le wrapper apply_delete_actor (sans hooks) #[doc(hidden)] ? Le plus utilisé historiquement mais le moins utile post-RFC 0033. Décision : non, garder visible — utile pour les call-sites qui n'ont vraiment besoin de rien (ex. réplication offline depuis un dump).

  2. Faut-il ajouter #[must_use] sur DeleteOutcome ? Indépendant de cette RFC, mais à considérer en hardening connexe.

  3. DeleteActorHooks mérite-t-il un Debug derive ? Les dyn Trait interdisent le Debug direct. On peut imprimer la présence/absence via une impl manuelle. À voir au moment de l'implémentation si un test ou un log le réclame.

Workflow

  1. PR feat(fediverse): unify apply_delete_actor variants behind DeleteActorHooks sur la branche fedivers.
  2. Quality gates : cargo build, cargo test -p rustwarden-fediverse, cargo clippy --all-targets -D warnings.
  3. Mettre à jour docs/SERVICES_REFERENCE.md §21 avec la nouvelle signature canonique.
  4. Mettre à jour docs/rfcs/README.md avec l'entrée 0033.
  5. Statut → Accepted → Implemented après merge.