Skip to content

Commit

Permalink
Support structures with optional fields
Browse files Browse the repository at this point in the history
Support in the encoding/decoding macros for StructureWithOptionalFields.
This involves decoding/encoding an "EncodingMask", and using that to
determine which fields to read.

It is meaningful for binary, less so for JSON, so we ignore it when
decoding JSON objects. No reason to introduce unnecessary fragility.

There was also a bug in the way this was implemented in the custom
struct, which I only discovered after more closely investigating.
  • Loading branch information
einarmo committed Feb 7, 2025
1 parent 9d2a452 commit 997e262
Show file tree
Hide file tree
Showing 7 changed files with 275 additions and 27 deletions.
1 change: 0 additions & 1 deletion async-opcua-macros/src/encoding/attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ pub(crate) struct EncodingFieldAttribute {
pub rename: Option<String>,
pub ignore: bool,
pub no_default: bool,
#[allow(dead_code)]
pub optional: bool,
}

Expand Down
101 changes: 91 additions & 10 deletions async-opcua-macros/src/encoding/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,73 @@ pub fn generate_binary_encode_impl(strct: EncodingStruct) -> syn::Result<TokenSt
let mut byte_len_body = quote! {};
let mut encode_body = quote! {};

let any_optional = strct
.fields
.iter()
.any(|f| f.attr.optional && !f.attr.ignore);

if any_optional {
let mut optional_index = 0;

// Add 4 for the byte length of the 32-bit encoding mask.
byte_len_body.extend(quote! {
size += 4;
});
encode_body.extend(quote! {
let mut encoding_mask = 0u32;
});
for field in &strct.fields {
if !field.attr.optional {
continue;
}
let ident = &field.ident;
encode_body.extend(quote! {
if self.#ident.is_some() {
encoding_mask |= 1 << #optional_index;
}
});
optional_index += 1;
}
encode_body.extend(quote! {
encoding_mask.encode(stream, ctx)?;
});
}

for field in strct.fields {
if field.attr.ignore {
continue;
}

let ident = field.ident;
byte_len_body.extend(quote! {
size += self.#ident.byte_len(ctx);
});
encode_body.extend(quote! {
self.#ident.encode(stream, ctx)?;
});
if field.attr.optional {
byte_len_body.extend(quote! {
if let Some(item) = &self.#ident {
size += item.byte_len(ctx);
}
});
encode_body.extend(quote! {
if let Some(item) = &self.#ident {
item.encode(stream, ctx)?;
}
});
} else {
byte_len_body.extend(quote! {
size += self.#ident.byte_len(ctx);
});
encode_body.extend(quote! {
self.#ident.encode(stream, ctx)?;
});
}
}
let ident = strct.ident;

if any_optional {
encode_body = quote! {

#encode_body
}
}

