Skip to content

Commit ef415e0

Browse files
authored
[rust-guard] Deduplicate owner/repo parsing paths in labels backend (#5548)
## ✨ Enhancement This change removes two duplication hotspots in `guards/github-guard/rust-guard/src/labels/backend.rs` without altering behavior: repeated item-scan logic in owner-type extraction and repeated `full_name`/`fullName` lookup logic in repo ID extraction. The result is a smaller, more maintainable parsing path with unchanged field precedence and fallback behavior. **What does this improve?** - **Owner-type lookup deduplication** - Introduces `find_org_in_items(items, repo_id)` and reuses it for both response shapes: - `{ "items": [...] }` - plain array `[...]` - **Repo ID field lookup deduplication** - Replaces duplicate `if` blocks for `"full_name"` and `"fullName"` with a single ordered loop. - **Regression coverage for refactor paths** - Adds focused tests for: - plain-array owner/org extraction path - camelCase `fullName` repo ID extraction path **Why is this valuable?** - Reduces copy/paste control flow in high-traffic parsing helpers. - Centralizes repo-ID match logic used by owner-type extraction. - Makes future alias additions and edge-case coverage lower-risk. **Implementation approach:** - Extracted shared iterator-based helper: ```rust fn find_org_in_items(items: &[Value], repo_id: &str) -> Option<bool> { items.iter().find_map(|item| { let item_repo_id = repo_id_from_repo_object(item)?; if item_repo_id.eq_ignore_ascii_case(repo_id) { owner_type_from_repo_object(item) } else { None } }) } ``` - Updated `owner_is_org_from_items` to delegate both array shapes to the helper. - Updated `repo_id_from_repo_object` to iterate `["full_name", "fullName"]` in priority order, preserving existing behavior.
2 parents 1f16fe7 + 9012567 commit ef415e0

1 file changed

Lines changed: 42 additions & 1 deletion

File tree

  • guards/github-guard/rust-guard/src/labels

guards/github-guard/rust-guard/src/labels/backend.rs

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1243,6 +1243,47 @@ mod tests {
12431243
});
12441244
assert_eq!(extract_owner_is_org(&response, "myorg/myrepo"), None);
12451245
}
1246+
1247+
#[test]
1248+
fn test_extract_owner_is_org_plain_array_response() {
1249+
let response = serde_json::json!([{
1250+
"full_name": "myorg/myrepo",
1251+
"private": false,
1252+
"owner": { "login": "myorg", "type": "Organization" }
1253+
}]);
1254+
assert_eq!(extract_owner_is_org(&response, "myorg/myrepo"), Some(true));
1255+
}
1256+
1257+
#[test]
1258+
fn test_extract_owner_is_org_first_match_missing_type_returns_none() {
1259+
let response = serde_json::json!({
1260+
"items": [
1261+
{
1262+
"full_name": "myorg/myrepo",
1263+
"private": false,
1264+
"owner": { "login": "myorg" }
1265+
},
1266+
{
1267+
"full_name": "myorg/myrepo",
1268+
"private": false,
1269+
"owner": { "login": "myorg", "type": "Organization" }
1270+
}
1271+
]
1272+
});
1273+
assert_eq!(extract_owner_is_org(&response, "myorg/myrepo"), None);
1274+
}
1275+
1276+
#[test]
1277+
fn test_repo_id_from_repo_object_full_name_camelcase() {
1278+
let item = serde_json::json!({
1279+
"fullName": "myorg/myrepo",
1280+
"owner": { "login": "myorg", "type": "Organization" }
1281+
});
1282+
assert_eq!(
1283+
repo_id_from_repo_object(&item),
1284+
Some("myorg/myrepo".to_string())
1285+
);
1286+
}
12461287
}
12471288

12481289
fn repo_visibility_from_items(value: &Value, repo_id: &str) -> Option<bool> {
@@ -1384,7 +1425,7 @@ fn owner_type_from_repo_object(item: &Value) -> Option<bool> {
13841425
}
13851426

13861427
fn repo_id_from_repo_object(item: &Value) -> Option<String> {
1387-
for field in &["full_name", "fullName"] {
1428+
for field in ["full_name", "fullName"] {
13881429
if let Some(full_name) = item.get(field).and_then(|v| v.as_str()) {
13891430
if !full_name.is_empty() {
13901431
return Some(full_name.to_string());

0 commit comments

Comments
 (0)