From 8cfbb515fc528df6752f32483533bb4b904c92ef Mon Sep 17 00:00:00 2001 From: daguimu Date: Thu, 14 May 2026 23:35:29 +0800 Subject: [PATCH] fix(sentinel): make SentinelFeignClientProperties#copy() fail fast on Jackson errors The copy() method serializes/deserializes via Jackson to produce a deep backup used by CircuitBreakerRuleChangeListener for change detection in afterSingletonsInstantiated() and updateBackup(). On any serialization failure the previous catch (Exception ignored) silently returned a brand-new SentinelFeignClientProperties() with default values, dropping every user-configured rule. The listener then compared the live properties against that empty backup and treated every existing rule as "removed", triggering an erroneous full-rule refresh. Replace the silent fallback with throw new IllegalStateException(...) that wraps the original cause. Since Jackson 3 (tools.jackson) throws unchecked JacksonException, the try/catch was already legacy from the Jackson 2 (com.fasterxml.jackson) era; fail-fast surfaces the real issue at startup or refresh time instead of corrupting the diff baseline. Adds three unit tests covering: equal-but-distinct deep copy, copy independence from the original, and IllegalStateException propagation when Jackson serialization fails. --- .../feign/SentinelFeignClientProperties.java | 5 +- .../SentinelFeignClientPropertiesTest.java | 87 +++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/test/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientPropertiesTest.java diff --git a/spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/main/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientProperties.java b/spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/main/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientProperties.java index 0d97c9ef36..ab746507a0 100644 --- a/spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/main/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientProperties.java +++ b/spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/main/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientProperties.java @@ -97,9 +97,10 @@ public SentinelFeignClientProperties copy() { String json = objectMapper.writeValueAsString(this); return objectMapper.readValue(json, this.getClass()); } - catch (Exception ignored) { + catch (Exception e) { + throw new IllegalStateException( + "Failed to deep-copy SentinelFeignClientProperties via Jackson", e); } - return new SentinelFeignClientProperties(); } } diff --git a/spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/test/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientPropertiesTest.java b/spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/test/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientPropertiesTest.java new file mode 100644 index 0000000000..afbd2537da --- /dev/null +++ b/spring-cloud-alibaba-starters/spring-cloud-circuitbreaker-sentinel/src/test/java/com/alibaba/cloud/circuitbreaker/sentinel/feign/SentinelFeignClientPropertiesTest.java @@ -0,0 +1,87 @@ +/* + * Copyright 2013-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.alibaba.cloud.circuitbreaker.sentinel.feign; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import com.alibaba.csp.sentinel.slots.block.degrade.DegradeRule; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Unit tests for {@link SentinelFeignClientProperties#copy()}. + */ +public class SentinelFeignClientPropertiesTest { + + @Test + public void copyReturnsEqualButDistinctInstance() { + SentinelFeignClientProperties original = new SentinelFeignClientProperties(); + original.setDefaultRule("custom-default"); + original.setEnableRefreshRules(false); + Map> rules = new HashMap<>(); + List ruleList = new ArrayList<>(); + DegradeRule rule = new DegradeRule("resource-a"); + rule.setCount(5.0); + ruleList.add(rule); + rules.put("resource-a", ruleList); + original.setRules(rules); + + SentinelFeignClientProperties copied = original.copy(); + + assertThat(copied).isNotSameAs(original); + assertThat(copied).isEqualTo(original); + } + + @Test + public void modifyingCopyDoesNotAffectOriginal() { + SentinelFeignClientProperties original = new SentinelFeignClientProperties(); + Map> rules = new HashMap<>(); + List ruleList = new ArrayList<>(); + ruleList.add(new DegradeRule("resource-a")); + rules.put("resource-a", ruleList); + original.setRules(rules); + + SentinelFeignClientProperties copied = original.copy(); + copied.getRules().put("resource-b", + new ArrayList<>(List.of(new DegradeRule("resource-b")))); + copied.setDefaultRule("changed"); + + assertThat(original.getRules()).containsOnlyKeys("resource-a"); + assertThat(original.getDefaultRule()).isEqualTo("default"); + } + + @Test + public void copyFailsFastWhenJacksonCannotSerialize() { + SentinelFeignClientProperties brokenProps = new SentinelFeignClientProperties() { + @Override + public Map> getRules() { + throw new RuntimeException("simulated serialization failure"); + } + }; + + assertThatThrownBy(brokenProps::copy) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("SentinelFeignClientProperties") + .hasCauseInstanceOf(RuntimeException.class); + } + +}