Skip to content

Remove license column on crates table #1815

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

Merged
merged 2 commits into from
Sep 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- This file should undo anything in `up.sql`
ALTER TABLE crates ADD COLUMN license VARCHAR;
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
-- ALERT! DESTRUCTIVE MIGRATION
-- DO NOT DEPLOY UNTIL PREVIOUS COMMIT IS DEPLOYED
ALTER TABLE crates DROP COLUMN license;
9 changes: 2 additions & 7 deletions src/controllers/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,12 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {
documentation: new_crate.documentation.as_ref().map(|s| &**s),
readme: new_crate.readme.as_ref().map(|s| &**s),
repository: repo.as_ref().map(String::as_str),
license: new_crate.license.as_ref().map(|s| &**s),
max_upload_size: None,
};

let license_file = new_crate.license_file.as_ref().map(|s| &**s);
let krate = persist.create_or_update(
&conn,
license_file,
user.id,
Some(&app.config.publish_rate_limit),
)?;
let krate =
persist.create_or_update(&conn, user.id, Some(&app.config.publish_rate_limit))?;

let owners = krate.owners(&conn)?;
if user.rights(req.app(), &owners)? < Rights::Publish {
Expand Down
31 changes: 2 additions & 29 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ pub struct Crate {
pub description: Option<String>,
pub homepage: Option<String>,
pub documentation: Option<String>,
pub license: Option<String>,
pub repository: Option<String>,
pub max_upload_size: Option<i32>,
}
Expand All @@ -58,7 +57,6 @@ type AllColumns = (
crates::description,
crates::homepage,
crates::documentation,
crates::license,
crates::repository,
crates::max_upload_size,
);
Expand All @@ -72,7 +70,6 @@ pub const ALL_COLUMNS: AllColumns = (
crates::description,
crates::homepage,
crates::documentation,
crates::license,
crates::repository,
crates::max_upload_size,
);
Expand All @@ -97,20 +94,18 @@ pub struct NewCrate<'a> {
pub readme: Option<&'a str>,
pub repository: Option<&'a str>,
pub max_upload_size: Option<i32>,
pub license: Option<&'a str>,
}

impl<'a> NewCrate<'a> {
pub fn create_or_update(
mut self,
conn: &PgConnection,
license_file: Option<&'a str>,
uploader: i32,
rate_limit: Option<&PublishRateLimit>,
) -> CargoResult<Crate> {
use diesel::update;

self.validate(license_file)?;
self.validate()?;
self.ensure_name_not_reserved(conn)?;

conn.transaction(|| {
Expand All @@ -132,7 +127,7 @@ impl<'a> NewCrate<'a> {
})
}

fn validate(&mut self, license_file: Option<&'a str>) -> CargoResult<()> {
fn validate(&mut self) -> CargoResult<()> {
fn validate_url(url: Option<&str>, field: &str) -> CargoResult<()> {
let url = match url {
Some(s) => s,
Expand Down Expand Up @@ -163,28 +158,6 @@ impl<'a> NewCrate<'a> {
validate_url(self.homepage, "homepage")?;
validate_url(self.documentation, "documentation")?;
validate_url(self.repository, "repository")?;
self.validate_license(license_file)?;
Ok(())
}

fn validate_license(&mut self, license_file: Option<&str>) -> CargoResult<()> {
if let Some(license) = self.license {
for part in license.split('/') {
license_exprs::validate_license_expr(part).map_err(|e| {
human(&format_args!(
"{}; see http://opensource.org/licenses \
for options, and http://spdx.org/licenses/ \
for their identifiers",
e
))
})?;
}
} else if license_file.is_some() {
// If no license is given, but a license file is given, flag this
// crate as having a nonstandard license. Note that we don't
// actually do anything else with license_file currently.
self.license = Some("non-standard");
}
Ok(())
}

Expand Down
1 change: 0 additions & 1 deletion src/models/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,4 @@ mod tests {
.find(r#""last_used_at":"2017-01-06T14:23:12+00:00""#)
.is_some());
}

}
6 changes: 0 additions & 6 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,6 @@ table! {
///
/// (Automatically generated by Diesel.)
textsearchable_index_col -> Tsvector,
/// The `license` column of the `crates` table.
///
/// Its SQL type is `Nullable<Varchar>`.
///
/// (Automatically generated by Diesel.)
license -> Nullable<Varchar>,
/// The `repository` column of the `crates` table.
///
/// Its SQL type is `Nullable<Varchar>`.
Expand Down
2 changes: 1 addition & 1 deletion src/tasks/update_downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ mod test {
name: "foo",
..Default::default()
}
.create_or_update(conn, None, user_id, None)
.create_or_update(conn, user_id, None)
.unwrap();
let version = NewVersion::new(
krate.id,
Expand Down
2 changes: 1 addition & 1 deletion src/tests/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl<'a> CrateBuilder<'a> {

let mut krate = self
.krate
.create_or_update(connection, None, self.owner_id, None)?;
.create_or_update(connection, self.owner_id, None)?;

// Since we are using `NewCrate`, we can't set all the
// crate properties in a single DB call.
Expand Down