Skip to content

Commit

Permalink
Lint: fmt, clippy and changed --app-id error text.
Browse files Browse the repository at this point in the history
  • Loading branch information
korvyashka committed Aug 6, 2024
1 parent b7c7014 commit 005a916
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 57 deletions.
44 changes: 9 additions & 35 deletions src/commands/edge_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ impl EdgeAppCommand {
let manifest_path = transform_edge_app_path_to_manifest(&path);

EdgeAppManifest::ensure_manifest_is_valid(&manifest_path)?;
let mut manifest = EdgeAppManifest::new(&manifest_path)?;
let manifest = EdgeAppManifest::new(&manifest_path)?;

let actual_app_id = match self.get_app_id(path.clone()) {
Ok(id) => id,
Expand Down Expand Up @@ -725,13 +725,10 @@ impl EdgeAppCommand {
name: &str,
) -> Result<String, CommandError> {
// Though we could either allow --force to re-create it or --new to create a new instance w/o writing to instance.yml
match InstanceManifest::new(path) {
Ok(manifest) => {
if manifest.id.is_some() {
return Err(CommandError::InstanceAlreadyExists);
}
if let Ok(manifest) = InstanceManifest::new(path) {
if manifest.id.is_some() {
return Err(CommandError::InstanceAlreadyExists);
}
Err(_) => {}
}

let installation_id = self.install_edge_app(app_id, name, None)?;
Expand Down Expand Up @@ -1309,27 +1306,6 @@ impl EdgeAppCommand {
}
}

pub fn get_actual_app_id(
&self,
app_id: &Option<String>,
path: &Option<String>,
) -> Result<String, CommandError> {
match app_id {
Some(id) if id.is_empty() => Err(CommandError::EmptyAppId),
Some(id) => Ok(id.clone()),
None => {
let manifest_path = transform_edge_app_path_to_manifest(path);
EdgeAppManifest::ensure_manifest_is_valid(manifest_path.as_path())?;

let manifest = EdgeAppManifest::new(manifest_path.as_path())?;
match manifest.id {
Some(id) if !id.is_empty() => Ok(id),
_ => Err(CommandError::MissingAppId),
}
}
}
}

pub fn get_installation_id(&self, path: Option<String>) -> Result<String, CommandError> {
let instance_manifest =
InstanceManifest::new(&transform_instance_path_to_instance_manifest(&path)?)?;
Expand All @@ -1356,10 +1332,8 @@ mod tests {
use tempfile::TempDir;

use commands::edge_app_manifest::MANIFEST_VERSION;
use commands::instance_manifest;
use httpmock::Method::{DELETE, GET, PATCH, POST};
use httpmock::MockServer;
use reqwest::header::Entry;

use crate::commands::edge_app_server::MOCK_DATA_FILENAME;
use crate::commands::edge_app_utils::EdgeAppFile;
Expand Down Expand Up @@ -1511,7 +1485,7 @@ mod tests {

#[test]
fn test_edge_app_create_when_manifest_or_index_html_exist_should_return_error() {
let (tmp_dir, command, mock_server, _manifest, _instance_manifest) =
let (tmp_dir, command, _mock_server, _manifest, _instance_manifest) =
prepare_edge_apps_test(true, false);

let result = command.create(
Expand Down Expand Up @@ -2915,7 +2889,7 @@ mod tests {

#[test]
fn test_generate_mock_data_creates_file_with_expected_content() {
let (dir, command, mock_server, _manifest, _instance_manifest) =
let (_dir, command, _mock_server, _manifest, _instance_manifest) =
prepare_edge_apps_test(false, false);
let dir = tempdir().unwrap();
let file_path = dir.path().join("test_manifest.yml");
Expand Down Expand Up @@ -3188,7 +3162,7 @@ mod tests {
assert!(result.is_err());
assert_eq!(
result.unwrap_err().to_string(),
"App id is required. Either in manifest or with --app-id."
"App id is required in manifest."
);
}

Expand Down Expand Up @@ -3608,7 +3582,7 @@ mod tests {

#[test]
fn test_update_instance_when_name_changed_should_update_instance() {
let (temp_dir, command, mock_server, manifest, _instance_manifest) =
let (temp_dir, command, mock_server, _manifest, _instance_manifest) =
prepare_edge_apps_test(true, true);

let get_instance_mock = mock_server.mock(|when, then| {
Expand Down Expand Up @@ -3883,7 +3857,7 @@ mod tests {

#[test]
fn test_delete_instance_should_delete_instance() {
let (temp_dir, command, mock_server, manifest, _instance_manifest) =
let (temp_dir, command, mock_server, _manifest, _instance_manifest) =
prepare_edge_apps_test(true, true);

let instance_manifest_path = temp_dir.path().join("instance.yml");
Expand Down
4 changes: 0 additions & 4 deletions src/commands/edge_app_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ pub struct EdgeAppManifest {
pub settings: Vec<Setting>,
}

fn default_syntax() -> String {
MANIFEST_VERSION.to_owned()
}

fn deserialize_auth<'de, D>(deserializer: D) -> Result<Option<Auth>, D::Error>
where
D: serde::de::Deserializer<'de>,
Expand Down
8 changes: 0 additions & 8 deletions src/commands/edge_app_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ use strum_macros::{Display, EnumIter, EnumString};

use crate::commands::serde_utils::{deserialize_string_field, serialize_non_empty_string_field};

const RESERVED_SETTING_NAMES: &[&str] = &[
"basic_auth_username",
"basic_auth_password",
"bearer_token",
"azure_ad_client_id",
"azure_ad_client_secret",
];

#[derive(Clone, Debug, Eq, PartialEq, Serialize, Default, EnumString, Display, EnumIter)]
pub enum SettingType {
#[default]
Expand Down
13 changes: 6 additions & 7 deletions src/commands/edge_app_utils.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::commands::edge_app::AssetSignature;
use crate::commands::edge_app_manifest::EdgeAppManifest;
use crate::commands::edge_app_settings::{Setting, SettingType};
use crate::commands::instance_manifest::{InstanceManifest, INSTANCE_MANIFEST_VERSION};
use crate::commands::instance_manifest::InstanceManifest;
use crate::commands::CommandError;
use crate::signature::{generate_signature, sig_to_hex};
use log::debug;
Expand Down Expand Up @@ -281,12 +281,10 @@ pub fn validate_manifests_dependacies(
}
}
}
} else {
if instance_manifest.entrypoint_uri.is_some() {
return Err(CommandError::InvalidManifest(
"entrypoint_uri must not be set when entrypoint is not set".to_owned(),
));
}
} else if instance_manifest.entrypoint_uri.is_some() {
return Err(CommandError::InvalidManifest(
"entrypoint_uri must not be set when entrypoint is not set".to_owned(),
));
}
Ok(())
}
Expand All @@ -295,6 +293,7 @@ pub fn validate_manifests_dependacies(
mod tests {
use super::*;
use crate::commands::edge_app_manifest::{Auth, Entrypoint, EntrypointType, MANIFEST_VERSION};
use crate::commands::instance_manifest::INSTANCE_MANIFEST_VERSION;
use crate::commands::manifest_auth::AuthType;
use crate::commands::SettingType;
use std::fs::File;
Expand Down
4 changes: 1 addition & 3 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,8 @@ pub enum CommandError {
InitializationError(String),
#[error("Asset processing error: {0}")]
AssetProcessingError(String),
#[error("App id is required. Either in manifest or with --app-id.")]
#[error("App id is required in manifest.")]
MissingAppId,
#[error("App id cannot be empty. Provide it either in manifest.")]
EmptyAppId,
#[error("Edge App Revision {0} not found")]
RevisionNotFound(String),
#[error("Manifest file validation failed with error: {0}")]
Expand Down

0 comments on commit 005a916

Please sign in to comment.