Skip to content

Commit 60642ab

Browse files
committed
Fixing get object for specific ID not checking compiled working index
1 parent 7288357 commit 60642ab

3 files changed

Lines changed: 246 additions & 22 deletions

File tree

vcs-worker/src/operations/object/object_get_op.rs

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -43,52 +43,47 @@ impl ObjectGetOperation {
4343
request.object_name, resolved_change_id
4444
);
4545

46-
// Get the change to find the object version at that point
47-
let change = self
46+
// Compute the complete object state at this change by walking through history
47+
let object_state = self
4848
.database
4949
.index()
50-
.get_change(&resolved_change_id)
51-
.map_err(|e| ObjectsTreeError::SerializationError(e.to_string()))?
52-
.ok_or_else(|| {
53-
ObjectsTreeError::SerializationError(format!(
54-
"Change '{}' not found",
55-
resolved_change_id
56-
))
57-
})?;
50+
.compute_object_state_at_change(&resolved_change_id)
51+
.map_err(|e| ObjectsTreeError::SerializationError(e.to_string()))?;
5852

59-
// Find the object in the change's modified or added objects
60-
let object_info = change
61-
.modified_objects
62-
.iter()
63-
.chain(change.added_objects.iter())
64-
.find(|obj| {
65-
obj.object_type == VcsObjectType::MooObject && obj.name == request.object_name
66-
})
53+
// Check if the object exists in the compiled state at this change
54+
let version = object_state
55+
.get(&request.object_name)
56+
.copied()
6757
.ok_or_else(|| {
6858
ObjectsTreeError::SerializationError(format!(
69-
"Object '{}' not found in change '{}'",
59+
"Object '{}' not found in compiled state at change '{}'",
7060
request.object_name, resolved_change_id
7161
))
7262
})?;
7363

64+
info!(
65+
"Object '{}' exists at change '{}' with version {}",
66+
request.object_name, resolved_change_id, version
67+
);
68+
7469
// Get the SHA256 for this specific version
7570
let sha256 = self
7671
.database
7772
.refs()
7873
.get_ref(
7974
VcsObjectType::MooObject,
8075
&request.object_name,
81-
Some(object_info.version),
76+
Some(version),
8277
)
8378
.map_err(|e| ObjectsTreeError::SerializationError(e.to_string()))?
8479
.ok_or_else(|| {
8580
ObjectsTreeError::SerializationError(format!(
8681
"Object '{}' version {} not found in refs",
87-
request.object_name, object_info.version
82+
request.object_name, version
8883
))
8984
})?;
9085

91-
(sha256, Some(object_info.version))
86+
(sha256, Some(version))
9287
} else {
9388
info!("Retrieving object '{}'", request.object_name);
9489

vcs-worker/src/providers/index.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,12 @@ pub trait IndexProvider: Send + Sync {
7575
/// Compute complete object list by walking through all changes chronologically
7676
fn compute_complete_object_list(&self) -> ProviderResult<Vec<crate::types::ObjectInfo>>;
7777

78+
/// Compute object state at a specific change ID by walking through history up to and including that change
79+
fn compute_object_state_at_change(
80+
&self,
81+
change_id: &str,
82+
) -> ProviderResult<std::collections::HashMap<String, u64>>;
83+
7884
// ===== SOURCE METHODS =====
7985
/// Get the source URL if this is a clone
8086
fn get_source(&self) -> ProviderResult<Option<String>>;
@@ -661,6 +667,76 @@ impl IndexProvider for IndexProviderImpl {
661667
Ok(object_list)
662668
}
663669

670+
fn compute_object_state_at_change(
671+
&self,
672+
change_id: &str,
673+
) -> ProviderResult<std::collections::HashMap<String, u64>> {
674+
info!(
675+
"Computing object state at change ID '{}'",
676+
change_id
677+
);
678+
679+
// Get all changes in chronological order
680+
let changes_order = self.get_change_order_internal()?;
681+
682+
// Find the position of the target change in the order
683+
let target_position = changes_order
684+
.iter()
685+
.position(|id| id == change_id)
686+
.ok_or_else(|| {
687+
ProviderError::SerializationError(format!(
688+
"Change '{}' not found in change order",
689+
change_id
690+
))
691+
})?;
692+
693+
info!(
694+
"Change '{}' found at position {} of {} total changes",
695+
change_id,
696+
target_position,
697+
changes_order.len()
698+
);
699+
700+
// Use the change operation processor to build state up to this change
701+
let mut processor = Self::create_change_processor();
702+
703+
// Walk through changes up to and including the target change
704+
for (idx, change_id_iter) in changes_order.iter().enumerate() {
705+
if idx > target_position {
706+
break;
707+
}
708+
709+
match self.get_change(change_id_iter)? {
710+
Some(change) => {
711+
info!(
712+
"Processing change '{}' ({}) at position {}/{}: {} added, {} modified, {} deleted, {} renamed",
713+
change.id,
714+
Self::format_change_status(&change.status),
715+
idx + 1,
716+
target_position + 1,
717+
change.added_objects.len(),
718+
change.modified_objects.len(),
719+
change.deleted_objects.len(),
720+
change.renamed_objects.len()
721+
);
722+
723+
processor.process_change(&change);
724+
}
725+
None => {
726+
warn!("Change '{}' not found in database", change_id_iter);
727+
}
728+
}
729+
}
730+
731+
info!(
732+
"Computed state at change '{}' contains {} objects",
733+
change_id,
734+
processor.objects.len()
735+
);
736+
737+
Ok(processor.objects)
738+
}
739+
664740
fn get_source(&self) -> ProviderResult<Option<String>> {
665741
if let Some(data) = self.working_index.get(Self::SOURCE_KEY)? {
666742
Ok(Some(String::from_utf8(data.to_vec()).map_err(|e| {

vcs-worker/tests/operations/object/get_tests.rs

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,3 +1082,156 @@ async fn test_get_empty_lines_preserved() {
10821082
println!("✅ Line format verified");
10831083
println!("\n✅ Test passed: Empty lines and structure preserved in list");
10841084
}
1085+
1086+
#[tokio::test]
1087+
async fn test_get_object_from_compiled_state_at_change() {
1088+
let server = TestServer::start()
1089+
.await
1090+
.expect("Failed to start test server");
1091+
let client = server.client();
1092+
let db = server.db_assertions();
1093+
1094+
println!("Test: Getting an object at a change should work even if not modified in that change (compiled state)");
1095+
1096+
// Step 1: Create and approve object_a in change1
1097+
println!("\nStep 1: Creating change 1 with object_a...");
1098+
client
1099+
.change_create("change1", "test_author", None)
1100+
.await
1101+
.expect("Failed to create change");
1102+
1103+
client
1104+
.object_update_from_file("object_a", "test_object.moo")
1105+
.await
1106+
.expect("Failed to create object");
1107+
1108+
let (change_id1, _) = db.require_top_change();
1109+
client
1110+
.change_approve(&change_id1)
1111+
.await
1112+
.expect("Failed to approve")
1113+
.assert_success("Approve");
1114+
1115+
println!("✅ Change 1 approved with object_a, change ID: {}", change_id1);
1116+
1117+
// Step 2: Create and approve object_b in change2 (object_a still exists but not modified)
1118+
println!("\nStep 2: Creating change 2 with object_b...");
1119+
client
1120+
.change_create("change2", "test_author", None)
1121+
.await
1122+
.expect("Failed to create change");
1123+
1124+
client
1125+
.object_update_from_file("object_b", "detailed_test_object.moo")
1126+
.await
1127+
.expect("Failed to create object");
1128+
1129+
let (change_id2, _) = db.require_top_change();
1130+
client
1131+
.change_approve(&change_id2)
1132+
.await
1133+
.expect("Failed to approve")
1134+
.assert_success("Approve");
1135+
1136+
println!("✅ Change 2 approved with object_b, change ID: {}", change_id2);
1137+
1138+
// Step 3: Create and approve object_c in change3 (object_a and object_b still exist)
1139+
println!("\nStep 3: Creating change 3 with object_c...");
1140+
client
1141+
.change_create("change3", "test_author", None)
1142+
.await
1143+
.expect("Failed to create change");
1144+
1145+
client
1146+
.object_update_from_file("object_c", "test_object.moo")
1147+
.await
1148+
.expect("Failed to create object");
1149+
1150+
let (change_id3, _) = db.require_top_change();
1151+
client
1152+
.change_approve(&change_id3)
1153+
.await
1154+
.expect("Failed to approve")
1155+
.assert_success("Approve");
1156+
1157+
println!("✅ Change 3 approved with object_c, change ID: {}", change_id3);
1158+
1159+
// Step 4: Get object_a at change2 (should work - it exists in compiled state)
1160+
println!("\nStep 4: Getting object_a at change 2 (not modified in that change)...");
1161+
let response = client
1162+
.object_get_at_change("object_a", &change_id2)
1163+
.await
1164+
.expect("Failed to get object_a at change2");
1165+
1166+
response.assert_success("Get object_a at change2");
1167+
let content = list_to_string(&response);
1168+
assert!(
1169+
content.contains("Test Object"),
1170+
"Should retrieve object_a from compiled state at change2"
1171+
);
1172+
println!("✅ object_a retrieved at change 2 (compiled state)");
1173+
1174+
// Step 5: Get object_a at change3 (should work - it exists in compiled state)
1175+
println!("\nStep 5: Getting object_a at change 3 (not modified in that change)...");
1176+
let response = client
1177+
.object_get_at_change("object_a", &change_id3)
1178+
.await
1179+
.expect("Failed to get object_a at change3");
1180+
1181+
response.assert_success("Get object_a at change3");
1182+
let content = list_to_string(&response);
1183+
assert!(
1184+
content.contains("Test Object"),
1185+
"Should retrieve object_a from compiled state at change3"
1186+
);
1187+
println!("✅ object_a retrieved at change 3 (compiled state)");
1188+
1189+
// Step 6: Get object_b at change3 (should work - it exists in compiled state)
1190+
println!("\nStep 6: Getting object_b at change 3 (not modified in that change)...");
1191+
let response = client
1192+
.object_get_at_change("object_b", &change_id3)
1193+
.await
1194+
.expect("Failed to get object_b at change3");
1195+
1196+
response.assert_success("Get object_b at change3");
1197+
let content = list_to_string(&response);
1198+
assert!(
1199+
content.contains("Detailed Test Object"),
1200+
"Should retrieve object_b from compiled state at change3"
1201+
);
1202+
println!("✅ object_b retrieved at change 3 (compiled state)");
1203+
1204+
// Step 7: Try to get object_b at change1 (should fail - doesn't exist yet)
1205+
println!("\nStep 7: Attempting to get object_b at change 1 (before it existed)...");
1206+
let response = client
1207+
.object_get_at_change("object_b", &change_id1)
1208+
.await
1209+
.expect("Request should complete");
1210+
1211+
// Should fail because object_b didn't exist yet at change1
1212+
let result_str = response.get_result_str().unwrap_or("");
1213+
assert!(
1214+
result_str.contains("Error") || result_str.contains("not found"),
1215+
"Should indicate object not found in compiled state at change1, got: {}",
1216+
result_str
1217+
);
1218+
println!("✅ Get failed appropriately (object didn't exist yet)");
1219+
1220+
// Step 8: Try to get object_c at change2 (should fail - doesn't exist yet)
1221+
println!("\nStep 8: Attempting to get object_c at change 2 (before it existed)...");
1222+
let response = client
1223+
.object_get_at_change("object_c", &change_id2)
1224+
.await
1225+
.expect("Request should complete");
1226+
1227+
// Should fail because object_c didn't exist yet at change2
1228+
let result_str = response.get_result_str().unwrap_or("");
1229+
assert!(
1230+
result_str.contains("Error") || result_str.contains("not found"),
1231+
"Should indicate object not found in compiled state at change2, got: {}",
1232+
result_str
1233+
);
1234+
println!("✅ Get failed appropriately (object didn't exist yet)");
1235+
1236+
println!("\n✅ Test passed: Objects can be retrieved from compiled state at any change where they existed");
1237+
}

0 commit comments

Comments
 (0)