Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding barrel feature #1573

Merged
merged 69 commits into from
Apr 17, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
69 commits
Select commit Hold shift + click to select a range
7f841e5
Optionally adding `barrel` crate to diesel_migrations
spacekookie Feb 15, 2018
5739101
Initial support for barrel migrations in diesel cli
spacekookie Feb 15, 2018
f258e56
Using the barrel `diesel` feature
spacekookie Feb 15, 2018
439620e
Changing barrel call signature to build Migration on diesel side
spacekookie Feb 18, 2018
4cd7b79
Connecting barrel to diesel migration layouts
spacekookie Feb 20, 2018
25267f9
Formatting with rustfmt
spacekookie Feb 20, 2018
9b95920
Changing generated migration function signatures
spacekookie Feb 21, 2018
b4bacf4
Cleaning up migration process slightly
spacekookie Feb 22, 2018
67fd97e
Pointing barrel to github version
spacekookie Feb 22, 2018
4f64f21
Implementing requested changes in diesel_cli
spacekookie Feb 22, 2018
e46570d
Fix git repository link
spacekookie Feb 22, 2018
7f4b65b
Preliminarily commiting refactoring changes
spacekookie Feb 22, 2018
487404d
Moving Migration trait to core diesel library
spacekookie Feb 27, 2018
a4cb6dd
Adjusting the module comment formatting to let CI pass
spacekookie Feb 27, 2018
f0b153f
Moving MigrationName back to migrations_internals
spacekookie Feb 27, 2018
1f74f90
Adding new import to fix some Migration tests
spacekookie Mar 2, 2018
c4cee3d
Changing the migration generation function signature
spacekookie Mar 2, 2018
c62e6ea
Another clippy change
spacekookie Mar 2, 2018
1aa7232
Renaming external feature flag. Changing barrel version to non-git
spacekookie Mar 4, 2018
5f4b402
Changing barrel version to non-git in diesel_cli
spacekookie Mar 4, 2018
7da5525
Adjust the diesel feature flag on barrel
spacekookie Mar 6, 2018
1887333
Re-exporting diesel::migration::* to avoid a breaking change
spacekookie Mar 6, 2018
bac30c0
Cleaning up diesel_migration exports
spacekookie Mar 6, 2018
1aaaf5c
Adjusting a few function calls into barrel to the v0.2.0 API
spacekookie Mar 6, 2018
a3275f9
Re-adding more re-exports to diesel_migrations
spacekookie Mar 6, 2018
6f374e3
Changing feature and type flag names in diesel_cli
spacekookie Mar 23, 2018
e0e97d6
Making error enums non exhaustive. Only exporting speific error types
spacekookie Mar 24, 2018
ed2b8a1
Breaking formatting
spacekookie Mar 24, 2018
b650a55
Fixing formatting again
spacekookie Mar 24, 2018
bc1607c
Touching code to make github see changes
spacekookie Mar 24, 2018
a27d1d4
Changing formatting back to the way it's meant to be
spacekookie Mar 24, 2018
4fdfb3d
Handling __NonExaustive options for migration errors
spacekookie Mar 24, 2018
8c025b6
Not exposing the entire mirations/error module anymore
spacekookie Mar 24, 2018
feed83f
Fixing some formatting issues
spacekookie Mar 25, 2018
df26c2b
Optionally adding `barrel` crate to diesel_migrations
spacekookie Feb 15, 2018
4e2668b
Initial support for barrel migrations in diesel cli
spacekookie Feb 15, 2018
3c992c4
Changing barrel call signature to build Migration on diesel side
spacekookie Feb 18, 2018
8468bc2
Connecting barrel to diesel migration layouts
spacekookie Feb 20, 2018
9a9b01b
Formatting with rustfmt
spacekookie Feb 20, 2018
56234c8
Changing generated migration function signatures
spacekookie Feb 21, 2018
daa998d
Cleaning up migration process slightly
spacekookie Feb 22, 2018
a14287b
Pointing barrel to github version
spacekookie Feb 22, 2018
3101a99
Implementing requested changes in diesel_cli
spacekookie Feb 22, 2018
d5330d2
Preliminarily commiting refactoring changes
spacekookie Feb 22, 2018
a7cd615
Moving Migration trait to core diesel library
spacekookie Feb 27, 2018
9dd24cf
Adjusting the module comment formatting to let CI pass
spacekookie Feb 27, 2018
8960d05
Moving MigrationName back to migrations_internals
spacekookie Feb 27, 2018
cfc453d
Adding new import to fix some Migration tests
spacekookie Mar 2, 2018
2fc3637
Changing the migration generation function signature
spacekookie Mar 2, 2018
6b902d4
Another clippy change
spacekookie Mar 2, 2018
051661f
Renaming external feature flag. Changing barrel version to non-git
spacekookie Mar 4, 2018
b4a2b3b
Changing barrel version to non-git in diesel_cli
spacekookie Mar 4, 2018
5029b64
Adjust the diesel feature flag on barrel
spacekookie Mar 6, 2018
4eebacf
Re-exporting diesel::migration::* to avoid a breaking change
spacekookie Mar 6, 2018
03e0248
Cleaning up diesel_migration exports
spacekookie Mar 6, 2018
b7bd404
Adjusting a few function calls into barrel to the v0.2.0 API
spacekookie Mar 6, 2018
3bb1233
Re-adding more re-exports to diesel_migrations
spacekookie Mar 6, 2018
a5d375d
Changing feature and type flag names in diesel_cli
spacekookie Mar 23, 2018
e8bd6f5
Making error enums non exhaustive. Only exporting speific error types
spacekookie Mar 24, 2018
6693e87
Breaking formatting
spacekookie Mar 24, 2018
13e8caa
Fixing formatting again
spacekookie Mar 24, 2018
0dc23b8
Touching code to make github see changes
spacekookie Mar 24, 2018
8595def
Changing formatting back to the way it's meant to be
spacekookie Mar 24, 2018
f84f394
Handling __NonExaustive options for migration errors
spacekookie Mar 24, 2018
c1a217f
Not exposing the entire mirations/error module anymore
spacekookie Mar 24, 2018
5168672
Fixing some formatting issues
spacekookie Mar 25, 2018
2086b99
Cleaning a bad merge artefact
Apr 16, 2018
ed20737
Resolving rebase differences
Apr 16, 2018
ef98ddc
Fixing a bad rebase
Apr 16, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions diesel/src/migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pub use self::errors::*;

