Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 31e6ddf

Browse files
fix: batched changes (#428)
1 parent a621abc commit 31e6ddf

File tree

2 files changed

+172
-38
lines changed

2 files changed

+172
-38
lines changed

‎crates/pgt_lsp/tests/server.rs‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,3 +1542,139 @@ async fn extends_config(test_db: PgPool) -> Result<()> {
15421542

15431543
Ok(())
15441544
}
1545+
1546+
#[sqlx::test(migrator = "pgt_test_utils::MIGRATIONS")]
1547+
async fn test_multiple_content_changes_single_request(test_db: PgPool) -> Result<()> {
1548+
let factory = ServerFactory::default();
1549+
let mut fs = MemoryFileSystem::default();
1550+
1551+
let setup = r#"
1552+
create table public.campaign_contact_list (
1553+
id serial primary key,
1554+
contact_list_id integer
1555+
);
1556+
1557+
create table public.contact_list (
1558+
id serial primary key,
1559+
name varchar(255)
1560+
);
1561+
1562+
create table public.journey_node_contact_list (
1563+
id serial primary key,
1564+
contact_list_id integer
1565+
);
1566+
"#;
1567+
1568+
test_db
1569+
.execute(setup)
1570+
.await
1571+
.expect("Failed to setup test database");
1572+
1573+
let mut conf = PartialConfiguration::init();
1574+
conf.merge_with(PartialConfiguration {
1575+
db: Some(PartialDatabaseConfiguration {
1576+
database: Some(
1577+
test_db
1578+
.connect_options()
1579+
.get_database()
1580+
.unwrap()
1581+
.to_string(),
1582+
),
1583+
..Default::default()
1584+
}),
1585+
..Default::default()
1586+
});
1587+
fs.insert(
1588+
url!("postgrestools.jsonc").to_file_path().unwrap(),
1589+
serde_json::to_string_pretty(&conf).unwrap(),
1590+
);
1591+
1592+
let (service, client) = factory
1593+
.create_with_fs(None, DynRef::Owned(Box::new(fs)))
1594+
.into_inner();
1595+
1596+
let (stream, sink) = client.split();
1597+
let mut server = Server::new(service);
1598+
1599+
let (sender, mut receiver) = channel(CHANNEL_BUFFER_SIZE);
1600+
let reader = tokio::spawn(client_handler(stream, sink, sender));
1601+
1602+
server.initialize().await?;
1603+
server.initialized().await?;
1604+
1605+
server.load_configuration().await?;
1606+
1607+
// Open document with initial content that matches the log trace
1608+
let initial_content = r#"
1609+
1610+
1611+
1612+
ALTER TABLE ONLY "public"."campaign_contact_list"
1613+
ADD CONSTRAINT "campaign_contact_list_contact_list_id_fkey" FOREIGN KEY ("contact_list_id") REFERENCES "public"."contact_list"("id") ON UPDATE RESTRICT ON DELETE CASCADE;
1614+
"#;
1615+
1616+
server.open_document(initial_content).await?;
1617+
1618+
// Apply multiple content changes in a single request, similar to the log trace
1619+
// This simulates changing "campaign" to "journey_node" in two places simultaneously
1620+
server
1621+
.change_document(
1622+
4,
1623+
vec![
1624+
// First change: line 4, character 27-35 (changing "campaign" to "journey_node")
1625+
TextDocumentContentChangeEvent {
1626+
range: Some(Range {
1627+
start: Position {
1628+
line: 4,
1629+
character: 27,
1630+
},
1631+
end: Position {
1632+
line: 4,
1633+
character: 35,
1634+
},
1635+
}),
1636+
range_length: Some(8),
1637+
text: "journey_node".to_string(),
1638+
},
1639+
// Second change: line 5, character 20-28 (changing "campaign" to "journey_node")
1640+
TextDocumentContentChangeEvent {
1641+
range: Some(Range {
1642+
start: Position {
1643+
line: 5,
1644+
character: 20,
1645+
},
1646+
end: Position {
1647+
line: 5,
1648+
character: 28,
1649+
},
1650+
}),
1651+
range_length: Some(8),
1652+
text: "journey_node".to_string(),
1653+
},
1654+
],
1655+
)
1656+
.await?;
1657+
1658+
// make sure there is no diagnostics
1659+
let notification = tokio::time::timeout(Duration::from_secs(2), async {
1660+
loop {
1661+
match receiver.next().await {
1662+
Some(ServerNotification::PublishDiagnostics(msg)) => {
1663+
if !msg.diagnostics.is_empty() {
1664+
return true;
1665+
}
1666+
}
1667+
_ => continue,
1668+
}
1669+
}
1670+
})
1671+
.await
1672+
.is_ok();
1673+
1674+
assert!(!notification, "did not expect diagnostics");
1675+
1676+
server.shutdown().await?;
1677+
reader.abort();
1678+
1679+
Ok(())
1680+
}

‎crates/pgt_workspace/src/workspace/server/change.rs‎

Lines changed: 36 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -69,28 +69,19 @@ impl Document {
6969
// this is why we pass both varaints to apply_change
7070
let mut changes = Vec::new();
7171

72-
let mut offset: i64 = 0;
73-
74-
for change in &change.changes {
75-
let adjusted_change = if offset != 0 && change.range.is_some() {
76-
&ChangeParams {
77-
text: change.text.clone(),
78-
range: change.range.map(|range| {
79-
let start = u32::from(range.start());
80-
let end = u32::from(range.end());
81-
TextRange::new(
82-
TextSize::from((start as i64 + offset).try_into().unwrap_or(0)),
83-
TextSize::from((end as i64 + offset).try_into().unwrap_or(0)),
84-
)
85-
}),
86-
}
87-
} else {
88-
change
89-
};
90-
91-
changes.extend(self.apply_change(adjusted_change, change));
92-
93-
offset += adjusted_change.change_size();
72+
let mut change_indices: Vec<usize> = (0..change.changes.len()).collect();
73+
change_indices.sort_by(|&a, &b| {
74+
match (change.changes[a].range, change.changes[b].range) {
75+
(Some(range_a), Some(range_b)) => range_b.start().cmp(&range_a.start()),
76+
(Some(_), None) => std::cmp::Ordering::Greater, // full changes will never be sent in a batch so this does not matter
77+
(None, Some(_)) => std::cmp::Ordering::Less,
78+
(None, None) => std::cmp::Ordering::Equal,
79+
}
80+
});
81+
82+
// Sort changes by start position and process from last to first to avoid position invalidation
83+
for &idx in &change_indices {
84+
changes.extend(self.apply_change(&change.changes[idx]));
9485
}
9586

9687
self.version = change.version;
@@ -245,11 +236,7 @@ impl Document {
245236
///
246237
/// * `change`: The range-adjusted change to use for statement changes
247238
/// * `original_change`: The original change to use for text changes (yes, this is a bit confusing, and we might want to refactor this entire thing at some point.)
248-
fn apply_change(
249-
&mut self,
250-
change: &ChangeParams,
251-
original_change: &ChangeParams,
252-
) -> Vec<StatementChange> {
239+
fn apply_change(&mut self, change: &ChangeParams) -> Vec<StatementChange> {
253240
// if range is none, we have a full change
254241
if change.range.is_none() {
255242
// doesnt matter what change since range is null
@@ -265,7 +252,7 @@ impl Document {
265252

266253
let change_range = change.range.unwrap();
267254
let previous_content = self.content.clone();
268-
let new_content = original_change.apply_to_text(&self.content);
255+
let new_content = change.apply_to_text(&self.content);
269256

270257
// we first need to determine the affected range and all affected statements, as well as
271258
// the index of the prev and the next statement, if any. The full affected range is the
@@ -1676,9 +1663,15 @@ mod tests {
16761663
}
16771664

16781665
#[test]
1679-
fn test_content_out_of_sync() {
1666+
fn test_another_issue() {
16801667
let path = PgTPath::new("test.sql");
1681-
let initial_content = "select 1, 2, 2232231313393319 from unknown_users;\n";
1668+
let initial_content = r#"
1669+
1670+
1671+
1672+
ALTER TABLE ONLY "public"."campaign_contact_list"
1673+
ADD CONSTRAINT "campaign_contact_list_contact_list_id_fkey" FOREIGN KEY ("contact_list_id") REFERENCES "public"."contact_list"("id") ON UPDATE RESTRICT ON DELETE CASCADE;
1674+
"#;
16821675

16831676
let mut doc = Document::new(initial_content.to_string(), 0);
16841677

@@ -1687,22 +1680,27 @@ mod tests {
16871680
version: 1,
16881681
changes: vec![
16891682
ChangeParams {
1690-
range: Some(TextRange::new(29.into(), 29.into())),
1691-
text: "3".to_string(),
1683+
range: Some(TextRange::new(31.into(), 39.into())),
1684+
text: "journey_node".to_string(),
16921685
},
16931686
ChangeParams {
1694-
range: Some(TextRange::new(30.into(), 30.into())),
1695-
text: "1".to_string(),
1687+
range: Some(TextRange::new(74.into(), 82.into())),
1688+
text: "journey_node".to_string(),
16961689
},
16971690
],
16981691
};
16991692

17001693
let _changes = doc.apply_file_change(&change1);
17011694

1702-
assert_eq!(
1703-
doc.content,
1704-
"select 1, 2, 223223131339331931 from unknown_users;\n"
1705-
);
1695+
let expected_content = r#"
1696+
1697+
1698+
1699+
ALTER TABLE ONLY "public"."journey_node_contact_list"
1700+
ADD CONSTRAINT "journey_node_contact_list_contact_list_id_fkey" FOREIGN KEY ("contact_list_id") REFERENCES "public"."contact_list"("id") ON UPDATE RESTRICT ON DELETE CASCADE;
1701+
"#;
1702+
1703+
assert_eq!(doc.content, expected_content);
17061704

17071705
assert_document_integrity(&doc);
17081706
}

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /