Skip to content

Properly serialize fields when emit_fields option is enabled - #160

Open
hackermondev wants to merge 1 commit into
influxdata:mainfrom
hackermondev:daniel/fix-optional-fields-serialization
Open

hackermondev wants to merge 1 commit into
influxdata:mainfrom
hackermondev:daniel/fix-optional-fields-serialization

Conversation

@hackermondev

Copy link
Copy Markdown

When the emit_fields option is enabled, it doesn't properly differentiate between default fields (that should be emitted) and optional fields (which should only be emitted if they have a value); there's a mismatch in write_struct_serialize_start and write_serialize_field on whether optional fields are emitted, resulting in an invalid length passed to Serializer::serialize_struct which breaks serialization.

message Host {
    required string host = 1;
    optional agent.http_crawling.ranking.RankingSignals ranking_signals = 2;
    map<string, double> queue_scores = 3;
}
impl serde::Serialize for Host {
    #[allow(deprecated)]
    fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
    where
        S: serde::Serializer,
    {
        use serde::ser::SerializeStruct;
        let mut len = 0;
        if true {
            len += 1;
        }
        if true {
            len += 1;
        }
        if true {
            len += 1;
        }
        let mut struct_ser = serializer.serialize_struct("agent.http_crawling.data.Host", len)?;
        if true {
            struct_ser.serialize_field("host", &self.host)?;
        }
        if let Some(v) = self.ranking_signals.as_ref() {
            struct_ser.serialize_field("rankingSignals", v)?;
        }
        if true {
            struct_ser.serialize_field("queueScores", &self.queue_scores)?;
        }
        struct_ser.end()
    }
}

It only emits rankingSignals if the field has a value, but when calculating the length, it always counts the field.

This change ensures only default fields are emitted when emit_fields is enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant