fix(feature-flags): close Phase 22 KEEP-VERIFY — 2 real defects + doc overclaims
KEEP-VERIFY re-audit of the Feature Flag configuration extension (STATUS's "Phase 22 部署运维配置扩展"): marked ✅ but contained real code defects and overstated completion. Real defects fixed: - D-22-A (High): FeatureFlagController.updateFlag used reflection `getMethod("set"+cap, boolean.class)` and only supported boolean flags. 7 non-boolean flags (livenessProvider, localEvidenceAllowedTypes, counselingAttachmentsBucket, localEvidenceMaxFiles, localEvidenceMaxFileSizeMB, localEvidenceRetentionDays) failed to update via the API. Now dispatches by field type (boolean/int/String). - D-22-B (Medium): `body.getOrDefault("value", false)` silently set false when value was missing and crashed on non-boolean casts. Now returns 400 on missing value / type mismatch and leaves the original value unchanged. Doc overclaims corrected (no silent scope creep): - T01-T05 module-toggle flags (isCounselingEnabled etc.) have zero business call sites -> dead config (exposed but not gating). Left for each module's own phase to wire; documented as residual. - T07 private-storage bucket flags have zero references; storage already uses object-storage.bucket-private. Documented as residual. - STATUS claimed "application.yml defaults (15 flags)"; yml has no `features:` block, defaults are Java field defaults. Functionally fine, docs corrected. T06 upload policy verified actually enforced in LocalEligibilityService + PhotoService (kept ✅). Guard tests: new FeatureFlagControllerTest (7 cases: 21 keys exposed, boolean/ string/int update reflected, unknown flag / missing value / wrong type -> 400). Negative validation: reverting to boolean-only dispatch made exactly 2 update cases fail red, then green after restore — proves guards are real. Full regression: 657 tests / 0 failures / 0 errors / 3 skipped (baseline 650, +7).
This commit is contained in:
WorkBuddy
+45
-8
@@ -34,20 +34,57 @@ public class FeatureFlagController {
|
||||
/**
|
||||
* 更新单个 Feature Flag
|
||||
* PUT /api/v1/operator/feature-flags/{flagName}
|
||||
* Body: { "value": true }
|
||||
* Body: { "value": <boolean|int|string> }
|
||||
*
|
||||
* <p>支持布尔 / 整数 / 字符串三类 flag（此前仅支持布尔，导致 7 个非布尔 flag
|
||||
* 经 API 更新必失败）。未知 flag 或缺失 value 返回 400，且不会静默把功能置为关闭。
|
||||
*/
|
||||
@PutMapping("/{flagName}")
|
||||
public ApiResponse<Map<String, Object>> updateFlag(
|
||||
@AuthenticationPrincipal OperatorPrincipal principal,
|
||||
@PathVariable String flagName,
|
||||
@RequestBody Map<String, Boolean> body) {
|
||||
boolean value = body.getOrDefault("value", false);
|
||||
// 使用反射更新对应字段
|
||||
@RequestBody Map<String, Object> body) {
|
||||
if (!featureFlagConfig.getAllFlags().containsKey(flagName)) {
|
||||
return ApiResponse.error(
|
||||
com.danyang.match.platform.shared.enums.ApiResponseCode.BAD_REQUEST,
|
||||
"未知的 Feature Flag: " + flagName);
|
||||
}
|
||||
if (!body.containsKey("value")) {
|
||||
return ApiResponse.error(
|
||||
com.danyang.match.platform.shared.enums.ApiResponseCode.BAD_REQUEST,
|
||||
"请求体必须包含 value 字段");
|
||||
}
|
||||
Object value = body.get("value");
|
||||
try {
|
||||
java.lang.reflect.Method setter = featureFlagConfig.getClass()
|
||||
.getMethod("set" + capitalize(flagName), boolean.class);
|
||||
setter.invoke(featureFlagConfig, value);
|
||||
return ApiResponse.ok(Map.of("flagName", flagName, "value", value, "updated", true));
|
||||
java.lang.reflect.Field field = featureFlagConfig.getClass().getDeclaredField(flagName);
|
||||
Class<?> type = field.getType();
|
||||
if (type == boolean.class || type == Boolean.class) {
|
||||
boolean b = ((Boolean) value);
|
||||
featureFlagConfig.getClass()
|
||||
.getMethod("set" + capitalize(flagName), boolean.class)
|
||||
.invoke(featureFlagConfig, b);
|
||||
} else if (type == int.class || type == Integer.class) {
|
||||
int i = ((Number) value).intValue();
|
||||
featureFlagConfig.getClass()
|
||||
.getMethod("set" + capitalize(flagName), int.class)
|
||||
.invoke(featureFlagConfig, i);
|
||||
} else if (type == String.class) {
|
||||
featureFlagConfig.getClass()
|
||||
.getMethod("set" + capitalize(flagName), String.class)
|
||||
.invoke(featureFlagConfig, String.valueOf(value));
|
||||
} else {
|
||||
return ApiResponse.error(
|
||||
com.danyang.match.platform.shared.enums.ApiResponseCode.BAD_REQUEST,
|
||||
"不支持的 Flag 类型: " + type.getSimpleName());
|
||||
}
|
||||
return ApiResponse.ok(Map.of(
|
||||
"flagName", flagName,
|
||||
"value", featureFlagConfig.getAllFlags().get(flagName),
|
||||
"updated", true));
|
||||
} catch (ClassCastException e) {
|
||||
return ApiResponse.error(
|
||||
com.danyang.match.platform.shared.enums.ApiResponseCode.BAD_REQUEST,
|
||||
"value 类型与 Flag " + flagName + " 不匹配: " + e.getMessage());
|
||||
} catch (Exception e) {
|
||||
return ApiResponse.error(
|
||||
com.danyang.match.platform.shared.enums.ApiResponseCode.BAD_REQUEST,
|
||||
|
||||
+113
@@ -0,0 +1,113 @@
|
||||
package com.danyang.match.platform.shared.controller;
|
||||
|
||||
import com.danyang.match.platform.shared.config.FeatureFlagConfig;
|
||||
import com.danyang.match.platform.shared.dto.ApiResponse;
|
||||
import com.danyang.match.platform.shared.security.principal.OperatorPrincipal;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.boot.test.mock.mockito.MockBean;
|
||||
import org.springframework.data.redis.core.StringRedisTemplate;
|
||||
import org.springframework.test.context.ActiveProfiles;
|
||||
import org.springframework.transaction.annotation.Transactional;
|
||||
|
||||
import java.util.Map;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
/**
|
||||
* Phase 22（部署运维配置扩展 — Feature Flag）真实守卫测试。
|
||||
*
|
||||
* <p>背景（KEEP-VERIFY 复验发现）：
|
||||
* <ul>
|
||||
* <li>D-22-A（High）：原 {@code updateFlag} 仅支持布尔 flag（反射 {@code getMethod("set"+cap, boolean.class)}），
|
||||
* 7 个非布尔 flag（livenessProvider / localEvidenceAllowedTypes / counselingAttachmentsBucket /
|
||||
* localEvidenceMaxFiles / localEvidenceMaxFileSizeMB / localEvidenceRetentionDays）经 API 更新必失败。</li>
|
||||
* <li>D-22-B（Medium）：原 {@code body.getOrDefault("value", false)} 缺 value 时静默置 false（可能误关功能），
|
||||
* 且非布尔类型转型会崩。</li>
|
||||
* </ul>
|
||||
*
|
||||
* <p>本测试直接驱动控制器方法（与 BannerOperatorPhase19IntegrationTest 同一装配模式），
|
||||
* 使用隔离的 {@link FeatureFlagConfig} 实例，避免污染共享单例影响其他读取默认 flag 的测试。
|
||||
*/
|
||||
@SpringBootTest
|
||||
@ActiveProfiles("test")
|
||||
@Transactional
|
||||
class FeatureFlagControllerTest {
|
||||
|
||||
@MockBean
|
||||
private StringRedisTemplate stringRedisTemplate;
|
||||
|
||||
private FeatureFlagConfig fixture;
|
||||
private FeatureFlagController controller;
|
||||
private OperatorPrincipal principal;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
fixture = new FeatureFlagConfig();
|
||||
controller = new FeatureFlagController(fixture);
|
||||
principal = new OperatorPrincipal("00000000-0000-0000-0000-000000000099");
|
||||
}
|
||||
|
||||
@Test
|
||||
void getAllFlagsReturnsEveryDeclaredFlag() {
|
||||
ApiResponse<Map<String, Object>> resp = controller.getAllFlags();
|
||||
assertTrue(resp.isSuccess());
|
||||
Map<String, Object> flags = resp.getData();
|
||||
// 21 个 flag 必须全部暴露，否则运维看不到/改不了某个配置（含此前仅布尔可更新的死区）
|
||||
assertEquals(21, flags.size());
|
||||
for (String key : new String[]{
|
||||
"activityEnabled", "matchmakerEnabled", "chatImageEnabled", "appealEnabled",
|
||||
"counselingEnabled", "premaritalEnabled", "contentEnabled", "serviceLocationEnabled",
|
||||
"localEligibilityEnabled", "referralEnabled", "mutualConsentEnabled", "serviceProgramEnabled",
|
||||
"livenessEnabled", "livenessProvider", "localEvidenceUploadEnabled",
|
||||
"localEvidenceMaxFiles", "localEvidenceMaxFileSizeMB", "localEvidenceAllowedTypes",
|
||||
"localEvidenceRetentionDays", "counselingAttachmentsPrivate", "counselingAttachmentsBucket"
|
||||
}) {
|
||||
assertTrue(flags.containsKey(key), "缺少暴露的 flag: " + key);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateBooleanFlagIsReflected() {
|
||||
ApiResponse<Map<String, Object>> resp = controller.updateFlag(principal, "counselingEnabled", Map.of("value", false));
|
||||
assertTrue(resp.isSuccess());
|
||||
assertFalse(fixture.isCounselingEnabled());
|
||||
assertEquals(false, fixture.getAllFlags().get("counselingEnabled"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateStringFlagIsReflected() {
|
||||
ApiResponse<Map<String, Object>> resp = controller.updateFlag(principal, "livenessProvider", Map.of("value", "third-party"));
|
||||
assertTrue(resp.isSuccess());
|
||||
assertEquals("third-party", fixture.getLivenessProvider());
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateIntFlagIsReflected() {
|
||||
ApiResponse<Map<String, Object>> resp = controller.updateFlag(principal, "localEvidenceMaxFiles", Map.of("value", 3));
|
||||
assertTrue(resp.isSuccess());
|
||||
assertEquals(3, fixture.getLocalEvidenceMaxFiles());
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateUnknownFlagReturnsBadRequest() {
|
||||
ApiResponse<Map<String, Object>> resp = controller.updateFlag(principal, "noSuchFlag", Map.of("value", true));
|
||||
assertFalse(resp.isSuccess());
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateMissingValueReturnsBadRequest() {
|
||||
ApiResponse<Map<String, Object>> resp = controller.updateFlag(principal, "counselingEnabled", Map.of());
|
||||
assertFalse(resp.isSuccess());
|
||||
}
|
||||
|
||||
@Test
|
||||
void updateBooleanFlagWithWrongTypeReturnsBadRequestAndDoesNotSilentlyDisable() {
|
||||
ApiResponse<Map<String, Object>> resp = controller.updateFlag(principal, "counselingEnabled", Map.of("value", "not-a-bool"));
|
||||
assertFalse(resp.isSuccess());
|
||||
assertTrue(fixture.isCounselingEnabled(), "类型不匹配不应静默把功能置为关闭");
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user