From c84f584865c58d90b28221c8765b5954a9f396de Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Sun, 14 Jan 2024 10:19:27 +0800 Subject: [PATCH 1/4] Use pretty json in .butane files --- butane_core/src/db/mod.rs | 2 +- butane_core/src/migrations/fs.rs | 8 ++ butane_core/src/migrations/fsmigrations.rs | 15 ++- butane_core/tests/many.rs | 1 + .../.butane/migrations/current/Blog.table | 32 +++++- .../.butane/migrations/current/Post.table | 100 +++++++++++++++++- .../migrations/current/Post_tags_Many.table | 41 ++++++- .../.butane/migrations/current/Tag.table | 19 +++- 8 files changed, 209 insertions(+), 9 deletions(-) create mode 100644 butane_core/tests/many.rs diff --git a/butane_core/src/db/mod.rs b/butane_core/src/db/mod.rs index 2a14970a..755e8271 100644 --- a/butane_core/src/db/mod.rs +++ b/butane_core/src/db/mod.rs @@ -107,7 +107,7 @@ impl ConnectionSpec { pub fn save(&self, path: &Path) -> Result<()> { let path = conn_complete_if_dir(path); let mut f = fs::File::create(path)?; - f.write_all(serde_json::to_string(self)?.as_bytes()) + f.write_all(serde_json::to_string_pretty(self)?.as_bytes()) .map_err(|e| e.into()) } /// Load a previously saved connection spec diff --git a/butane_core/src/migrations/fs.rs b/butane_core/src/migrations/fs.rs index 705fa2eb..a0ee704e 100644 --- a/butane_core/src/migrations/fs.rs +++ b/butane_core/src/migrations/fs.rs @@ -13,6 +13,8 @@ pub trait Filesystem: Debug { fn list_dir(&self, path: &Path) -> std::io::Result>; /// Opens a file for writing. Creates it if it does not exist. Truncates it otherwise. fn write(&self, path: &Path) -> std::io::Result>; + /// Opens a file for writing in append mode. Fails if file does not exist. + fn append(&self, path: &Path) -> std::io::Result>; /// Opens a file for reading. fn read(&self, path: &Path) -> std::io::Result>; } @@ -32,6 +34,12 @@ impl Filesystem for OsFilesystem { fn write(&self, path: &Path) -> std::io::Result> { std::fs::File::create(path).map(|f| Box::new(f) as Box) } + fn append(&self, path: &Path) -> std::io::Result> { + std::fs::OpenOptions::new() + .append(true) + .open(path) + .map(|f| Box::new(f) as Box) + } fn read(&self, path: &Path) -> std::io::Result> { std::fs::File::open(path).map(|f| Box::new(f) as Box) } diff --git a/butane_core/src/migrations/fsmigrations.rs b/butane_core/src/migrations/fsmigrations.rs index 8480fdb0..c3c1a927 100644 --- a/butane_core/src/migrations/fsmigrations.rs +++ b/butane_core/src/migrations/fsmigrations.rs @@ -68,7 +68,7 @@ impl FsMigration { } fn write_info(&self, info: &MigrationInfo) -> Result<()> { - self.write_contents("info.json", serde_json::to_string(info)?.as_bytes()) + self.write_contents("info.json", serde_json::to_string_pretty(info)?.as_bytes()) } fn write_sql(&self, name: &str, sql: &str) -> Result<()> { @@ -95,7 +95,14 @@ impl FsMigration { self.fs .write(&path)? .write_all(contents) - .map_err(|e| e.into()) + .map_err(>::into)?; + if contents[contents.len() - 1] != b'\n' { + self.fs + .append(&path)? + .write(b"\n") + .map_err(>::into)?; + } + Ok(()) } fn ensure_dir(&self) -> Result<()> { @@ -124,7 +131,7 @@ impl MigrationMut for FsMigration { fn write_table(&mut self, table: &ATable) -> Result<()> { self.write_contents( &format!("{}.table", table.name), - serde_json::to_string(table)?.as_bytes(), + serde_json::to_string_pretty(table)?.as_bytes(), ) } @@ -269,7 +276,7 @@ impl FsMigrations { fn save_state(&mut self, state: &MigrationsState) -> Result<()> { let path = self.root.join("state.json"); let mut f = self.fs.write(&path)?; - f.write_all(serde_json::to_string(state)?.as_bytes()) + f.write_all(serde_json::to_string_pretty(state)?.as_bytes()) .map_err(|e| e.into()) } /// Detach the latest migration from the list of migrations, diff --git a/butane_core/tests/many.rs b/butane_core/tests/many.rs new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/butane_core/tests/many.rs @@ -0,0 +1 @@ + diff --git a/examples/getting_started/.butane/migrations/current/Blog.table b/examples/getting_started/.butane/migrations/current/Blog.table index 13a5d316..6dbe6909 100644 --- a/examples/getting_started/.butane/migrations/current/Blog.table +++ b/examples/getting_started/.butane/migrations/current/Blog.table @@ -1 +1,31 @@ -{"name":"Blog","columns":[{"name":"id","sqltype":{"KnownId":{"Ty":"BigInt"}},"nullable":false,"pk":true,"auto":true,"unique":false,"default":null},{"name":"name","sqltype":{"KnownId":{"Ty":"Text"}},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null}]} \ No newline at end of file +{ + "name": "Blog", + "columns": [ + { + "name": "id", + "sqltype": { + "KnownId": { + "Ty": "BigInt" + } + }, + "nullable": false, + "pk": true, + "auto": true, + "unique": false, + "default": null + }, + { + "name": "name", + "sqltype": { + "KnownId": { + "Ty": "Text" + } + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null + } + ] +} diff --git a/examples/getting_started/.butane/migrations/current/Post.table b/examples/getting_started/.butane/migrations/current/Post.table index 6b030e1d..fc624a57 100644 --- a/examples/getting_started/.butane/migrations/current/Post.table +++ b/examples/getting_started/.butane/migrations/current/Post.table @@ -1 +1,99 @@ -{"name":"Post","columns":[{"name":"id","sqltype":{"KnownId":{"Ty":"Int"}},"nullable":false,"pk":true,"auto":true,"unique":false,"default":null},{"name":"title","sqltype":{"KnownId":{"Ty":"Text"}},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null},{"name":"body","sqltype":{"KnownId":{"Ty":"Text"}},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null},{"name":"published","sqltype":{"KnownId":{"Ty":"Bool"}},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null},{"name":"blog","sqltype":{"Deferred":"PK:Blog"},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null,"reference":{"Deferred":{"Deferred":"PK:Blog"}}},{"name":"byline","sqltype":{"KnownId":{"Ty":"Text"}},"nullable":true,"pk":false,"auto":false,"unique":false,"default":null},{"name":"likes","sqltype":{"KnownId":{"Ty":"Int"}},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null}]} \ No newline at end of file +{ + "name": "Post", + "columns": [ + { + "name": "id", + "sqltype": { + "KnownId": { + "Ty": "Int" + } + }, + "nullable": false, + "pk": true, + "auto": true, + "unique": false, + "default": null + }, + { + "name": "title", + "sqltype": { + "KnownId": { + "Ty": "Text" + } + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null + }, + { + "name": "body", + "sqltype": { + "KnownId": { + "Ty": "Text" + } + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null + }, + { + "name": "published", + "sqltype": { + "KnownId": { + "Ty": "Bool" + } + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null + }, + { + "name": "blog", + "sqltype": { + "Deferred": "PK:Blog" + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null, + "reference": { + "Deferred": { + "Deferred": "PK:Blog" + } + } + }, + { + "name": "byline", + "sqltype": { + "KnownId": { + "Ty": "Text" + } + }, + "nullable": true, + "pk": false, + "auto": false, + "unique": false, + "default": null + }, + { + "name": "likes", + "sqltype": { + "KnownId": { + "Ty": "Int" + } + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null + } + ] +} diff --git a/examples/getting_started/.butane/migrations/current/Post_tags_Many.table b/examples/getting_started/.butane/migrations/current/Post_tags_Many.table index 6c5704f7..36afc8ce 100644 --- a/examples/getting_started/.butane/migrations/current/Post_tags_Many.table +++ b/examples/getting_started/.butane/migrations/current/Post_tags_Many.table @@ -1 +1,40 @@ -{"name":"Post_tags_Many","columns":[{"name":"owner","sqltype":{"KnownId":{"Ty":"Int"}},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null,"reference":{"Literal":{"table_name":"Post","column_name":"id"}}},{"name":"has","sqltype":{"Deferred":"PK:Tag"},"nullable":false,"pk":false,"auto":false,"unique":false,"default":null,"reference":{"Deferred":{"Deferred":"PK:Tag"}}}]} \ No newline at end of file +{ + "name": "Post_tags_Many", + "columns": [ + { + "name": "owner", + "sqltype": { + "KnownId": { + "Ty": "Int" + } + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null, + "reference": { + "Literal": { + "table_name": "Post", + "column_name": "id" + } + } + }, + { + "name": "has", + "sqltype": { + "Deferred": "PK:Tag" + }, + "nullable": false, + "pk": false, + "auto": false, + "unique": false, + "default": null, + "reference": { + "Deferred": { + "Deferred": "PK:Tag" + } + } + } + ] +} diff --git a/examples/getting_started/.butane/migrations/current/Tag.table b/examples/getting_started/.butane/migrations/current/Tag.table index a5117916..784abd8b 100644 --- a/examples/getting_started/.butane/migrations/current/Tag.table +++ b/examples/getting_started/.butane/migrations/current/Tag.table @@ -1 +1,18 @@ -{"name":"Tag","columns":[{"name":"tag","sqltype":{"KnownId":{"Ty":"Text"}},"nullable":false,"pk":true,"auto":false,"unique":false,"default":null}]} \ No newline at end of file +{ + "name": "Tag", + "columns": [ + { + "name": "tag", + "sqltype": { + "KnownId": { + "Ty": "Text" + } + }, + "nullable": false, + "pk": true, + "auto": false, + "unique": false, + "default": null + } + ] +} From 043327e231f7a363836db0e4753bb8ef9552d8d5 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Sun, 14 Jan 2024 10:37:12 +0800 Subject: [PATCH 2/4] Append EOL to pretty json explicitly --- butane_core/src/migrations/fsmigrations.rs | 25 ++++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/butane_core/src/migrations/fsmigrations.rs b/butane_core/src/migrations/fsmigrations.rs index c3c1a927..50c389cf 100644 --- a/butane_core/src/migrations/fsmigrations.rs +++ b/butane_core/src/migrations/fsmigrations.rs @@ -68,11 +68,14 @@ impl FsMigration { } fn write_info(&self, info: &MigrationInfo) -> Result<()> { - self.write_contents("info.json", serde_json::to_string_pretty(info)?.as_bytes()) + self.write_contents( + "info.json", + &[serde_json::to_string_pretty(info)?.as_bytes(), b"\n"].concat(), + ) } fn write_sql(&self, name: &str, sql: &str) -> Result<()> { - self.write_contents(&format!("{name}.sql"), sql.as_bytes()) + self.write_contents(&format!("{name}.sql"), &[sql.as_bytes(), b"\n"].concat()) } fn read_sql(&self, backend: &str, direction: &str) -> Result> { @@ -131,7 +134,7 @@ impl MigrationMut for FsMigration { fn write_table(&mut self, table: &ATable) -> Result<()> { self.write_contents( &format!("{}.table", table.name), - serde_json::to_string_pretty(table)?.as_bytes(), + &[serde_json::to_string_pretty(table)?.as_bytes(), b"\n"].concat(), ) } @@ -166,12 +169,16 @@ impl MigrationMut for FsMigration { types.insert(key, sqltype); self.write_contents( TYPES_FILENAME, - serde_json::to_string(&types) - .map_err(|e| { - eprintln!("failed to write types {typefile:?}"); - e - })? - .as_bytes(), + &[ + serde_json::to_string(&types) + .map_err(|e| { + eprintln!("failed to write types {typefile:?}"); + e + })? + .as_bytes(), + b"\n", + ] + .concat(), )?; Ok(()) } From d3f393e8836187db2e0386b57d540a8dededdf93 Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Sun, 14 Jan 2024 12:11:10 +0800 Subject: [PATCH 3/4] Revert "Append EOL to pretty json explicitly" This reverts commit 043327e231f7a363836db0e4753bb8ef9552d8d5. --- butane_core/src/migrations/fsmigrations.rs | 25 ++++++++-------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/butane_core/src/migrations/fsmigrations.rs b/butane_core/src/migrations/fsmigrations.rs index 50c389cf..c3c1a927 100644 --- a/butane_core/src/migrations/fsmigrations.rs +++ b/butane_core/src/migrations/fsmigrations.rs @@ -68,14 +68,11 @@ impl FsMigration { } fn write_info(&self, info: &MigrationInfo) -> Result<()> { - self.write_contents( - "info.json", - &[serde_json::to_string_pretty(info)?.as_bytes(), b"\n"].concat(), - ) + self.write_contents("info.json", serde_json::to_string_pretty(info)?.as_bytes()) } fn write_sql(&self, name: &str, sql: &str) -> Result<()> { - self.write_contents(&format!("{name}.sql"), &[sql.as_bytes(), b"\n"].concat()) + self.write_contents(&format!("{name}.sql"), sql.as_bytes()) } fn read_sql(&self, backend: &str, direction: &str) -> Result> { @@ -134,7 +131,7 @@ impl MigrationMut for FsMigration { fn write_table(&mut self, table: &ATable) -> Result<()> { self.write_contents( &format!("{}.table", table.name), - &[serde_json::to_string_pretty(table)?.as_bytes(), b"\n"].concat(), + serde_json::to_string_pretty(table)?.as_bytes(), ) } @@ -169,16 +166,12 @@ impl MigrationMut for FsMigration { types.insert(key, sqltype); self.write_contents( TYPES_FILENAME, - &[ - serde_json::to_string(&types) - .map_err(|e| { - eprintln!("failed to write types {typefile:?}"); - e - })? - .as_bytes(), - b"\n", - ] - .concat(), + serde_json::to_string(&types) + .map_err(|e| { + eprintln!("failed to write types {typefile:?}"); + e + })? + .as_bytes(), )?; Ok(()) } From e2e35e396fc797e6784fdc68f3f73448432a093c Mon Sep 17 00:00:00 2001 From: John Vandenberg Date: Sun, 14 Jan 2024 12:18:22 +0800 Subject: [PATCH 4/4] Avoid second fs write --- butane_core/src/migrations/fs.rs | 8 -------- butane_core/src/migrations/fsmigrations.rs | 13 ++++++------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/butane_core/src/migrations/fs.rs b/butane_core/src/migrations/fs.rs index a0ee704e..705fa2eb 100644 --- a/butane_core/src/migrations/fs.rs +++ b/butane_core/src/migrations/fs.rs @@ -13,8 +13,6 @@ pub trait Filesystem: Debug { fn list_dir(&self, path: &Path) -> std::io::Result>; /// Opens a file for writing. Creates it if it does not exist. Truncates it otherwise. fn write(&self, path: &Path) -> std::io::Result>; - /// Opens a file for writing in append mode. Fails if file does not exist. - fn append(&self, path: &Path) -> std::io::Result>; /// Opens a file for reading. fn read(&self, path: &Path) -> std::io::Result>; } @@ -34,12 +32,6 @@ impl Filesystem for OsFilesystem { fn write(&self, path: &Path) -> std::io::Result> { std::fs::File::create(path).map(|f| Box::new(f) as Box) } - fn append(&self, path: &Path) -> std::io::Result> { - std::fs::OpenOptions::new() - .append(true) - .open(path) - .map(|f| Box::new(f) as Box) - } fn read(&self, path: &Path) -> std::io::Result> { std::fs::File::open(path).map(|f| Box::new(f) as Box) } diff --git a/butane_core/src/migrations/fsmigrations.rs b/butane_core/src/migrations/fsmigrations.rs index c3c1a927..80ab1365 100644 --- a/butane_core/src/migrations/fsmigrations.rs +++ b/butane_core/src/migrations/fsmigrations.rs @@ -92,16 +92,15 @@ impl FsMigration { fn write_contents(&self, fname: &str, contents: &[u8]) -> Result<()> { self.ensure_dir()?; let path = self.root.join(fname); + let mut contents: Vec = contents.into(); + if contents[contents.len() - 1] != b'\n' { + contents.push(b'\n'); + } self.fs .write(&path)? - .write_all(contents) + .write_all(&contents) .map_err(>::into)?; - if contents[contents.len() - 1] != b'\n' { - self.fs - .append(&path)? - .write(b"\n") - .map_err(>::into)?; - } + Ok(()) }