togglz icon indicating copy to clipboard operation
togglz copied to clipboard

Faulty FeatureStateChangedEvent for ApplicationEventPublisherRepository with CachingStateRepository

Open MarcelLorke6666 opened this issue 8 months ago • 0 comments

We use the Spring Boot Actuator GET /actuator/togglz and POST /actuator/togglz/TOGGLE endpoints to view and update our toggles paired with ApplicationEventPublisherRepository and CachingStateRepository.

In our scenario we first list the toggles via GET /actuator/togglz and then update them afterwards via POST /actuator/togglz/TOGGLE.

TogglzEndpoint#getAllFeatures will load them into the cache upon GET /actuator/togglz

@ReadOperation
public List<TogglzFeature> getAllFeatures() {
    return this.featureManager.getFeatures().stream()
            .map(this::generateTogglzFeature)
            .sorted()
            .collect(Collectors.toList());
}

The subsequent call to update it will first load it from the cache, set the enabled flag and then update it again.

protected FeatureState changeFeatureStatus(
        Feature feature, Boolean enabled, String strategy, Map<String, String> parameters) {
    FeatureState featureState = featureManager.getFeatureState(feature);

    if (enabled != null) {
        featureState.setEnabled(enabled);
    }
    if (strategy != null) {
        featureState.setStrategyId(strategy);
    }
    parameters.forEach(featureState::setParameter);

    featureManager.setFeatureState(featureState); <-- This is the same object as in the cache

    return featureState;
}

In case we now invoke setFeatureState on ApplicationEventPublisherRepository (which in our case delegates to CachingStateRepository) it will first get the object to determine its previous state

@Override
public void setFeatureState(FeatureState featureState) {
    FeatureState previousFeatureState = getFeatureState(featureState.getFeature());
    delegate.setFeatureState(featureState);
    applicationEventPublisher.publishEvent(new FeatureStateChangedEvent(previousFeatureState, featureState));
}

However, since the passed in featureState is the actual object in the cache and previousFeatureState is taken from the cache they are the exact same object and thus the state of previousFeatureState is faulty.

My proposal is therefore to adapt CachingStateRepository#getFeatureState to return not the actual cached FeatureState object, but a copy of it.

MarcelLorke6666 avatar Jun 19 '24 13:06 MarcelLorke6666