Skip to content

Commit

Permalink
improve the usability of Element::set_character_data()
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielT committed Jun 1, 2024
1 parent 189c9d5 commit 2817fd4
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 62 deletions.
17 changes: 11 additions & 6 deletions autosar-data/src/autosarmodel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1339,9 +1339,7 @@ mod test {
.create_sub_element(ElementName::FibexElementRefConditional)
.and_then(|ferc| ferc.create_sub_element(ElementName::FibexElementRef))
.unwrap();
el_fibex_element_ref
.set_character_data(CharacterData::String("/Pkg/System".to_string()))
.unwrap();
el_fibex_element_ref.set_character_data("/Pkg/System").unwrap();
let invalid_refs = model.check_references();
assert_eq!(invalid_refs.len(), 3);
let ref0 = invalid_refs[0].upgrade().unwrap();
Expand Down Expand Up @@ -1380,9 +1378,16 @@ mod test {
let model = AutosarModel::new();
let file1 = model.create_file("filename1", AutosarVersion::Autosar_00042).unwrap();
let file2 = model.create_file("filename2", AutosarVersion::Autosar_00042).unwrap();
let el_ar_packages = model.root_element().create_sub_element(ElementName::ArPackages).unwrap();
let el_pkg1 = el_ar_packages.create_named_sub_element(ElementName::ArPackage, "pkg1").unwrap();
let el_pkg2 = el_ar_packages.create_named_sub_element(ElementName::ArPackage, "pkg2").unwrap();
let el_ar_packages = model
.root_element()
.create_sub_element(ElementName::ArPackages)
.unwrap();
let el_pkg1 = el_ar_packages
.create_named_sub_element(ElementName::ArPackage, "pkg1")
.unwrap();
let el_pkg2 = el_ar_packages
.create_named_sub_element(ElementName::ArPackage, "pkg2")
.unwrap();

assert_eq!(el_ar_packages.file_membership().unwrap().1.len(), 2);
el_pkg1.remove_from_file(&file2).unwrap();
Expand Down
48 changes: 48 additions & 0 deletions autosar-data/src/chardata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,36 @@ impl CharacterData {
}
}

impl From<String> for CharacterData {
fn from(value: String) -> Self {
Self::String(value)
}
}

impl From<&str> for CharacterData {
fn from(value: &str) -> Self {
Self::String(value.to_string())
}
}

impl From<EnumItem> for CharacterData {
fn from(value: EnumItem) -> Self {
Self::Enum(value)
}
}

impl From<u64> for CharacterData {
fn from(value: u64) -> Self {
Self::UnsignedInteger(value)
}
}

impl From<f64> for CharacterData {
fn from(value: f64) -> Self {
Self::Double(value)
}
}

impl Display for CharacterData {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Expand Down Expand Up @@ -494,4 +524,22 @@ mod test {
let result = data.decode_integer::<i32>().unwrap();
assert_eq!(result, -55);
}

#[test]
fn cdata_conversion() {
let cdata: CharacterData = 0.into();
assert_eq!(cdata, CharacterData::UnsignedInteger(0));

let cdata: CharacterData = (1.0).into();
assert_eq!(cdata, CharacterData::Double(1.0));

let cdata: CharacterData = "text".into();
assert_eq!(cdata, CharacterData::String("text".to_string()));

let cdata: CharacterData = String::from("text").into();
assert_eq!(cdata, CharacterData::String("text".to_string()));

let cdata: CharacterData = EnumItem::Abstract.into();
assert_eq!(cdata, CharacterData::Enum(EnumItem::Abstract));
}
}
80 changes: 41 additions & 39 deletions autosar-data/src/element.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl Element {
/// # let file = model.create_file("test", AutosarVersion::Autosar_00050).unwrap();
/// # let element = model.root_element().create_sub_element(ElementName::ArPackages).and_then(|pkgs| pkgs.create_named_sub_element(ElementName::ArPackage, "name")).unwrap();
/// if let Some(short_name) = element.get_sub_element(ElementName::ShortName) {
/// short_name.set_character_data(CharacterData::String("the_new_name".to_string()));
/// short_name.set_character_data("the_new_name");
/// }
/// ```
///
Expand Down Expand Up @@ -874,7 +874,7 @@ impl Element {
/// # let element = model.root_element().create_sub_element(ElementName::ArPackages)
/// # .and_then(|e| e.create_named_sub_element(ElementName::ArPackage, "Pkg"))?
/// # .get_sub_element(ElementName::ShortName).unwrap();
/// element.set_character_data(CharacterData::String("value".to_string()))?;
/// element.set_character_data("value")?;
/// # Ok(())
/// # }
/// ```
Expand All @@ -885,13 +885,27 @@ impl Element {
/// - [`AutosarDataError::ParentElementLocked`]: a parent element was locked and did not become available after waiting briefly.
/// The operation was aborted to avoid a deadlock, but can be retried.
/// - [`AutosarDataError::IncorrectContentType`]: Cannot set character data on an element which does not contain character data
pub fn set_character_data(&self, chardata: CharacterData) -> Result<(), AutosarDataError> {
pub fn set_character_data<T: Into<CharacterData>>(
&self,
value: T,
) -> Result<(), AutosarDataError> {
let mut chardata: CharacterData = value.into();
let elemtype = self.elemtype();
if elemtype.content_mode() == ContentMode::Characters || elemtype.content_mode() == ContentMode::Mixed {
if let Some(cdata_spec) = elemtype.chardata_spec() {
let model = self.model()?;
let version = self.min_version()?;
if CharacterData::check_value(&chardata, cdata_spec, version) {
let mut compatible_value = CharacterData::check_value(&chardata, cdata_spec, version);
if !compatible_value
&& matches!(
cdata_spec,
CharacterDataSpec::Pattern { .. } | CharacterDataSpec::String { .. }
)
{
chardata = CharacterData::String(chardata.to_string());
compatible_value = CharacterData::check_value(&chardata, cdata_spec, version);
}
if compatible_value {
// if this is a SHORT-NAME element a whole lot of handling is needed in order to unbreak all the cross references
let mut prev_path = None;
if self.element_name() == ElementName::ShortName {
Expand Down Expand Up @@ -2648,7 +2662,7 @@ mod test {
.and_then(|el| el.create_sub_element(ElementName::FibexElements))
.and_then(|el| el.create_sub_element(ElementName::FibexElementRefConditional))
.and_then(|el| el.create_sub_element(ElementName::FibexElementRef))
.and_then(|el| el.set_character_data(CharacterData::String("/invalid".to_string())))
.and_then(|el| el.set_character_data("/invalid"))
.unwrap();

let project2 = AutosarModel::new();
Expand Down Expand Up @@ -2732,7 +2746,7 @@ mod test {
.and_then(|el| el.create_sub_element(ElementName::FibexElements))
.and_then(|el| el.create_sub_element(ElementName::FibexElementRefConditional))
.and_then(|el| el.create_sub_element(ElementName::FibexElementRef))
.and_then(|el| el.set_character_data(CharacterData::String("/invalid".to_string())))
.and_then(|el| el.set_character_data("/invalid"))
.unwrap();

// removing the SHORT-NAME of an identifiable element is forbidden
Expand Down Expand Up @@ -3189,14 +3203,10 @@ mod test {
el_fibex_element_ref
.set_attribute(AttributeName::Dest, CharacterData::Enum(EnumItem::EcuInstance))
.unwrap();
el_fibex_element_ref
.set_character_data(CharacterData::String("/does/not/exist".to_string()))
.unwrap();
el_fibex_element_ref.set_character_data("/does/not/exist").unwrap();
assert!(el_fibex_element_ref.get_reference_target().is_err());
// invalid reference: refers to the wrong type of element
el_fibex_element_ref
.set_character_data(CharacterData::String("/Package".to_string()))
.unwrap();
el_fibex_element_ref.set_character_data("/Package").unwrap();
assert!(el_fibex_element_ref.get_reference_target().is_err());
// invalid reference: no reference string
el_fibex_element_ref.remove_character_data().unwrap();
Expand Down Expand Up @@ -3233,7 +3243,10 @@ mod test {
// set character data on an "ordinary" element that has no special handling
assert!(el_pnc_vector_length
.set_character_data(CharacterData::String("2".to_string()))
.is_ok());
.is_ok()); // "native" type is String, without automatic wrapping
assert!(el_pnc_vector_length.set_character_data("2".to_string()).is_ok()); // "native" type is String
assert!(el_pnc_vector_length.set_character_data("2").is_ok()); // automatic conversion: &str -> String
assert!(el_pnc_vector_length.set_character_data(2).is_ok()); // automatic conversion: u64 -> String

// set a new SHORT-NAME, this also updates path cache
assert!(el_short_name
Expand All @@ -3247,7 +3260,7 @@ mod test {

// set a new reference target, which creates an entry in the reference origin cache
assert!(el_fibex_element_ref
.set_character_data(CharacterData::String("/PackageRenamed/EcuInstance1".to_string()))
.set_character_data("/PackageRenamed/EcuInstance1")
.is_ok());
model
.0
Expand All @@ -3258,7 +3271,7 @@ mod test {

// modify the reference target, which updates the entry in the reference origin cache
assert!(el_fibex_element_ref
.set_character_data(CharacterData::String("/PackageRenamed/EcuInstance2".to_string()))
.set_character_data("/PackageRenamed/EcuInstance2")
.is_ok());
model
.0
Expand All @@ -3274,14 +3287,11 @@ mod test {
.is_none());

// can only set character data that are specified with ContentMode::Characters
assert!(el_autosar
.set_character_data(CharacterData::String("text".to_string()))
.is_err());
assert!(el_autosar.set_character_data("text").is_err());

// can't set a value that doesn't match the target spec
assert!(el_short_name
.set_character_data(CharacterData::UnsignedInteger(0))
.is_err());
assert!(el_short_name.set_character_data(0).is_err());
assert!(el_short_name.set_character_data("").is_err());

// remove character data
assert!(el_pnc_vector_length.remove_character_data().is_ok());
Expand All @@ -3308,21 +3318,21 @@ mod test {
assert!(el_autosar
.0
.write()
.set_character_data(CharacterData::UnsignedInteger(0), AutosarVersion::Autosar_00050)
.set_character_data(0, AutosarVersion::Autosar_00050)
.is_err());
assert!(el_fibex_element_ref
.0
.write()
.set_character_data(CharacterData::UnsignedInteger(0), AutosarVersion::Autosar_00050)
.set_character_data(0, AutosarVersion::Autosar_00050)
.is_err());

// operation fails if the model is needed (e.g. reference or short name update), but the model has been deleted
el_fibex_element_ref
.set_character_data(CharacterData::String("/PackageRenamed/EcuInstance2".to_string()))
.set_character_data("/PackageRenamed/EcuInstance2")
.unwrap();
drop(model);
assert!(el_fibex_element_ref
.set_character_data(CharacterData::String("/PackageRenamed/EcuInstance1".to_string()))
.set_character_data("/PackageRenamed/EcuInstance1")
.is_err());
assert!(el_fibex_element_ref.remove_character_data().is_err());
}
Expand Down Expand Up @@ -3575,9 +3585,7 @@ mod test {
el_defref1
.set_attribute(AttributeName::Dest, CharacterData::Enum(EnumItem::EcucBooleanParamDef))
.unwrap();
el_defref1
.set_character_data(CharacterData::String("/DefRef_999".to_string()))
.unwrap();
el_defref1.set_character_data("/DefRef_999").unwrap();
// second bsw value
let el_value2 = el_paramvalues
.create_sub_element(ElementName::EcucNumericalParamValue)
Expand All @@ -3586,9 +3594,7 @@ mod test {
el_defref2
.set_attribute(AttributeName::Dest, CharacterData::Enum(EnumItem::EcucBooleanParamDef))
.unwrap();
el_defref2
.set_character_data(CharacterData::String("/DefRef_111".to_string()))
.unwrap();
el_defref2.set_character_data("/DefRef_111").unwrap();
// Create some misc value sto sort inside el_ar_package2 "A"
let el_elements2 = el_ar_package2.create_sub_element(ElementName::Elements).unwrap();
let el_system = el_elements2
Expand All @@ -3604,9 +3610,7 @@ mod test {
el_fibex_element_ref1
.set_attribute(AttributeName::Dest, CharacterData::Enum(EnumItem::ISignal))
.unwrap();
el_fibex_element_ref1
.set_character_data(CharacterData::String("/ZZZZZ".to_string()))
.unwrap();
el_fibex_element_ref1.set_character_data("/ZZZZZ").unwrap();
let el_fibex_element2 = el_fibex_elements
.create_sub_element(ElementName::FibexElementRefConditional)
.unwrap();
Expand All @@ -3616,9 +3620,7 @@ mod test {
el_fibex_element_ref2
.set_attribute(AttributeName::Dest, CharacterData::Enum(EnumItem::ISignal))
.unwrap();
el_fibex_element_ref2
.set_character_data(CharacterData::String("/AAAAA".to_string()))
.unwrap();
el_fibex_element_ref2.set_character_data("/AAAAA").unwrap();

model.sort();
// validate that identifiable elements have been sorted
Expand Down Expand Up @@ -3650,7 +3652,7 @@ mod test {
) -> Result<Element, AutosarDataError> {
let e = el_subcontainers.create_named_sub_element(ElementName::EcucContainerValue, short_name)?;
let defrefelem = e.create_sub_element(ElementName::DefinitionRef)?;
defrefelem.set_character_data(CharacterData::String(defref.to_string()))?;
defrefelem.set_character_data(defref)?;
Ok(e)
}

Expand All @@ -3662,7 +3664,7 @@ mod test {
) -> Result<Element, AutosarDataError> {
let e = helper_create_bsw_subelem(el_subcontainers, short_name, defref)?;
let indexelem = e.create_sub_element(ElementName::Index)?;
indexelem.set_character_data(CharacterData::String(indexstr.to_string()))?;
indexelem.set_character_data(indexstr)?;
Ok(e)
}

Expand Down
19 changes: 7 additions & 12 deletions autosar-data/src/elementraw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl ElementRaw {
if let Some(ElementContent::Element(subelem_wrapped)) = self.content.first() {
let mut subelem = subelem_wrapped.0.write();
if subelem.element_name() == ElementName::ShortName {
subelem.set_character_data(CharacterData::String(new_name.to_owned()), version)?;
subelem.set_character_data(new_name.to_owned(), version)?;
model.fix_identifiables(&old_path, &new_path);
let new_prefix = new_path;
let mut model_locked = model.0.write();
Expand Down Expand Up @@ -831,10 +831,7 @@ impl ElementRaw {
let refstr = format!("{dest_path}{suffix}");
for ref_element_weak in &ref_elements {
if let Some(ref_element) = ref_element_weak.upgrade() {
ref_element
.0
.write()
.set_character_data(CharacterData::String(refstr.clone()), version)?;
ref_element.0.write().set_character_data(refstr.clone(), version)?;
}
}
model_locked.reference_origins.insert(refstr, ref_elements);
Expand Down Expand Up @@ -940,10 +937,7 @@ impl ElementRaw {
let mut refstr = old_ref.clone();
if let Some(suffix) = old_ref.strip_prefix(&src_path_prefix) {
refstr = format!("{dest_path}{suffix}");
ref_element
.0
.write()
.set_character_data(CharacterData::String(refstr.clone()), version)?;
ref_element.0.write().set_character_data(refstr.clone(), version)?;
}
model.add_reference_origin(&refstr, ref_element.downgrade());
}
Expand Down Expand Up @@ -1060,7 +1054,7 @@ impl ElementRaw {
) -> Result<(), AutosarDataError> {
let path = Cow::from(self.path_unchecked()?);
let mut sub_element_locked = sub_element.0.write();
// find the position of sub_element in the parent element first to verify that sub_element actuall *is* a sub element
// find the position of sub_element in the parent element first to verify that sub_element actually *is* a sub element
let pos = self
.content
.iter()
Expand Down Expand Up @@ -1116,11 +1110,12 @@ impl ElementRaw {
///
/// This method only applies to elements which contain character data, i.e. `element.content_type` == `CharacterData`,
/// or elements with `element.content_type` == Mixed, but which only contain a single `CharacterData` item
pub(crate) fn set_character_data(
pub(crate) fn set_character_data<T: Into<CharacterData>>(
&mut self,
chardata: CharacterData,
value: T,
version: AutosarVersion,
) -> Result<(), AutosarDataError> {
let chardata: CharacterData = value.into();
if self.elemtype.content_mode() == ContentMode::Characters
|| (self.elemtype.content_mode() == ContentMode::Mixed && self.content.len() <= 1)
{
Expand Down
21 changes: 16 additions & 5 deletions autosar-data/src/iterators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,11 +270,22 @@ mod test {
fn elements_iterator() {
let model = AutosarModel::new();
model.create_file("filename", AutosarVersion::LATEST).unwrap();
let element = model.root_element().create_sub_element(ElementName::ArPackages).unwrap();
element.create_named_sub_element(ElementName::ArPackage, "pkg1").unwrap();
element.create_named_sub_element(ElementName::ArPackage, "pkg2").unwrap();
element.create_named_sub_element(ElementName::ArPackage, "pkg3").unwrap();
element.create_named_sub_element(ElementName::ArPackage, "pkg4").unwrap();
let element = model
.root_element()
.create_sub_element(ElementName::ArPackages)
.unwrap();
element
.create_named_sub_element(ElementName::ArPackage, "pkg1")
.unwrap();
element
.create_named_sub_element(ElementName::ArPackage, "pkg2")
.unwrap();
element
.create_named_sub_element(ElementName::ArPackage, "pkg3")
.unwrap();
element
.create_named_sub_element(ElementName::ArPackage, "pkg4")
.unwrap();

// verify that all elements are returned by the iterator
assert_eq!(element.sub_elements().count(), 4);
Expand Down

0 comments on commit 2817fd4

Please sign in to comment.