Skip to content

Commit

Permalink
fix: Fixed regression introduced by 0.5.3 release
Browse files Browse the repository at this point in the history
  • Loading branch information
rholshausen committed Nov 17, 2024
1 parent 7abcf38 commit d6b1626
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 14 deletions.
19 changes: 17 additions & 2 deletions src/dynamic_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::iter::Peekable;
use std::slice::Iter;

use anyhow::{anyhow, bail};
use bytes::{BufMut, Bytes};
use bytes::{Buf, BufMut, Bytes};
use itertools::Itertools;
use pact_matching::generators::DefaultVariantMatcher;
use pact_models::expression_parser::DataValue;
Expand Down Expand Up @@ -108,6 +108,20 @@ impl DynamicMessage {
self.fields.values().cloned().collect()
}

/// Return a flattened vector of the fields. This will expand repeated fields.
pub fn flatten_fields(&self) -> Vec<ProtobufField> {
self.fields.values()
.flat_map(|f| {
let mut result = vec![ f.clone() ];
if f.repeated_field() && !f.additional_data.is_empty() {
result.extend(f.additional_data.iter()
.map(|d| f.clone_with_data(d)));
}
result
})
.collect()
}

/// Encode this message to the provided buffer
pub fn write_to<B>(&self, buffer: &mut B) -> anyhow::Result<()> where B: BufMut {
for (field_num, field) in self.fields.iter()
Expand Down Expand Up @@ -416,8 +430,9 @@ impl Decoder for DynamicMessageDecoder {
type Item = DynamicMessage;
type Error = Status;

#[instrument]
#[instrument(skip_all, fields(bytes = src.remaining()))]
fn decode(&mut self, src: &mut DecodeBuf<'_>) -> Result<Option<Self::Item>, Self::Error> {
trace!("Incoming bytes = {:?}", src);
match decode_message(src, &self.descriptor, &self.file_descriptor_set) {
Ok(fields) => Ok(Some(DynamicMessage::new(fields.as_slice(), &self.file_descriptor_set))),
Err(err) => {
Expand Down
16 changes: 9 additions & 7 deletions src/matching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ pub fn match_service(
}

/// Compare the expected message to the actual one
#[tracing::instrument(ret, skip_all)]
#[tracing::instrument(ret, skip_all, fields(expected_fields = expected_message.len(), actual_fields = actual_message.len()) )]
pub(crate) fn compare(
message_descriptor: &DescriptorProto,
expected_message: &[ProtobufField],
Expand Down Expand Up @@ -205,7 +205,7 @@ fn should_use_default(descriptor: &FieldDescriptorProto) -> bool {
/// Compare the fields of the expected and actual messages
#[tracing::instrument(ret,
skip_all,
fields(%path, expected_message_fields, actual_message_fields)
fields(%path, expected_fields = expected_message_fields.len(), actual_fields=actual_message_fields.len())
)]
pub fn compare_message(
path: DocPath,
Expand Down Expand Up @@ -565,7 +565,7 @@ fn compare_value<T>(
}

/// Compare a repeated field
#[tracing::instrument(ret, skip_all, fields(%path))]
#[tracing::instrument(ret, skip_all, fields(%path, expected = expected_fields.len(), actual = actual_fields.len()))]
fn compare_repeated_field(
path: &DocPath,
descriptor: &FieldDescriptorProto,
Expand Down Expand Up @@ -595,12 +595,12 @@ fn compare_repeated_field(
}
}
} else if expected_fields.is_empty() && !actual_fields.is_empty() {
debug!("Expected an empty list, but actual has {} fields", actual_fields.len());
debug!("Expected an empty list, but actual has {} field(s)", actual_fields.len());
result.push(Mismatch::BodyMismatch {
path: path.to_string(),
expected: None,
actual: None,
mismatch: format!("Expected repeated field '{}' to be empty but received {} values",
mismatch: format!("Expected repeated field '{}' to be empty but received {} value(s)",
descriptor.name.clone().unwrap_or_else(|| descriptor.number.unwrap_or_default().to_string()),
actual_fields.len()
)
Expand All @@ -613,10 +613,12 @@ fn compare_repeated_field(
path: path.to_string(),
expected: None,
actual: None,
mismatch: format!("Expected repeated field '{}' to have {} values but received {} values",
mismatch: format!("Expected repeated field '{}' to have {} value{} but received {} value{}",
descriptor.name.clone().unwrap_or_else(|| descriptor.number.unwrap_or_default().to_string()),
expected_fields.len(),
actual_fields.len()
if expected_fields.len() > 1 { "s" } else { "" },
actual_fields.len(),
if actual_fields.len() > 1 { "s" } else { "" }
)
})
}
Expand Down
18 changes: 16 additions & 2 deletions src/message_decoder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,15 @@ impl ProtobufField {
pub fn repeated_field(&self) -> bool {
is_repeated_field(&self.descriptor)
}

/// Creates a new field that is a clone of this one but with the data set
pub fn clone_with_data(&self, data: &ProtobufFieldData) -> Self {
ProtobufField {
data: data.clone(),
additional_data: vec![],
.. self.clone()
}
}
}

fn default_field_data(
Expand Down Expand Up @@ -389,7 +398,6 @@ impl Display for ProtobufFieldData {
/// Decodes the Protobuf message using the descriptors and returns an array of ProtobufField values.
/// This will return a value for each field value in the incoming bytes in the same order, and will
/// not consolidate repeated fields.
#[tracing::instrument(ret, skip_all)]
pub fn decode_message<B>(
buffer: &mut B,
descriptor: &DescriptorProto,
Expand Down Expand Up @@ -550,7 +558,13 @@ pub fn decode_message<B>(
}
}

Ok(fields.iter().sorted_by(|a, b| Ord::cmp(&a.field_num, &b.field_num)).cloned().collect())
let result = fields.iter()
.sorted_by(|a, b| Ord::cmp(&a.field_num, &b.field_num))
.cloned()
.collect_vec();
debug!("Decoded message has {} fields", result.len());
trace!("Decoded message = {:?}", result);
Ok(result)
}

fn decode_enum(
Expand Down
151 changes: 148 additions & 3 deletions src/mock_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,12 @@ impl MockService {
response_descriptor: DescriptorProto,
request_metadata: MetadataMap
) -> Result<Response<DynamicMessage>, Status> {
trace!(?request, "Handling request message");
// 1. Compare the incoming message to the request message from the interaction
let mut expected_message_bytes = self.message.request.contents.value().unwrap_or_default();
let expected_message = decode_message(&mut expected_message_bytes, &message_descriptor, &self.file_descriptor_set)
.map_err(|err| Status::invalid_argument(err.to_string()))?;
trace!("Expected message has {} fields", expected_message.len());
let plugin_config = self.pact.plugin_data().iter()
.map(|pd| {
(pd.name.clone(), PluginInteractionConfig {
Expand All @@ -76,8 +78,14 @@ impl MockService {
let context = CoreMatchingContext::new(DiffConfig::NoUnexpectedKeys,
&self.message.request.matching_rules.rules_for_category("body").unwrap_or_default(),
&plugin_config);
let mismatches = compare(&message_descriptor, &expected_message, request.proto_fields().as_slice(), &context,
&expected_message_bytes, &self.file_descriptor_set);
let mismatches = compare(
&message_descriptor,
&expected_message,
request.flatten_fields().as_slice(),
&context,
&expected_message_bytes,
&self.file_descriptor_set
);

// 2. Compare any metadata from the incoming message
let md_context = CoreMatchingContext::new(DiffConfig::NoUnexpectedKeys,
Expand Down Expand Up @@ -231,6 +239,7 @@ impl Service<Request<DynamicMessage>> for MockService {

fn call(&mut self, req: Request<DynamicMessage>) -> Self::Future {
let (request_metadata, _, request) = req.into_parts();
trace!(?request, "Incoming message received");
let message_descriptor = self.input_message.clone();
let response_descriptor = self.output_message.clone();
let service = self.clone();
Expand All @@ -250,7 +259,7 @@ mod tests {
use prost::Message;
use prost_types::FileDescriptorSet;
use serde_json::json;
use tonic::metadata::MetadataMap;
use tonic::metadata::{MetadataMap, MetadataKey, MetadataValue};

use crate::dynamic_message::DynamicMessage;
use crate::message_decoder::decode_message;
Expand Down Expand Up @@ -385,4 +394,140 @@ mod tests {
let area = &response_fields[0];
expect!(area.data.to_string()).to_not(be_equal_to("12"));
}

#[test_log::test(tokio::test)]
async fn handle_message_handles_multiple_field_values() {
// taken from https://github.com/pact-foundation/pact-plugins/tree/main/examples/gRPC/area_calculator
let descriptor_encoded = "CsoHChVhcmVhX2NhbGN1bGF0b3IucHJvdG8SD2FyZWFfY2FsY3VsYXRvciK6Ago\
MU2hhcGVNZXNzYWdlEjEKBnNxdWFyZRgBIAEoCzIXLmFyZWFfY2FsY3VsYXRvci5TcXVhcmVIAFIGc3F1YXJlEjoKC\
XJlY3RhbmdsZRgCIAEoCzIaLmFyZWFfY2FsY3VsYXRvci5SZWN0YW5nbGVIAFIJcmVjdGFuZ2xlEjEKBmNpcmNsZRg\
DIAEoCzIXLmFyZWFfY2FsY3VsYXRvci5DaXJjbGVIAFIGY2lyY2xlEjcKCHRyaWFuZ2xlGAQgASgLMhkuYXJlYV9jY\
WxjdWxhdG9yLlRyaWFuZ2xlSABSCHRyaWFuZ2xlEkYKDXBhcmFsbGVsb2dyYW0YBSABKAsyHi5hcmVhX2NhbGN1bGF\
0b3IuUGFyYWxsZWxvZ3JhbUgAUg1wYXJhbGxlbG9ncmFtQgcKBXNoYXBlIikKBlNxdWFyZRIfCgtlZGdlX2xlbmd0a\
BgBIAEoAlIKZWRnZUxlbmd0aCI5CglSZWN0YW5nbGUSFgoGbGVuZ3RoGAEgASgCUgZsZW5ndGgSFAoFd2lkdGgYAiA\
BKAJSBXdpZHRoIiAKBkNpcmNsZRIWCgZyYWRpdXMYASABKAJSBnJhZGl1cyJPCghUcmlhbmdsZRIVCgZlZGdlX2EYAS\
ABKAJSBWVkZ2VBEhUKBmVkZ2VfYhgCIAEoAlIFZWRnZUISFQoGZWRnZV9jGAMgASgCUgVlZGdlQyJICg1QYXJhbGxlbG\
9ncmFtEh8KC2Jhc2VfbGVuZ3RoGAEgASgCUgpiYXNlTGVuZ3RoEhYKBmhlaWdodBgCIAEoAlIGaGVpZ2h0IkQKC0FyZW\
FSZXF1ZXN0EjUKBnNoYXBlcxgBIAMoCzIdLmFyZWFfY2FsY3VsYXRvci5TaGFwZU1lc3NhZ2VSBnNoYXBlcyIkCgxBcm\
VhUmVzcG9uc2USFAoFdmFsdWUYASADKAJSBXZhbHVlMq0BCgpDYWxjdWxhdG9yEk4KDGNhbGN1bGF0ZU9uZRIdLmFyZW\
FfY2FsY3VsYXRvci5TaGFwZU1lc3NhZ2UaHS5hcmVhX2NhbGN1bGF0b3IuQXJlYVJlc3BvbnNlIgASTwoOY2FsY3VsY\
XRlTXVsdGkSHC5hcmVhX2NhbGN1bGF0b3IuQXJlYVJlcXVlc3QaHS5hcmVhX2NhbGN1bGF0b3IuQXJlYVJlc3BvbnNl\
IgBCHFoXaW8ucGFjdC9hcmVhX2NhbGN1bGF0b3LQAgFiBnByb3RvMw==";
let bytes = BASE64.decode(descriptor_encoded).unwrap();
let bytes1 = Bytes::copy_from_slice(bytes.as_slice());
let file_descriptor_set = FileDescriptorSet::decode(bytes1).unwrap();

let ac_desc = file_descriptor_set.file.iter()
.find(|ds| ds.name.clone().unwrap_or_default() == "area_calculator.proto")
.cloned()
.unwrap();
let service_desc = ac_desc.service.iter()
.find(|sd| sd.name.clone().unwrap_or_default() == "Calculator")
.unwrap();
let method = service_desc.method.iter()
.find(|md| md.name.clone().unwrap_or_default() == "calculateMulti")
.unwrap();
let input_message = ac_desc.message_type.iter()
.find(|md| md.name.clone().unwrap_or_default() == "AreaRequest")
.unwrap();
let output_message = ac_desc.message_type.iter()
.find(|md| md.name.clone().unwrap_or_default() == "AreaResponse")
.unwrap();

let request_bytes: &[u8] = [10, 12, 18, 10, 13, 0, 0, 64, 64, 21, 0, 0, 128, 64, 10, 7, 10, 5, 13, 0, 0, 64, 64].as_slice();
let pact_json = json!({
"interactions": [
{
"type": "Synchronous/Messages",
"description": "calculate rectangle area request",
"request": {
"contents": {
"content": BASE64.encode(request_bytes),
"contentType": "application/protobuf; message=AreaRequest",
"contentTypeHint": "BINARY",
"encoded": "base64"
},
"metadata": {
"contentType": "application/protobuf;message=.area_calculator.AreaRequest"
},
"matchingRules": {
"body": {
"$.shapes[0].rectangle.length": {
"combine": "AND",
"matchers": [
{
"match": "number"
}
]
},
"$.shapes[0].rectangle.width": {
"combine": "AND",
"matchers": [
{
"match": "number"
}
]
},
"$.shapes[1].square.edge_length": {
"combine": "AND",
"matchers": [
{
"match": "number"
}
]
}
}
}
},
"response": [
{
"contents": {
"content": "CgQAAEBB",
"contentType": "application/protobuf; message=.area_calculator.AreaResponse",
"contentTypeHint": "BINARY",
"encoded": "base64"
},
"metadata": {
"contentType": "application/protobuf;message=.area_calculator.AreaResponse"
}
}
],
"pluginConfiguration": {
"protobuf": {
"descriptorKey": "d58838959e37498cddf51805bedf4dca",
"service": ".area_calculator.Calculator/calculateMulti"
}
},
"transport": "grpc"
}
],
"metadata": {
"pactSpecification": { "version": "4.0" }
}
});

let pact = V4Pact::pact_from_json(&pact_json, "<>").unwrap();
let message = pact.interactions.first().unwrap();

let mut bytes2 = Bytes::from(b"\n\x0c\x12\n\r\0\0@@\x15\0\0\x80@\n\x07\n\x05\r\0\0@@".as_slice());
let fields = decode_message(&mut bytes2, input_message, &file_descriptor_set).unwrap();
let request = DynamicMessage::new(fields.as_slice(), &file_descriptor_set);

let mock_service = MockService {
file_descriptor_set: file_descriptor_set.clone(),
service_name: "Calculator".to_string(),
message: message.as_v4_sync_message().unwrap(),
method_descriptor: method.clone(),
input_message: input_message.clone(),
output_message: output_message.clone(),
server_key: "9876789".to_string(),
pact
};

let mut md = MetadataMap::new();
md.insert(MetadataKey::from_static("contenttype"), MetadataValue::from_static("application/protobuf;message=.area_calculator.AreaRequest"));
let response = mock_service.handle_message(request, input_message.clone(), output_message.clone(),
md).await;
expect!(response).to(be_ok());
}
}

0 comments on commit d6b1626

Please sign in to comment.