From b91a134d5ff65f8a24057f1de6da80d94de8a3ba Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Wed, 21 Feb 2024 14:17:19 +0800 Subject: [PATCH 1/4] fix typo on 'required' --- pmtiles/bucket.go | 6 +++--- pmtiles/bucket_test.go | 10 +++++----- pmtiles/server.go | 8 ++++---- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pmtiles/bucket.go b/pmtiles/bucket.go index be054ff..63be8f9 100644 --- a/pmtiles/bucket.go +++ b/pmtiles/bucket.go @@ -172,7 +172,7 @@ func (b HTTPBucket) NewRangeReaderEtag(ctx context.Context, key string, offset, if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusPartialContent { resp.Body.Close() - if isRefreshRequredCode(resp.StatusCode) { + if isRefreshRequiredCode(resp.StatusCode) { err = &RefreshRequiredError{resp.StatusCode} } else { err = fmt.Errorf("HTTP error: %d", resp.StatusCode) @@ -187,7 +187,7 @@ func (b HTTPBucket) Close() error { return nil } -func isRefreshRequredCode(code int) bool { +func isRefreshRequiredCode(code int) bool { return code == http.StatusPreconditionFailed || code == http.StatusRequestedRangeNotSatisfiable } @@ -217,7 +217,7 @@ func (ba BucketAdapter) NewRangeReaderEtag(ctx context.Context, key string, offs status = 404 if resp != nil { status = resp.StatusCode() - if isRefreshRequredCode(resp.StatusCode()) { + if isRefreshRequiredCode(resp.StatusCode()) { return nil, "", resp.StatusCode(), &RefreshRequiredError{resp.StatusCode()} } } diff --git a/pmtiles/bucket_test.go b/pmtiles/bucket_test.go index bf247c0..70d54ab 100644 --- a/pmtiles/bucket_test.go +++ b/pmtiles/bucket_test.go @@ -110,16 +110,16 @@ func TestHttpBucketRequestRequestEtagFailed(t *testing.T) { _, _, status, err := bucket.NewRangeReaderEtag(context.Background(), "a/b/c", 0, 3, "etag1") assert.Equal(t, "etag1", mock.request.Header.Get("If-Match")) assert.Equal(t, 412, status) - assert.True(t, isRefreshRequredError(err)) + assert.True(t, isRefreshRequiredError(err)) mock.response.StatusCode = 416 _, _, status, err = bucket.NewRangeReaderEtag(context.Background(), "a/b/c", 0, 3, "etag1") assert.Equal(t, 416, status) - assert.True(t, isRefreshRequredError(err)) + assert.True(t, isRefreshRequiredError(err)) mock.response.StatusCode = 404 _, _, status, err = bucket.NewRangeReaderEtag(context.Background(), "a/b/c", 0, 3, "etag1") - assert.False(t, isRefreshRequredError(err)) + assert.False(t, isRefreshRequiredError(err)) assert.Equal(t, 404, status) } @@ -154,7 +154,7 @@ func TestFileBucketReplace(t *testing.T) { // and requesting with old etag fails with refresh required error _, _, status, err = bucket.NewRangeReaderEtag(context.Background(), "archive.pmtiles", 1, 1, etag1) assert.Equal(t, 412, status) - assert.True(t, isRefreshRequredError(err)) + assert.True(t, isRefreshRequiredError(err)) } func TestFileBucketRename(t *testing.T) { @@ -192,5 +192,5 @@ func TestFileBucketRename(t *testing.T) { // and requesting with old etag fails with refresh required error _, _, status, err = bucket.NewRangeReaderEtag(context.Background(), "archive.pmtiles", 1, 1, etag1) assert.Equal(t, 412, status) - assert.True(t, isRefreshRequredError(err)) + assert.True(t, isRefreshRequiredError(err)) } diff --git a/pmtiles/server.go b/pmtiles/server.go index 8111b3d..755f225 100644 --- a/pmtiles/server.go +++ b/pmtiles/server.go @@ -159,7 +159,7 @@ func (server *Server) Start() { if err != nil { ok = false - result.badEtag = isRefreshRequredError(err) + result.badEtag = isRefreshRequiredError(err) resps <- response{key: key, value: result} server.logger.Printf("failed to fetch %s %d-%d, %v", key.name, key.offset, key.length, err) return @@ -256,7 +256,7 @@ func (server *Server) getHeaderMetadataAttempt(ctx context.Context, name, purgeE defer func() { tracker.finish(ctx, status) }() r, _, statusCode, err := server.bucket.NewRangeReaderEtag(ctx, name+".pmtiles", int64(header.MetadataOffset), int64(header.MetadataLength), rootValue.etag) status = strconv.Itoa(statusCode) - if isRefreshRequredError(err) { + if isRefreshRequiredError(err) { return false, HeaderV3{}, nil, rootValue.etag, nil } if err != nil { @@ -393,7 +393,7 @@ func (server *Server) getTileAttempt(ctx context.Context, httpHeaders map[string defer func() { tracker.finish(ctx, status) }() r, _, statusCode, err := server.bucket.NewRangeReaderEtag(ctx, name+".pmtiles", int64(header.TileDataOffset+entry.Offset), int64(entry.Length), rootValue.etag) status = strconv.Itoa(statusCode) - if isRefreshRequredError(err) { + if isRefreshRequiredError(err) { return 500, httpHeaders, []byte("I/O Error"), rootValue.etag } // possible we have the header/directory cached but the archive has disappeared @@ -429,7 +429,7 @@ func (server *Server) getTileAttempt(ctx context.Context, httpHeaders map[string return 204, httpHeaders, nil, "" } -func isRefreshRequredError(err error) bool { +func isRefreshRequiredError(err error) bool { _, ok := err.(*RefreshRequiredError) return ok } From 891f1bbd072baae988f48e89735573262bf5d8ba Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Wed, 21 Feb 2024 14:34:19 +0800 Subject: [PATCH 2/4] remove extraneous print --- pmtiles/bucket_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pmtiles/bucket_test.go b/pmtiles/bucket_test.go index 70d54ab..53a2457 100644 --- a/pmtiles/bucket_test.go +++ b/pmtiles/bucket_test.go @@ -127,7 +127,6 @@ func TestFileBucketReplace(t *testing.T) { tmp := t.TempDir() bucketURL, _, err := NormalizeBucketKey("", tmp, "") assert.Nil(t, err) - fmt.Println(bucketURL) bucket, err := OpenBucket(context.Background(), bucketURL, "") assert.Nil(t, err) assert.NotNil(t, bucket) From e0f80f908c494ed164e3772b0f2edc34b101cc92 Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Wed, 21 Feb 2024 15:11:18 +0800 Subject: [PATCH 3/4] File bucket handles archives less than 16kb --- pmtiles/bucket.go | 7 +++++++ pmtiles/bucket_test.go | 17 +++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/pmtiles/bucket.go b/pmtiles/bucket.go index 63be8f9..bf985ba 100644 --- a/pmtiles/bucket.go +++ b/pmtiles/bucket.go @@ -124,12 +124,19 @@ func (b FileBucket) NewRangeReaderEtag(_ context.Context, key string, offset, le } result := make([]byte, length) read, err := file.ReadAt(result, offset) + + if err == io.EOF && offset == 0 { + part := result[0:read] + return io.NopCloser(bytes.NewReader(part)), newEtag, 206, nil + } + if err != nil { return nil, "", 500, err } if read != int(length) { return nil, "", 416, fmt.Errorf("Expected to read %d bytes but only read %d", length, read) } + return io.NopCloser(bytes.NewReader(result)), newEtag, 206, nil } diff --git a/pmtiles/bucket_test.go b/pmtiles/bucket_test.go index 53a2457..cc976d1 100644 --- a/pmtiles/bucket_test.go +++ b/pmtiles/bucket_test.go @@ -2,7 +2,6 @@ package pmtiles import ( "context" - "fmt" "io" "net/http" "os" @@ -163,7 +162,6 @@ func TestFileBucketRename(t *testing.T) { bucketURL, _, err := NormalizeBucketKey("", tmp, "") assert.Nil(t, err) - fmt.Println(bucketURL) bucket, err := OpenBucket(context.Background(), bucketURL, "") assert.Nil(t, err) assert.NotNil(t, bucket) @@ -193,3 +191,18 @@ func TestFileBucketRename(t *testing.T) { assert.Equal(t, 412, status) assert.True(t, isRefreshRequiredError(err)) } + +func TestFileShorterThan16K(t *testing.T) { + tmp := t.TempDir() + assert.Nil(t, os.WriteFile(filepath.Join(tmp, "archive.pmtiles"), []byte{1, 2, 3}, 0666)) + + bucketURL, _, err := NormalizeBucketKey("", tmp, "") + bucket, err := OpenBucket(context.Background(), bucketURL, "") + + reader, _, status, err := bucket.NewRangeReaderEtag(context.Background(), "archive.pmtiles", 0, 16384, "") + assert.Equal(t, 206, status) + assert.Nil(t, err) + data, err := io.ReadAll(reader) + assert.Nil(t, err) + assert.Equal(t, 3, len(data)) +} From 08536b96303c4b696ec076645f54aaa853e66cc8 Mon Sep 17 00:00:00 2001 From: Brandon Liu Date: Sun, 25 Feb 2024 22:49:14 +0800 Subject: [PATCH 4/4] file bucket can return bytes than requested --- pmtiles/bucket.go | 10 +++++++--- pmtiles/server_test.go | 3 --- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/pmtiles/bucket.go b/pmtiles/bucket.go index bf985ba..f48724b 100644 --- a/pmtiles/bucket.go +++ b/pmtiles/bucket.go @@ -60,11 +60,15 @@ func (m mockBucket) NewRangeReaderEtag(_ context.Context, key string, offset int if len(etag) > 0 && resultEtag != etag { return nil, "", 412, &RefreshRequiredError{} } - if offset+length > int64(len(bs)) { + if offset > int64(len(bs)) { return nil, "", 416, &RefreshRequiredError{416} } - return io.NopCloser(bytes.NewReader(bs[offset:(offset + length)])), resultEtag, 206, nil + end := offset + length + if end > int64(len(bs)) { + end = int64(len(bs)) + } + return io.NopCloser(bytes.NewReader(bs[offset:end])), resultEtag, 206, nil } // FileBucket is a bucket backed by a directory on disk @@ -125,7 +129,7 @@ func (b FileBucket) NewRangeReaderEtag(_ context.Context, key string, offset, le result := make([]byte, length) read, err := file.ReadAt(result, offset) - if err == io.EOF && offset == 0 { + if err == io.EOF { part := result[0:read] return io.NopCloser(bytes.NewReader(part)), newEtag, 206, nil } diff --git a/pmtiles/server_test.go b/pmtiles/server_test.go index 76bbf46..38263d5 100644 --- a/pmtiles/server_test.go +++ b/pmtiles/server_test.go @@ -102,9 +102,6 @@ func fakeArchive(t *testing.T, header HeaderV3, metadata map[string]interface{}, archiveBytes = append(archiveBytes, metadataBytes...) archiveBytes = append(archiveBytes, leavesBytes...) archiveBytes = append(archiveBytes, tileDataBytes...) - if len(archiveBytes) < 16384 { - archiveBytes = append(archiveBytes, make([]byte, 16384-len(archiveBytes))...) - } return archiveBytes }