Ok(quote! {
impl opcua::types::BinaryEncodable for #ident {
#[allow(unused)]
Expand All @@ -48,14 +100,26 @@ pub fn generate_binary_decode_impl(strct: EncodingStruct) -> syn::Result<TokenSt
let mut decode_build = quote! {};

let mut has_context = false;
let any_optional = strct
.fields
.iter()
.any(|f| f.attr.optional && !f.attr.ignore);

if any_optional {
decode_impl.extend(quote! {
let encoding_mask = u32::decode(stream, ctx)?;
});
}

let mut optional_idx = 0;
for field in strct.fields {
if field.attr.ignore {
continue;
}

let ident = field.ident;
let ident_string = ident.to_string();
if ident_string == "request_header" {
let inner = if ident_string == "request_header" {
decode_impl.extend(quote! {
let request_header: opcua::types::RequestHeader = opcua::types::BinaryDecodable::decode(stream, ctx)?;
let __request_handle = request_header.request_handle;
Expand All @@ -64,6 +128,7 @@ pub fn generate_binary_decode_impl(strct: EncodingStruct) -> syn::Result<TokenSt
request_header,
});
has_context = true;
continue;
} else if ident_string == "response_header" {
decode_impl.extend(quote! {
let response_header: opcua::types::ResponseHeader = opcua::types::BinaryDecodable::decode(stream, ctx)?;
Expand All @@ -73,14 +138,30 @@ pub fn generate_binary_decode_impl(strct: EncodingStruct) -> syn::Result<TokenSt
response_header,
});
has_context = true;
continue;
} else if has_context {
quote! {
opcua::types::BinaryDecodable::decode(stream, ctx)
.map_err(|e| e.with_request_handle(__request_handle))?
}
} else {
quote! {
opcua::types::BinaryDecodable::decode(stream, ctx)?
}
};

if field.attr.optional {
decode_build.extend(quote! {
#ident: opcua::types::BinaryDecodable::decode(stream, ctx)
.map_err(|e| e.with_request_handle(__request_handle))?,
#ident: if (encoding_mask & (1 << #optional_idx)) != 0 {
Some(#inner)
} else {
None
},
});
optional_idx += 1;
} else {
decode_build.extend(quote! {
#ident: opcua::types::BinaryDecodable::decode(stream, ctx)?,
#ident: #inner,
});
}
}
Expand Down
53 changes: 47 additions & 6 deletions async-opcua-macros/src/encoding/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,36 @@ use super::{enums::SimpleEnum, EncodingStruct};
pub fn generate_json_encode_impl(strct: EncodingStruct) -> syn::Result<TokenStream> {
let ident = strct.ident;
let mut body = quote! {};

let any_optional = strct
.fields
.iter()
.any(|f| f.attr.optional && !f.attr.ignore);

if any_optional {
let mut optional_index = 0;

body.extend(quote! {
let mut encoding_mask = 0u32;
});
for field in &strct.fields {
if !field.attr.optional {
continue;
}
let ident = &field.ident;
body.extend(quote! {
if self.#ident.as_ref().is_some_and(|f| !f.is_null_json()) {
encoding_mask |= 1 << #optional_index;
}
});
optional_index += 1;
}
body.extend(quote! {
stream.name("EncodingMask")?;
opcua::types::json::JsonEncodable::encode(&encoding_mask, stream, ctx)?;
});
}

for field in strct.fields {
if field.attr.ignore {
continue;
Expand All @@ -20,12 +50,23 @@ pub fn generate_json_encode_impl(strct: EncodingStruct) -> syn::Result<TokenStre
.unwrap_or_else(|| field.ident.to_string().to_case(Case::Pascal));

let ident = field.ident;
body.extend(quote! {
if !self.#ident.is_null_json() {
stream.name(#name)?;
opcua::types::json::JsonEncodable::encode(&self.#ident, stream, ctx)?;
}
});
if field.attr.optional {
body.extend(quote! {
if let Some(item) = &self.#ident {
if !item.is_null_json() {
stream.name(#name)?;
opcua::types::json::JsonEncodable::encode(item, stream, ctx)?;
}
}
});
} else {
body.extend(quote! {
if !self.#ident.is_null_json() {
stream.name(#name)?;
opcua::types::json::JsonEncodable::encode(&self.#ident, stream, ctx)?;
}
});
}
}

Ok(quote! {
Expand Down
24 changes: 17 additions & 7 deletions async-opcua-types/src/custom/custom_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,13 @@ impl BinaryEncodable for DynamicStructure {
}
StructureType::StructureWithOptionalFields => {
let mut encoding_mask = 0u32;
for (idx, (value, field)) in self.data.iter().zip(s.fields.iter()).enumerate() {
if !field.is_optional || !matches!(value, Variant::Empty) {
encoding_mask |= 1 << idx;
let mut optional_idx = 0;
for (value, field) in self.data.iter().zip(s.fields.iter()) {
if field.is_optional {
if !matches!(value, Variant::Empty) {
encoding_mask |= 1 << optional_idx;
}
optional_idx += 1;
}
}
write_u32(stream, encoding_mask)?;
Expand Down Expand Up @@ -514,11 +518,17 @@ impl DynamicTypeLoader {
StructureType::StructureWithOptionalFields => {
let mask = <u32 as BinaryDecodable>::decode(stream, ctx)?;
let mut values = Vec::with_capacity(t.fields.len());
for (idx, field) in t.fields.iter().enumerate() {
if (1 << idx) & mask != 0 {
values.push(self.decode_field(field, stream, ctx)?);
let mut optional_idx = 0;
for field in t.fields.iter() {
if field.is_optional {
if (1 << optional_idx) & mask != 0 {
values.push(self.decode_field(field, stream, ctx)?);
} else {
values.push(Variant::Empty);
}
optional_idx += 1;
} else {
values.push(Variant::Empty);
values.push(self.decode_field(field, stream, ctx)?);
}
}
Ok(Box::new(DynamicStructure {
Expand Down
10 changes: 7 additions & 3 deletions async-opcua-types/src/custom/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,9 +411,13 @@ impl JsonEncodable for DynamicStructure {
}
crate::StructureType::StructureWithOptionalFields => {
let mut encoding_mask = 0u32;
for (idx, (value, field)) in self.data.iter().zip(s.fields.iter()).enumerate() {
if !field.is_optional || !matches!(value, Variant::Empty) {
encoding_mask |= 1 << idx;
let mut optional_idx = 0;
for (value, field) in self.data.iter().zip(s.fields.iter()) {
if field.is_optional {
if !matches!(value, Variant::Empty) {
encoding_mask |= 1 << optional_idx;
}
optional_idx += 1;
}
}
stream.name("EncodingMask")?;
Expand Down
47 changes: 47 additions & 0 deletions async-opcua-types/src/tests/encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,3 +548,50 @@ fn deep_encoding() {
let res = Variant::decode(&mut stream, &ctx);
assert_eq!(res.unwrap_err().status(), StatusCode::BadDecodingError);
}

#[test]
fn test_custom_struct_with_optional() {
mod opcua {
pub use crate as types;
}

#[derive(Debug, PartialEq, Clone, BinaryDecodable, BinaryEncodable)]
pub struct MyStructWithOptionalFields {
foo: i32,
#[opcua(optional)]
my_opt: Option<LocalizedText>,
#[opcua(optional)]
my_opt_2: Option<i32>,
}

let st = MyStructWithOptionalFields {
foo: 123,
my_opt: None,
my_opt_2: None,
};
let ctx_f = ContextOwned::default();
let ctx = ctx_f.context();
// Byte length reflects that optional fields are left out.
assert_eq!(st.byte_len(&ctx), 4 + 4);

serialize_test(st);

let st = MyStructWithOptionalFields {
foo: 123,
my_opt: None,
my_opt_2: Some(321),
};
assert_eq!(st.byte_len(&ctx), 4 + 4 + 4);
serialize_test(st);

let st = MyStructWithOptionalFields {
foo: 123,
my_opt: Some(LocalizedText::new("Foo", "Bar")),
my_opt_2: Some(321),
};
assert_eq!(
st.byte_len(&ctx),
4 + 4 + 4 + st.my_opt.as_ref().unwrap().byte_len(&ctx)
);
serialize_test(st);
}
Loading

0 comments on commit 997e262

Please sign in to comment.