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

Implement import command #34

Merged
merged 2 commits into from Aug 22, 2017
Jump to file or symbol
Failed to load files and symbols.
+85 −0
Diff settings

Always

Just for now

Copy path View file
@@ -0,0 +1,77 @@
// Copyright 2014-2017 The Rooster Developers
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
use getopts;
use serde_json;
use password::v2::{Password, PasswordStore};
use std::fs::File;
use std::io::Write;
pub fn callback_help() {
println!("Usage:");
println!(" rooster import -h");
println!(" rooster import <file_path>");
println!();
println!("Example:");
println!(" rooster import dump.json");
}
pub fn callback_exec(_matches: &getopts::Matches, store: &mut PasswordStore) -> Result<(), i32> {

This comment has been minimized.

@conradkdotcom

conradkdotcom Aug 21, 2017

Owner

Replace _matches with matches. I used _matches in some places to suppress warning when this variable was unused. Here, the variable is used so the _ is not needed.

let imported_pwds: Vec<Password> = {
let path_str = &_matches.free[1];

This comment has been minimized.

@conradkdotcom

conradkdotcom Aug 21, 2017

Owner

When you forget to pass the file path, for instance if you just call rooster import, instead of rooster import blabla.json, then the process has panic!(). This is why most command have check_args function.

let dump_file = File::open(path_str).map_err(|err|{
println_stderr!("Uh oh, could not open the file (reason: {:?})", err);

This comment has been minimized.

@conradkdotcom

conradkdotcom Aug 21, 2017

Owner

When the file does not exist, the error message is too verbose. For instance, I get Uh oh, could not open the file (reason: Error { repr: Os { code: 2, message: "No such file or directory" } }). I think that Uh oh, could not open the file (reason: No such file or directory) is enough.

1
})?;
serde_json::from_reader(&dump_file).map_err(|json_err| {
println_stderr!(
"Woops, I could not decode the passwords from JSON (reason: {:?}).",

This comment has been minimized.

@conradkdotcom

conradkdotcom Aug 21, 2017

Owner

Let's use import instead of decode here so we don't introduce new words that might confuse the user.

This comment has been minimized.

@conradkdotcom

conradkdotcom Aug 21, 2017

Owner

When you import invalid JSON, the error message is a bit difficult to understand. For instance: ErrorImpl { code: EofWhileParsingValue, line: 1, column: 0 }. Maybe there is a way to make this a bit more readable?

json_err,
);
1
})?
};
let mut added = 0;
for password in imported_pwds {
if let Some(_) = store.get_password(&password.name) {
println_stderr!(
"Oh, password for {} is already present! Skipping it.",
password.name,
);
continue;
}
if let Err(err) = store.add_password(password.clone()) {
println_stderr!(
"Woops, couldn't add password for {} (reason: {:?})",
password.name,
err,
);
continue;
}
added += 1;
}
if added == 0 {
println_stderr!("Apparently, I could not find any new password :(");
} else if added == 1 {
println_ok!("Imported {} brand new password into the store!", added);

This comment has been minimized.

@conradkdotcom

conradkdotcom Aug 21, 2017

Owner

Remove into the store or replace with Rooster file because store is not used anywhere in Rooster and this word might confuse users.

} else {
println_ok!("Imported {} brand new passwords into the store!", added);

This comment has been minimized.

@conradkdotcom

conradkdotcom Aug 21, 2017

Owner

Remove into the store or replace with Rooster file because store is not used anywhere in Rooster and this word might confuse users.

}
Ok(())
}
Copy path View file
@@ -18,6 +18,7 @@ pub mod delete;
pub mod generate;
pub mod list;
pub mod regenerate;
pub mod import;
pub mod export;
pub mod set_master_password;
pub mod rename;
Copy path View file
@@ -107,6 +107,12 @@ static COMMANDS: &'static [Command] = &[
callback_help: commands::list::callback_help,
callback_without_store: None,
},
Command {
name: "import",
callback_exec: Some(commands::import::callback_exec),
callback_help: commands::import::callback_help,
callback_without_store: None,
},
Command {
name: "export",
callback_exec: Some(commands::export::callback_exec),
@@ -568,6 +574,7 @@ fn usage(password_file: &str) {
println!(" rename Rename the app for a password");
println!(" transfer Change the username for a password");
println!(" list List all apps and usernames");
println!(" import Load all your raw password data from JSON file");
println!(" export Dump all your raw password data in JSON");
println!(" set-master-password Set your master password");
println!(" uninstall Show instructions to uninstall Rooster");
ProTip! Use n and p to navigate between commits in a pull request.