Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faulty FeatureStateChangedEvent for ApplicationEventPublisherRepository with CachingStateRepository #1196

Open
MarcelLorke6666 opened this issue Jun 19, 2024 · 0 comments

Comments

@MarcelLorke6666
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant