mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-04-11 13:23:00 +00:00
fix: address review findings for dotted-key handling
- ensure_array_defaults: replace None nodes with {} so recursion
proceeds into nested objects (was skipping when key existed as None)
- dotToNested: add tail-matching that checks the full remaining dotted
tail against the current schema level before greedy intermediate
matching, preventing leaf dotted keys from being split
- syncFormToJson: replace naive key.split('.') reconstruction with
dotToNested(flatConfig, schema) and schema-aware getSchemaProperty()
so the JSON tab save path produces the same correct nesting as the
form submit path
- Add regression tests for dotted-key array normalization and None
array default replacement
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -583,3 +583,129 @@ class TestAPIErrorHandling:
|
|||||||
response = client.get('/api/v3/display/on-demand/start')
|
response = client.get('/api/v3/display/on-demand/start')
|
||||||
|
|
||||||
assert response.status_code in [200, 405] # Depends on implementation
|
assert response.status_code in [200, 405] # Depends on implementation
|
||||||
|
|
||||||
|
|
||||||
|
class TestDottedKeyNormalization:
|
||||||
|
"""Regression tests for fix_array_structures / ensure_array_defaults with dotted schema keys."""
|
||||||
|
|
||||||
|
def test_save_plugin_config_dotted_key_arrays(self, client, mock_config_manager):
|
||||||
|
"""Nested dotted-key objects with numeric-keyed dicts are converted to arrays."""
|
||||||
|
from web_interface.blueprints.api_v3 import api_v3
|
||||||
|
|
||||||
|
api_v3.config_manager = mock_config_manager
|
||||||
|
mock_config_manager.load_config.return_value = {}
|
||||||
|
|
||||||
|
schema_mgr = MagicMock()
|
||||||
|
schema = {
|
||||||
|
'type': 'object',
|
||||||
|
'properties': {
|
||||||
|
'leagues': {
|
||||||
|
'type': 'object',
|
||||||
|
'properties': {
|
||||||
|
'eng.1': {
|
||||||
|
'type': 'object',
|
||||||
|
'properties': {
|
||||||
|
'enabled': {'type': 'boolean', 'default': True},
|
||||||
|
'favorite_teams': {
|
||||||
|
'type': 'array',
|
||||||
|
'items': {'type': 'string'},
|
||||||
|
'default': [],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
schema_mgr.load_schema.return_value = schema
|
||||||
|
schema_mgr.generate_default_config.return_value = {
|
||||||
|
'leagues': {'eng.1': {'enabled': True, 'favorite_teams': []}},
|
||||||
|
}
|
||||||
|
schema_mgr.merge_with_defaults.side_effect = lambda config, defaults: {**defaults, **config}
|
||||||
|
schema_mgr.validate_config_against_schema.return_value = []
|
||||||
|
api_v3.schema_manager = schema_mgr
|
||||||
|
|
||||||
|
request_data = {
|
||||||
|
'plugin_id': 'soccer-scoreboard',
|
||||||
|
'config': {
|
||||||
|
'leagues': {
|
||||||
|
'eng.1': {
|
||||||
|
'enabled': True,
|
||||||
|
'favorite_teams': ['Arsenal', 'Chelsea'],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
'/api/v3/plugins/config',
|
||||||
|
data=json.dumps(request_data),
|
||||||
|
content_type='application/json',
|
||||||
|
)
|
||||||
|
|
||||||
|
if response.status_code == 200:
|
||||||
|
saved = mock_config_manager.save_config_atomic.call_args[0][0]
|
||||||
|
soccer_cfg = saved.get('soccer-scoreboard', {})
|
||||||
|
leagues = soccer_cfg.get('leagues', {})
|
||||||
|
assert 'eng.1' in leagues, f"Expected 'eng.1' key, got: {list(leagues.keys())}"
|
||||||
|
assert isinstance(leagues['eng.1'].get('favorite_teams'), list)
|
||||||
|
assert leagues['eng.1']['favorite_teams'] == ['Arsenal', 'Chelsea']
|
||||||
|
|
||||||
|
def test_save_plugin_config_none_array_gets_default(self, client, mock_config_manager):
|
||||||
|
"""None array fields under dotted-key parents are replaced with defaults."""
|
||||||
|
from web_interface.blueprints.api_v3 import api_v3
|
||||||
|
|
||||||
|
api_v3.config_manager = mock_config_manager
|
||||||
|
mock_config_manager.load_config.return_value = {}
|
||||||
|
|
||||||
|
schema_mgr = MagicMock()
|
||||||
|
schema = {
|
||||||
|
'type': 'object',
|
||||||
|
'properties': {
|
||||||
|
'leagues': {
|
||||||
|
'type': 'object',
|
||||||
|
'properties': {
|
||||||
|
'eng.1': {
|
||||||
|
'type': 'object',
|
||||||
|
'properties': {
|
||||||
|
'favorite_teams': {
|
||||||
|
'type': 'array',
|
||||||
|
'items': {'type': 'string'},
|
||||||
|
'default': [],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
schema_mgr.load_schema.return_value = schema
|
||||||
|
schema_mgr.generate_default_config.return_value = {
|
||||||
|
'leagues': {'eng.1': {'favorite_teams': []}},
|
||||||
|
}
|
||||||
|
schema_mgr.merge_with_defaults.side_effect = lambda config, defaults: {**defaults, **config}
|
||||||
|
schema_mgr.validate_config_against_schema.return_value = []
|
||||||
|
api_v3.schema_manager = schema_mgr
|
||||||
|
|
||||||
|
request_data = {
|
||||||
|
'plugin_id': 'soccer-scoreboard',
|
||||||
|
'config': {
|
||||||
|
'leagues': {
|
||||||
|
'eng.1': {
|
||||||
|
'favorite_teams': None,
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
response = client.post(
|
||||||
|
'/api/v3/plugins/config',
|
||||||
|
data=json.dumps(request_data),
|
||||||
|
content_type='application/json',
|
||||||
|
)
|
||||||
|
|
||||||
|
if response.status_code == 200:
|
||||||
|
saved = mock_config_manager.save_config_atomic.call_args[0][0]
|
||||||
|
soccer_cfg = saved.get('soccer-scoreboard', {})
|
||||||
|
teams = soccer_cfg.get('leagues', {}).get('eng.1', {}).get('favorite_teams')
|
||||||
|
assert isinstance(teams, list), f"Expected list, got: {type(teams)}"
|
||||||
|
|||||||
@@ -4113,8 +4113,7 @@ def save_plugin_config():
|
|||||||
nested_dict = config_dict.get(prop_key)
|
nested_dict = config_dict.get(prop_key)
|
||||||
|
|
||||||
if nested_dict is None:
|
if nested_dict is None:
|
||||||
if prop_key not in config_dict:
|
config_dict[prop_key] = {}
|
||||||
config_dict[prop_key] = {}
|
|
||||||
nested_dict = config_dict[prop_key]
|
nested_dict = config_dict[prop_key]
|
||||||
|
|
||||||
if isinstance(nested_dict, dict):
|
if isinstance(nested_dict, dict):
|
||||||
|
|||||||
@@ -2334,6 +2334,15 @@ function dotToNested(obj, schema) {
|
|||||||
while (i < parts.length - 1) {
|
while (i < parts.length - 1) {
|
||||||
let matched = false;
|
let matched = false;
|
||||||
if (currentSchema) {
|
if (currentSchema) {
|
||||||
|
// First, check if the full remaining tail is a leaf property
|
||||||
|
// (e.g., "eng.1" as a complete dotted key with no sub-properties)
|
||||||
|
const tailCandidate = parts.slice(i).join('.');
|
||||||
|
if (tailCandidate in currentSchema) {
|
||||||
|
current[tailCandidate] = obj[key];
|
||||||
|
matched = true;
|
||||||
|
i = parts.length; // consumed all parts
|
||||||
|
break;
|
||||||
|
}
|
||||||
// Try progressively longer candidates (longest first) to greedily
|
// Try progressively longer candidates (longest first) to greedily
|
||||||
// match dotted property names like "eng.1"
|
// match dotted property names like "eng.1"
|
||||||
for (let j = parts.length - 1; j > i; j--) {
|
for (let j = parts.length - 1; j > i; j--) {
|
||||||
@@ -4463,42 +4472,35 @@ function switchPluginConfigView(view) {
|
|||||||
function syncFormToJson() {
|
function syncFormToJson() {
|
||||||
const form = document.getElementById('plugin-config-form');
|
const form = document.getElementById('plugin-config-form');
|
||||||
if (!form) return;
|
if (!form) return;
|
||||||
|
|
||||||
const formData = new FormData(form);
|
const formData = new FormData(form);
|
||||||
const config = {};
|
const flatConfig = {};
|
||||||
|
|
||||||
// Get schema for type conversion
|
// Get schema for type conversion
|
||||||
const schema = currentPluginConfigState.schema;
|
const schema = currentPluginConfigState.schema;
|
||||||
|
|
||||||
for (let [key, value] of formData.entries()) {
|
for (let [key, value] of formData.entries()) {
|
||||||
if (key === 'enabled') continue; // Skip enabled, managed separately
|
if (key === 'enabled') continue; // Skip enabled, managed separately
|
||||||
|
|
||||||
// Handle nested keys (dot notation)
|
// Use schema-aware property lookup for type conversion
|
||||||
const keys = key.split('.');
|
const prop = schema ? getSchemaProperty(schema, key) : null;
|
||||||
let current = config;
|
|
||||||
for (let i = 0; i < keys.length - 1; i++) {
|
|
||||||
if (!current[keys[i]]) {
|
|
||||||
current[keys[i]] = {};
|
|
||||||
}
|
|
||||||
current = current[keys[i]];
|
|
||||||
}
|
|
||||||
|
|
||||||
const finalKey = keys[keys.length - 1];
|
|
||||||
const prop = schema?.properties?.[finalKey] || (keys.length > 1 ? null : schema?.properties?.[key]);
|
|
||||||
|
|
||||||
// Type conversion based on schema
|
// Type conversion based on schema
|
||||||
if (prop?.type === 'array') {
|
if (prop?.type === 'array') {
|
||||||
current[finalKey] = value.split(',').map(item => item.trim()).filter(item => item.length > 0);
|
flatConfig[key] = value.split(',').map(item => item.trim()).filter(item => item.length > 0);
|
||||||
} else if (prop?.type === 'integer' || key === 'display_duration') {
|
} else if (prop?.type === 'integer' || key === 'display_duration') {
|
||||||
current[finalKey] = parseInt(value) || 0;
|
flatConfig[key] = parseInt(value) || 0;
|
||||||
} else if (prop?.type === 'number') {
|
} else if (prop?.type === 'number') {
|
||||||
current[finalKey] = parseFloat(value) || 0;
|
flatConfig[key] = parseFloat(value) || 0;
|
||||||
} else if (prop?.type === 'boolean') {
|
} else if (prop?.type === 'boolean') {
|
||||||
current[finalKey] = value === 'true' || value === true;
|
flatConfig[key] = value === 'true' || value === true;
|
||||||
} else {
|
} else {
|
||||||
current[finalKey] = value;
|
flatConfig[key] = value;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Convert flat dot-notation keys to nested object using schema-aware matching
|
||||||
|
const config = dotToNested(flatConfig, schema);
|
||||||
|
|
||||||
// Deep merge with existing config to preserve nested structures
|
// Deep merge with existing config to preserve nested structures
|
||||||
function deepMerge(target, source) {
|
function deepMerge(target, source) {
|
||||||
|
|||||||
Reference in New Issue
Block a user