Skip to content

Commit

Permalink
Remove urivalidator changes from the set, address other feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
AnotherDaniel committed Jan 31, 2024
1 parent c3cd869 commit 8b65d2c
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 78 deletions.
16 changes: 14 additions & 2 deletions src/rpc/rpcmapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,14 @@ impl RpcMapper {
T: MessageFull + Default,
{
let message = response?; // Directly returns in case of error
Any::try_from(message.payload.unwrap())

let Some(payload) = message.payload.into_option() else {
return Err(RpcMapperError::InvalidPayload(
"Payload is empty".to_string(),
));
};

Any::try_from(payload)
.map_err(|_e| {
RpcMapperError::UnknownType("Couldn't decode payload into Any".to_string())
})
Expand Down Expand Up @@ -123,7 +130,12 @@ impl RpcMapper {
// TODO This entire thing feels klunky and kludgy; this needs to be revisited...
pub fn map_response_to_result(response: RpcClientResult) -> RpcPayloadResult {
let message = response?; // Directly returns in case of error
Any::try_from(message.payload.unwrap())
let Some(payload) = message.payload.into_option() else {
return Err(RpcMapperError::InvalidPayload(
"Payload is empty".to_string(),
));
};
Any::try_from(payload)
.map_err(|_e| {
RpcMapperError::UnknownType("Couldn't decode payload into Any".to_string())
})
Expand Down
2 changes: 1 addition & 1 deletion src/uri/serializer/longuriserializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use regex::Regex;

use crate::uprotocol::{UAuthority, UEntity, UResource, UUri};
use crate::uri::serializer::{SerializationError, UriSerializer};
pub use crate::uri::serializer::{SerializationError, UriSerializer};
use crate::uri::validator::UriValidator;

/// `UriSerializer` that serializes a `UUri` to a string (long format) per
Expand Down
2 changes: 1 addition & 1 deletion src/uri/serializer/microuriserializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::io::Write;

use crate::uprotocol::{UAuthority, UEntity, UUri};
use crate::uri::builder::resourcebuilder::UResourceBuilder;
use crate::uri::serializer::{SerializationError, UriSerializer};
pub use crate::uri::serializer::{SerializationError, UriSerializer};
use crate::uri::validator::UriValidator;

const LOCAL_MICRO_URI_LENGTH: usize = 8; // local micro URI length
Expand Down
100 changes: 26 additions & 74 deletions src/uri/validator/urivalidator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
* SPDX-License-Identifier: Apache-2.0
********************************************************************************/

use protobuf::Message;

use crate::uprotocol::{UAuthority, UUri};
use crate::uri::validator::ValidationError;

Expand Down Expand Up @@ -122,7 +120,8 @@ impl UriValidator {
/// Returns `true` if the URI contains both names and numeric representations of the names,
/// meaning that this `UUri` can be serialized to long or micro formats.
pub fn is_resolved(uri: &UUri) -> bool {
!Self::is_empty(uri) && Self::is_long_form(uri) && Self::is_micro_form(uri)
!Self::is_empty(uri)
// TODO finish this
}

/// Checks if the URI is of type RPC.
Expand Down Expand Up @@ -165,31 +164,14 @@ impl UriValidator {
.as_ref()
.map_or(false, |instance| instance.contains("response"));

let has_zero_id = resource.id.map_or(false, |id| id == 0);
let has_non_zero_id = resource.id.map_or(false, |id| id != 0);

return has_valid_instance && has_zero_id;
return has_valid_instance || has_non_zero_id;
}
}
false
}

/// Determines if a `UAuthority` is local, which means it is not set with a name, IP, or ID.
///
/// This function checks whether the provided `UAuthority` lacks a name, IP address, or ID,
/// which would classify it as a local authority. The absence of these attributes implies a local scope.
///
/// # Arguments
///
/// * `authority` - A reference to the `UAuthority` to be checked.
///
/// # Returns
///
/// Returns `true` if the `UAuthority` is local, meaning it is not populated with a name, IP, or ID.
/// Returns `false` otherwise.
pub fn is_local(authority: &UAuthority) -> bool {
authority == UAuthority::default_instance()
}

/// Checks if a `UAuthority` is of type remote
///
/// # Arguments
Expand All @@ -216,23 +198,7 @@ impl UriValidator {
!Self::is_empty(uri)
&& uri.entity.has_id()
&& uri.resource.has_id()
&& Self::is_micro_form_authority(&uri.authority)
}

/// Checks if a `UAuthority` can be represented in micro format.
///
/// Micro UAuthorities are either local or ones that contain an IP address or IDs. This function
/// evaluates whether a given `UAuthority` meets the criteria to be considered as a micro format authority.
///
/// # Arguments
///
/// * `authority` - A reference to the `UAuthority` to check.
///
/// # Returns
///
/// Returns `true` if the `UAuthority` can be represented in micro format, `false` otherwise.
pub fn is_micro_form_authority(authority: &UAuthority) -> bool {
Self::is_local(authority) || (authority.has_id() || authority.has_ip())
&& (uri.authority.is_none() || uri.authority.has_id() || uri.authority.has_ip())
}

/// Checks if the URI contains names so that it can be serialized into long format.
Expand All @@ -243,41 +209,28 @@ impl UriValidator {
/// # Returns
/// Returns `true` if the URI contains names, allowing it to be serialized into long format.
pub fn is_long_form(uri: &UUri) -> bool {
if !Self::is_empty(uri) && Self::is_long_form_authority(&uri.authority) {
let has_entity_name = !uri
.entity
.as_ref()
.is_some_and(|e| e.name.trim().is_empty());

let has_resource_name = !uri
.resource
.as_ref()
.is_some_and(|e| e.name.trim().is_empty());

return has_entity_name && has_resource_name;
if Self::is_empty(uri) {
return false;
}
false
}

/// Determines if a `UAuthority` contains names suitable for serialization into long format.
///
/// This function checks whether the given `UAuthority` includes names, which are necessary
/// for it to be serialized into a long format. The presence of these names indicates that
/// the `UAuthority` can be represented in a more detailed, extended format.
///
/// # Arguments
///
/// * `authority` - A reference to the `UAuthority` to be checked.
///
/// # Returns
///
/// Returns `true` if the `UAuthority` contains names that allow it to be serialized into
/// long format. Returns `false` otherwise.
pub fn is_long_form_authority(authority: &UAuthority) -> bool {
authority
.name
.as_ref()
.is_some_and(|n| !n.trim().is_empty())
let mut auth_name = String::new();
if let Some(authority) = uri.authority.as_ref() {
if let Some(name) = UAuthority::get_name(authority) {
auth_name = name.to_string();
}
}

let mut ent_name = String::new();
if let Some(entity) = uri.entity.as_ref() {
ent_name = entity.name.to_string();
}

let mut res_name = String::new();
if let Some(resource) = uri.resource.as_ref() {
res_name = resource.name.to_string();
}

!auth_name.is_empty() && !ent_name.trim().is_empty() && !res_name.trim().is_empty()
}
}

Expand Down Expand Up @@ -1111,8 +1064,7 @@ mod tests {
};
let resource = UResource {
name: "rpc".into(),
id: Some(0),
instance: Some("response".into()),
id: Some(19999),
..Default::default()
};
let uuri = UUri {
Expand Down

0 comments on commit 8b65d2c

Please sign in to comment.