use connection::SimpleConnection;
use std::path::Path;
use std::fmt;

/// Represents a migration that interacts with diesel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed that's it's super hard to see what's up with SimpleConnection – it's not public and the only way aside from reading the code where you can find it is in the docs for Connection (which it's a supertrait of). Not something for this PR to fix but seemed worth commenting on.

pub trait Migration {
Expand All @@ -19,3 +20,67 @@ pub trait Migration {
None
}
}

/// A migration name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we document what this actually does rather than just repeating the name of the type?

#[allow(missing_debug_implementations)]
#[derive(Clone, Copy)]
pub struct MigrationName<'a> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this need to move?

/// Wraps around a migration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we provide more information than just repeating the type?

pub migration: &'a Migration,
}

/// Get the name of a migration
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we provide more information than just repeating the function/argument names? Maybe at least a usage example?

pub fn name(migration: &Migration) -> MigrationName {
MigrationName {
migration: migration,
}
}

impl<'a> fmt::Display for MigrationName<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let file_name = self.migration
.file_path()
.and_then(|file_path| file_path.file_name())
.and_then(|file| file.to_str());
if let Some(name) = file_name {
f.write_str(name)?;
} else {
f.write_str(self.migration.version())?;
}
Ok(())
}
}

impl Migration for Box<Migration> {
fn version(&self) -> &str {
(&**self).version()
}

fn run(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).run(conn)
}

fn revert(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).revert(conn)
}
fn file_path(&self) -> Option<&Path> {
(&**self).file_path()
}
}

impl<'a> Migration for &'a Migration {
fn version(&self) -> &str {
(&**self).version()
}

fn run(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).run(conn)
}

