Skip to content

Commit

Permalink
cli: sort does not duplicate channels and schemas on every message (#…
Browse files Browse the repository at this point in the history
…1171)

### Changelog

- fixed: `mcap sort` would rewrite the schema and channel for every
message in the output file. This is fixed.

### Docs

<!-- Link to a Docs PR, tracking ticket in Linear, OR write "None" if no
documentation changes are needed. -->

### Description

<!-- Describe the problem, what has changed, and motivation behind those
changes. Pretend you are advocating for this change and the reader is
skeptical. -->

<!-- In addition to unit tests, describe any manual testing you did to
validate this change. -->

<table><tr><th>Before</th><th>After</th></tr><tr><td>

<!--before content goes here-->

</td><td>

<!--after content goes here-->

</td></tr></table>

<!-- If necessary, link relevant Linear or Github issues. Use `Fixes:
foxglove/repo#1234` to auto-close the Github issue or Fixes: FG-### for
Linear isses. -->
  • Loading branch information
james-rms authored May 20, 2024
1 parent 2473746 commit c5db32f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 10 deletions.
4 changes: 3 additions & 1 deletion go/cli/mcap/cmd/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func sortFile(w io.Writer, r io.ReadSeeker) error {
return fmt.Errorf("failed to read messages: %w", err)
}
schemas := make(map[uint16]*mcap.Schema)
channels := make(map[uint16]*mcap.Schema)
channels := make(map[uint16]*mcap.Channel)
message := mcap.Message{}
for {
schema, channel, _, err := it.NextInto(&message)
Expand All @@ -138,13 +138,15 @@ func sortFile(w io.Writer, r io.ReadSeeker) error {
if err != nil {
return fmt.Errorf("failed to write schema: %w", err)
}
schemas[schema.ID] = schema
}
}
if _, ok := channels[channel.ID]; !ok {
err := writer.WriteChannel(channel)
if err != nil {
return fmt.Errorf("failed to write channel: %w", err)
}
channels[channel.ID] = channel
}
err = writer.WriteMessage(&message)
if err != nil {
Expand Down
38 changes: 29 additions & 9 deletions go/cli/mcap/cmd/sort_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package cmd

import (
"bytes"
"errors"
"io"
"testing"

"github.com/foxglove/mcap/go/mcap"
Expand Down Expand Up @@ -62,14 +64,32 @@ func TestSortFile(t *testing.T) {
w := &bytes.Buffer{}
require.NoError(t, sortFile(w, reader))

// verify it is now sorted
r, err := mcap.NewReader(bytes.NewReader(w.Bytes()))
lexer, err := mcap.NewLexer(bytes.NewReader(w.Bytes()))
require.NoError(t, err)

it, err := r.Messages(mcap.UsingIndex(false))
require.NoError(t, err)

_, _, msg, err := it.NextInto(nil)
require.NoError(t, err)
assert.Equal(t, 25, int(msg.LogTime))
var schemaCount, channelCount, messageCount int
var lastMessageTime uint64
top:
for {
token, record, err := lexer.Next(nil)
if errors.Is(err, io.EOF) {
break
}
require.NoError(t, err)
switch token {
case mcap.TokenMessage:
messageCount++
message, err := mcap.ParseMessage(record)
require.NoError(t, err)
require.GreaterOrEqual(t, message.LogTime, lastMessageTime)
lastMessageTime = message.LogTime
case mcap.TokenSchema:
schemaCount++
case mcap.TokenChannel:
channelCount++
case mcap.TokenDataEnd:
break top
}
}
assert.Equal(t, 1, schemaCount, "incorrect schema count")
assert.Equal(t, 2, channelCount, "incorrect channel count")
}

0 comments on commit c5db32f

Please sign in to comment.