Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
mhutchinson committed Dec 5, 2024
1 parent ba4dd8e commit f32793b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 13 deletions.
17 changes: 6 additions & 11 deletions storage/mysql/mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package mysql
import (
"bytes"
"context"
"crypto/sha256"
"database/sql"
"errors"
"fmt"
Expand Down Expand Up @@ -238,12 +239,9 @@ func (s *Storage) writeTreeState(ctx context.Context, tx *sql.Tx, size uint64, r
// ReadTile returns a full tile or a partial tile at the given level, index and treeSize.
// If the tile is not found, it returns os.ErrNotExist.
//
// TODO: Handle the following scenarios:
// 1. Full tile request with full tile output: Return full tile.
// 2. Full tile request with partial tile output: Return error.
// 3. Partial tile request with full/larger partial tile output: Return trimmed partial tile with correct tile width.
// 4. Partial tile request with partial tile (same width) output: Return partial tile.
// 5. Partial tile request with smaller partial tile output: Return error.
// Note that if a partial tile is requested, but a larger tile is available, this
// will return the largest tile available. This could be trimmed to return only the
// number of entries specifically requested if this behaviour becomes problematic.
func (s *Storage) ReadTile(ctx context.Context, level, index, minTreeSize uint64) ([]byte, error) {
row := s.db.QueryRowContext(ctx, selectSubtreeByLevelAndIndexSQL, level, index)
if err := row.Err(); err != nil {
Expand All @@ -260,10 +258,7 @@ func (s *Storage) ReadTile(ctx context.Context, level, index, minTreeSize uint64
}

requestedWidth := partialTileSize(level, index, minTreeSize)
if requestedWidth == 0 {
requestedWidth = 256
}
numEntries := uint64(len(tile) / 32)
numEntries := uint64(len(tile) / sha256.Size)

if requestedWidth > numEntries {
// If the user has requested a size larger than we have, they can't have it
Expand All @@ -279,7 +274,7 @@ func partialTileSize(level, index, logSize uint64) uint64 {
sizeAtLevel := logSize >> (level * 8)
fullTiles := sizeAtLevel / 256
if index < fullTiles {
return 0
return 256
}
return sizeAtLevel % 256
}
Expand Down
5 changes: 3 additions & 2 deletions storage/mysql/mysql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package mysql_test
import (
"bytes"
"context"
"crypto/sha256"
"database/sql"
"flag"
"fmt"
Expand Down Expand Up @@ -192,7 +193,7 @@ func TestGetTile(t *testing.T) {
wantNotFound bool
}{
{
name: "really too small but that's ok",
name: "requested partial tile for a complete tile",
level: 0, index: 0, treeSize: 10,
wantEntries: 256,
wantNotFound: false,
Expand Down Expand Up @@ -243,7 +244,7 @@ func TestGetTile(t *testing.T) {
}
t.Errorf("got err: %v", err)
}
numEntries := len(tile) / 32
numEntries := len(tile) / sha256.Size
if got, want := numEntries, test.wantEntries; got != want {
t.Errorf("got %d entries, but want %d", got, want)
}
Expand Down

0 comments on commit f32793b

Please sign in to comment.