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

remove-sequence command #28

Merged
merged 7 commits into from
Jul 23, 2016
Merged
Show file tree
Hide file tree
Changes from 4 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
5 changes: 5 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub enum Error {
MusicFileNotFound(String),
UnsupportedFileType(String),
UserNotFound,
SequenceNotFound(String),
UnauthorizedAction,
TodoErr,
}
Expand All @@ -45,6 +46,7 @@ impl error::Error for Error {
Error::MusicFileNotFound(_) => "Music file not found",
Error::UnsupportedFileType(_) => "Unsupported file type",
Error::UserNotFound => "User not found",
Error::SequenceNotFound(_) => "Sequence not found",
Error::UnauthorizedAction => "Unauthorized action",
Error::TodoErr => "Todo",
}
Expand All @@ -68,6 +70,7 @@ impl error::Error for Error {
Error::MusicFileNotFound(_) => None,
Error::UnsupportedFileType(_) => None,
Error::UserNotFound => None,
Error::SequenceNotFound(_) => None,
Error::UnauthorizedAction => None,
Error::TodoErr => None,
}
Expand Down Expand Up @@ -107,6 +110,8 @@ impl fmt::Display for Error {
Error::UnsupportedFileType(ref file_type) => write!(f,
"Unsupported file type: {}", file_type),
Error::UserNotFound => write!(f, "User not found"),
Error::SequenceNotFound(ref name) => write!(f,
"Sequence not found: '{}'", name),
Error::UnauthorizedAction => write!(f, "Unauthorized action"),
Error::TodoErr => write!(f, "TodoErr"),
}
Expand Down
12 changes: 9 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Usage:
./proton init <folder> <root-public-key>
./proton new-user <admin-key> <name> <public-key>
./proton new-sequence <admin-key> <name> <music-file>
./proton remove-sequence <admin-key> <name>
./proton id-user <private-key>
./proton list-permissions
./proton set-permission <admin-key> (add | remove) <name> <permission> [<target>]
Expand Down Expand Up @@ -53,6 +54,7 @@ fn main() {
"new-user" => run_new_user,
"id-user" => run_id_user,
"new-sequence" => run_new_sequence,
"remove-sequence" => run_remove_sequence,
"list-permissions" => run_list_permissions,
"set-permission" => run_set_permission,
_ => panic!("Invalid first argument"),
Expand Down Expand Up @@ -100,6 +102,13 @@ fn run_new_sequence(args: Args) -> Result<(), Error> {
proton_cli::new_sequence(&admin_key_path, &name, &music_file_path)
}

fn run_remove_sequence(args: Args) -> Result<(), Error> {
let admin_key = args.arg_admin_key.unwrap();
let admin_key_path = Path::new(&admin_key);
let name = args.arg_name.unwrap();
proton_cli::remove_sequence(&admin_key_path, &name)
}

#[allow(unused_variables)]
fn run_list_permissions(args: Args) -> Result<(), Error> {
let permissions = proton_cli::get_permissions();
Expand All @@ -117,6 +126,3 @@ fn run_set_permission(args: Args) -> Result<(), Error> {

proton_cli::set_permission(&auth_user, added, &username, permission, target)
}



11 changes: 11 additions & 0 deletions src/project_types/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ impl Project {
Ok(new_project)
}

pub fn remove_sequence(&self, name: &str) -> Result<Project, Error> {
let mut new_project = self.clone();
for i in 0..new_project.sequences.len() {
if new_project.sequences[i].name == name {
new_project.sequences.remove(i);
return Ok(new_project);
}
}
Err(Error::SequenceNotFound(name.to_string()))
}

/// Changes a user's permissions
pub fn set_user_permission(
&mut self,
Expand Down
36 changes: 36 additions & 0 deletions src/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,42 @@ pub fn new_sequence<P: AsRef<Path>>(
.map(|_| ())
}

pub fn remove_sequence<P: AsRef<Path>>(admin_key_path: P, name: &str) -> Result<(), Error> {

// Check that the admin has sufficient privileges
try!(utils::validate_admin(&admin_key_path));

// Make sure the name is valid (needed since it will be used in a file path)
try!(validate_seq_name(name));

// Make the name of the sequence's directory
let mut sequence_dir = String::from("seq_");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think "seq_".to_owned() is better since its just more commonly used.

sequence_dir.push_str(&name);
let sequence_dir = sequence_dir;

// Remove sequence's directory
let sequence_dir_path = Path::new(&sequence_dir);
if sequence_dir_path.exists() && sequence_dir_path.is_dir() {
let _ = fs::remove_dir_all(&sequence_dir_path)
.expect("Error removing sequence directory");
}

// Remove sequence from project
let project = try!(utils::read_protonfile(None::<P>));
Copy link
Contributor

Choose a reason for hiding this comment

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

You should move this section before remove sequence's directory, because if something goes wrong in the protonfile, its less damaging than something going wrong in an IO op. Rule of thumb: less risky things first.

let new_project = try!(project.remove_sequence(name));

// Save project
try!(utils::write_protonfile(&new_project, None::<P>));

// Commit changes
let signature = Signature::now("Proton Lights", "[email protected]").unwrap();
let msg = format!("Removing sequence '{}'", name);
let repo_path: Option<P> = None;

utils::commit_all(repo_path, &signature, &msg)
.map(|_| ())
}

/// Check that the music file is a valid format
/// Full list of supported formats can be found at
/// http://www.rust-sfml.org/doc/rsfml/audio/struct.Music.html
Expand Down
99 changes: 99 additions & 0 deletions tests/remove_sequence.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
extern crate proton_cli;
extern crate tempdir;
extern crate git2;

mod common;

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

use common::setup;
use common::rsa_keys::TestKey;


#[test]
fn works_with_valid_key_and_name() {
let root = setup::setup_init_cd();
let root_key_path = common::make_key_file(&root.path(), "root.pem", TestKey::RootKeyPem);

setup::try_make_sequence(&root_key_path.as_path(), "asdf", "Dissonance.ogg");

Copy link
Contributor

Choose a reason for hiding this comment

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

Add a check here that asdf really was added.

Copy link
Member Author

Choose a reason for hiding this comment

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

The asserts are part of try_make_sequence. Will look into them though

proton_cli::remove_sequence(&root_key_path.as_path(), "asdf")
.expect("Error removing sequence");

let project = proton_cli::utils::read_protonfile(None::<&Path>)
.expect("Error reading project from file");

let found_sequence = project.find_sequence_by_name("asdf");

assert!(found_sequence.is_none());

let mut sequence_dir_path = PathBuf::from(root.path());
sequence_dir_path.push("seq_asdf");
assert!(!sequence_dir_path.exists());

common::assert_repo_no_modified_files(&root.path());
}

#[test]
#[should_panic(expected = "Error removing sequence: Ssl")]
fn fails_with_invalid_admin_key() {
let root = setup::setup_init_cd();
let root_key_path = common::make_key_file(&root.path(), "root.pem", TestKey::RootKeyPem);
let root_key_bad_path = common::make_key_file(&root.path(), "root_bad.pub", TestKey::RootKeyPub);

setup::try_make_sequence(&root_key_path.as_path(), "asdf", "Dissonance.ogg");
proton_cli::remove_sequence(&root_key_bad_path.as_path(), "asdf")
.expect("Error removing sequence");
}

#[test]
#[should_panic(expected = "Error removing sequence: Io")]
fn fails_with_nonexistent_admin_key() {
let root = setup::setup_init_cd();
let root_key_path = common::make_key_file(&root.path(), "root.pem", TestKey::RootKeyPem);
let root_key_bad_path = PathBuf::from("nonexistent");

setup::try_make_sequence(&root_key_path.as_path(), "asdf", "Dissonance.ogg");
proton_cli::remove_sequence(&root_key_bad_path.as_path(), "asdf")
.expect("Error removing sequence");
}

#[test]
#[should_panic(expected = "Error removing sequence: UnauthorizedAction")]
fn fails_with_unauthorized_admin_key() {
let root = setup::setup_init_cd();
let root_key_path = common::make_key_file(&root.path(), "root.pem", TestKey::RootKeyPem);
let normal_key_pub_path = common::make_key_file(&root.path(), "normal.pub", TestKey::GoodKeyPub);
let normal_key_priv_path = common::make_key_file(&root.path(), "normal.pem", TestKey::GoodKeyPem);
let _ = proton_cli::new_user(
&root_key_path.as_path(),
&normal_key_pub_path.as_path(),
"normal_user"
).expect("Error creating user");

setup::try_make_sequence(&root_key_path.as_path(), "asdf", "Dissonance.ogg");
proton_cli::remove_sequence(&normal_key_priv_path.as_path(), "asdf")
.expect("Error removing sequence");
}

#[test]
#[should_panic(expected = "Error removing sequence: SequenceNotFound")]
fn fails_with_nonexistent_sequence_name() {
let root = setup::setup_init_cd();
let root_key_path = common::make_key_file(&root.path(), "root.pem", TestKey::RootKeyPem);

setup::try_make_sequence(&root_key_path.as_path(), "asdf", "Dissonance.ogg");
proton_cli::remove_sequence(&root_key_path.as_path(), "a")
.expect("Error removing sequence");
}

#[test]
#[should_panic(expected = "Error removing sequence: InvalidSequenceName")]
fn fails_with_bad_sequence_name() {
let root = setup::setup_init_cd();
let root_key_path = common::make_key_file(&root.path(), "root.pem", TestKey::RootKeyPem);

setup::try_make_sequence(&root_key_path.as_path(), "asdf", "Dissonance.ogg");
proton_cli::remove_sequence(&root_key_path.as_path(), "as df")
.expect("Error removing sequence");
}