fn revert(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).revert(conn)
}
fn file_path(&self) -> Option<&Path> {
(&**self).file_path()
}
}
1 change: 1 addition & 0 deletions diesel_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ lint = ["clippy"]
postgres = ["diesel/postgres", "infer_schema_internals/postgres", "url"]
sqlite = ["diesel/sqlite", "infer_schema_internals/sqlite"]
mysql = ["diesel/mysql", "infer_schema_internals/mysql", "url"]
barrels = ["migrations_internals/barrel", "barrel"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why barrels?


[[test]]
name = "tests"
Expand Down
3 changes: 1 addition & 2 deletions diesel_migrations/migrations_internals/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,10 @@ pub use self::migration::*;
use std::fs::DirEntry;
use std::io::{stdout, Write};

use diesel::migration::errors::*;
use diesel::migration::*;
use diesel::expression_methods::*;
use diesel::{Connection, QueryDsl, QueryResult, RunQueryDsl};
use self::schema::__diesel_schema_migrations::dsl::*;
use self::migration::Migration;

use std::env;
use std::path::{Path, PathBuf};
Expand Down
112 changes: 2 additions & 110 deletions diesel_migrations/migrations_internals/src/migration.rs
Original file line number Diff line number Diff line change
@@ -1,52 +1,12 @@
use diesel::connection::SimpleConnection;
use diesel::migration::errors::*;
use diesel::migration::*;

use std::path::{Path, PathBuf};
use std::fmt;

pub trait Migration {
fn version(&self) -> &str;
fn run(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError>;
fn revert(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError>;
fn file_path(&self) -> Option<&Path> {
None
}
}

#[allow(missing_debug_implementations)]
#[derive(Clone, Copy)]
pub struct MigrationName<'a> {
pub migration: &'a Migration,
}

pub fn name(migration: &Migration) -> MigrationName {
MigrationName {
migration: migration,
}
}

impl<'a> fmt::Display for MigrationName<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let file_name = self.migration
.file_path()
.and_then(|file_path| file_path.file_name())
.and_then(|file| file.to_str());
if let Some(name) = file_name {
f.write_str(name)?;
} else {
f.write_str(self.migration.version())?;
}
Ok(())
}
}

pub fn migration_from(path: PathBuf) -> Result<Box<Migration>, MigrationError> {
#[cfg(feature = "barrel")]
match ::barrel::diesel::migration_from(&path) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about changing this to if let Some(migration) = ::barrel::diesel::migration_from(path)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be more elegant although rustc tells me attributes are not yet allowed on 'if' expressions. The stabilisation is being tracked here: rust-lang/rust#15701.

I'd suggest we change the syntax when the feature becomes stable, as it is more elegant.

Some(sql) => {
let version = try!(version_from_path(&path));
return barrel_to_migration(path, version, sql);
}
Some(migration) => return Ok(migration),
None => {}
}

Expand All @@ -58,74 +18,6 @@ pub fn migration_from(path: PathBuf) -> Result<Box<Migration>, MigrationError> {
}
}

#[cfg(feature = "barrel")]
struct BarrelMigration(PathBuf, String, String, String);

#[cfg(feature = "barrel")]
impl Migration for BarrelMigration {
fn file_path(&self) -> Option<&Path> {
Some(self.0.as_path())
}

fn version(&self) -> &str {
&self.1
}

fn run(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
try!(conn.batch_execute(&self.2));
Ok(())
}

fn revert(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
try!(conn.batch_execute(&self.3));
Ok(())
}
}

#[cfg(feature = "barrel")]
fn barrel_to_migration(
path: PathBuf,
version: String,
sql: (String, String),
) -> Result<Box<Migration>, MigrationError> {
Ok(Box::new(BarrelMigration(path, version, sql.0, sql.1)))
}


impl Migration for Box<Migration> {
fn version(&self) -> &str {
(&**self).version()
}

fn run(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).run(conn)
}

fn revert(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).revert(conn)
}
fn file_path(&self) -> Option<&Path> {
(&**self).file_path()
}
}

impl<'a> Migration for &'a Migration {
fn version(&self) -> &str {
(&**self).version()
}

fn run(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).run(conn)
}

fn revert(&self, conn: &SimpleConnection) -> Result<(), RunMigrationsError> {
(&**self).revert(conn)
}
fn file_path(&self) -> Option<&Path> {
(&**self).file_path()
}
}

fn valid_sql_migration_directory(path: &Path) -> bool {
file_names(path)
.map(|files| files.contains(&"down.sql".into()) && files.contains(&"up.sql".into()))
Expand Down
8 changes: 2 additions & 6 deletions diesel_migrations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,6 @@ extern crate migrations_macros;
#[doc(hidden)]
pub use migrations_macros::*;
#[doc(inline)]
pub use migrations_internals::migration::Migration;
#[doc(inline)]
pub use migrations_internals::MigrationName;
#[doc(inline)]
pub use migrations_internals::MigrationConnection;
// #[doc(inline)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the commented lines?

// pub use migrations_internals::MigrationError;
Expand Down Expand Up @@ -117,8 +113,8 @@ pub use migrations_internals::find_migrations_directory;
pub use migrations_internals::search_for_migrations_directory;
#[doc(inline)]
pub use migrations_internals::version_from_path;
#[doc(inline)]
pub use migrations_internals::name;
// #[doc(inline)]
// pub use migrations_internals::name;

pub mod connection {
#[doc(inline)]
Expand Down