Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/update_schema_metadata_validation_logic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
livekit: patch
livekit-datatrack: patch
livekit-ffi: patch
livekit-uniffi: patch
---

Update schema metadata validation logic - #1328 (@ladvoc)
49 changes: 48 additions & 1 deletion livekit-datatrack/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ pub enum DataTrackSchemaEncoding {
JsonSchema,

/// Another well-known encoding not known to this client version.
///
/// Only produced for received tracks; cannot be used when publishing.
Other,
/// An application-specific encoding identified by the contained string.
///
Expand Down Expand Up @@ -163,6 +165,8 @@ pub enum DataTrackFrameEncoding {
Json,

/// Another well-known encoding not known to this client version.
///
/// Only produced for received tracks; cannot be used when publishing.
Other,
/// An application-specific encoding identified by the contained string.
///
Expand All @@ -186,14 +190,24 @@ pub enum DataTrackSchemaError {
/// Specified schema and frame encodings are incompatible.
#[error("Specified schema and frame encodings are incompatible")]
Incompatible,

/// The 'other' encoding represents an unrecognized encoding on received tracks
/// and cannot be used when publishing.
#[error("The 'other' encoding cannot be used when publishing")]
OtherEncoding,
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines +193 to +197

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick(non-blocking): I wonder if there's a way to do this where you aren't exposing ::Other in the publishing context by splitting up this DataTrackSchemaEncoding into a publishing variant and a subscribing variant. I don't think there would be a way to do this where it wouldn't be a breaking change though, so maybe disregard if you see to many problems with this idea.

}

/// Validates that the given frame and schema encodings are compatible.
pub(crate) fn validate_schema(
frame_encoding: Option<&DataTrackFrameEncoding>,
schema_encoding: Option<&DataTrackSchemaEncoding>,
) -> Result<(), DataTrackSchemaError> {
use DataTrackFrameEncoding as FrameEncoding;
use DataTrackSchemaEncoding as SchemaEncoding;
Comment on lines +205 to +206

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Why alias these here rather than just the fully qualified names below? IMO, it adds some confusion because it's not 100% clear that FrameEncoding and DataTrackFrameEncoding / etc mean the same thing if you enter the file from further down.

match (frame_encoding, schema_encoding) {
(Some(FrameEncoding::Other), _) | (_, Some(SchemaEncoding::Other)) => {
Err(DataTrackSchemaError::OtherEncoding)
}
(None, Some(_)) => Err(DataTrackSchemaError::MissingFrameEncoding),
(Some(frame_encoding), None) => match frame_encoding.is_self_describing() {
Some(false) => Err(DataTrackSchemaError::MissingSchemaId),
Expand Down Expand Up @@ -230,7 +244,10 @@ impl DataTrackFrameEncoding {
| (Self::Protobuf, SchemaEncoding::Protobuf)
| (Self::Flatbuffer, SchemaEncoding::Flatbuffer)
| (Self::Json, SchemaEncoding::JsonSchema) => Some(true),
(Self::Other, _) | (Self::Custom(_), _) => None, // Cannot be determined
(Self::Other, _)
| (Self::Custom(_), _)
| (_, SchemaEncoding::Other)
| (_, SchemaEncoding::Custom(_)) => None, // Cannot be determined
_ => Some(false),
}
}
Expand Down Expand Up @@ -383,6 +400,36 @@ mod tests {
);
}

#[test]
fn test_validate_schema_custom_schema_encoding() {
assert_eq!(
validate_schema(
Some(&DataTrackFrameEncoding::Json),
Some(&DataTrackSchemaEncoding::Custom("my-schema-encoding".to_string()))
),
Ok(())
);
}

#[test]
fn test_validate_schema_other_schema_encoding() {
assert_eq!(
validate_schema(
Some(&DataTrackFrameEncoding::Json),
Some(&DataTrackSchemaEncoding::Other)
),
Err(DataTrackSchemaError::OtherEncoding)
);
}

#[test]
fn test_validate_schema_other_frame_encoding() {
assert_eq!(
validate_schema(Some(&DataTrackFrameEncoding::Other), None),
Err(DataTrackSchemaError::OtherEncoding)
);
}

#[test]
fn test_validate_schema_missing_frame_encoding() {
assert_eq!(
Expand Down
Loading