Skip to content

Commit e510c76

Browse files
ckcnikckcnik
committed
fixes after review
1 parent cf5b2b7 commit e510c76

5 files changed

Lines changed: 114 additions & 13 deletions

File tree

‎conversion/conversion.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ type Params struct {
4444
ToStore *string `json:"store"`
4545

4646
// SaveInGroup saves a multi-page image conversion result as a file group.
47-
SaveInGroup *string `json:"save_in_group,omitempty"`
47+
SaveInGroup string `json:"save_in_group,omitempty"`
4848
}
4949

5050
// EncodeReq implements ucare.ReqEncoder

‎conversion/conversion_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func TestConversionParams_Encode(t *testing.T) {
2323
params: Params{
2424
Paths: []string{"uuid/document/-/format/png/"},
2525
ToStore: ucare.String(ToStoreTrue),
26-
SaveInGroup: ucare.String("1"),
26+
SaveInGroup: "1",
2727
},
2828
want: `{
2929
"paths": ["uuid/document/-/format/png/"],

‎conversion/path.go‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,17 @@
11
package conversion
22

33
import (
4+
"errors"
45
"fmt"
56
"strings"
67
)
78

9+
var (
10+
errEmptyUUID = errors.New("conversion: UUID must not be empty")
11+
errIncompleteCut = errors.New("conversion: CutStart and CutLength must both be set")
12+
errResizeModeNoSize = errors.New("conversion: ResizeMode requires Size to be set")
13+
)
14+
815
type DocumentPathOptions struct {
916
UUID string
1017
// Target document format. When empty, defaults to "png" if Page > 0
@@ -15,7 +22,11 @@ type DocumentPathOptions struct {
1522
Page int
1623
}
1724

18-
func BuildDocumentPath(opts DocumentPathOptions) string {
25+
func BuildDocumentPath(opts DocumentPathOptions) (string, error) {
26+
if opts.UUID == "" {
27+
return "", errEmptyUUID
28+
}
29+
1930
format := opts.Format
2031
if format == "" {
2132
if opts.Page > 0 {
@@ -32,7 +43,7 @@ func BuildDocumentPath(opts DocumentPathOptions) string {
3243
fmt.Fprintf(&b, "-/page/%d/", opts.Page)
3344
}
3445

35-
return b.String()
46+
return b.String(), nil
3647
}
3748

3849
type VideoPathOptions struct {
@@ -41,15 +52,19 @@ type VideoPathOptions struct {
4152
Format string
4253
// Output dimensions, for example "640x480".
4354
Size string
44-
ResizeMode string
45-
Quality string
55+
ResizeMode ResizeMode
56+
Quality Quality
4657
CutStart string
4758
CutLength string
4859
// Number of thumbnails to generate.
4960
Thumbs int
5061
}
5162

52-
func BuildVideoPath(opts VideoPathOptions) string {
63+
func BuildVideoPath(opts VideoPathOptions) (string, error) {
64+
if opts.UUID == "" {
65+
return "", errEmptyUUID
66+
}
67+
5368
format := opts.Format
5469
if format == "" {
5570
format = "mp4"
@@ -58,6 +73,9 @@ func BuildVideoPath(opts VideoPathOptions) string {
5873
var b strings.Builder
5974
fmt.Fprintf(&b, "%s/video/-/format/%s/", opts.UUID, format)
6075

76+
if opts.ResizeMode != "" && opts.Size == "" {
77+
return "", errResizeModeNoSize
78+
}
6179
if opts.Size != "" {
6280
fmt.Fprintf(&b, "-/size/%s/", opts.Size)
6381
if opts.ResizeMode != "" {
@@ -69,13 +87,16 @@ func BuildVideoPath(opts VideoPathOptions) string {
6987
fmt.Fprintf(&b, "-/quality/%s/", opts.Quality)
7088
}
7189

72-
if opts.CutStart != "" && opts.CutLength != "" {
90+
if (opts.CutStart != "") != (opts.CutLength != "") {
91+
return "", errIncompleteCut
92+
}
93+
if opts.CutStart != "" {
7394
fmt.Fprintf(&b, "-/cut/%s/%s/", opts.CutStart, opts.CutLength)
7495
}
7596

7697
if opts.Thumbs > 0 {
7798
fmt.Fprintf(&b, "-/thumbs~%d/", opts.Thumbs)
7899
}
79100

80-
return b.String()
101+
return b.String(), nil
81102
}

‎conversion/path_test.go‎

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"testing"
55

66
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
78
)
89

910
func TestBuildDocumentPath(t *testing.T) {
@@ -34,16 +35,34 @@ func TestBuildDocumentPath(t *testing.T) {
3435
opts: DocumentPathOptions{UUID: "abc-123", Format: "png", Page: 3},
3536
want: "abc-123/document/-/format/png/-/page/3/",
3637
},
38+
{
39+
name: "negative_page_ignored",
40+
opts: DocumentPathOptions{UUID: "abc-123", Page: -1},
41+
want: "abc-123/document/-/format/pdf/",
42+
},
43+
{
44+
name: "zero_page_ignored",
45+
opts: DocumentPathOptions{UUID: "abc-123", Format: "png", Page: 0},
46+
want: "abc-123/document/-/format/png/",
47+
},
3748
}
3849

3950
for _, tt := range tests {
4051
t.Run(tt.name, func(t *testing.T) {
4152
t.Parallel()
42-
assert.Equal(t, tt.want, BuildDocumentPath(tt.opts))
53+
got, err := BuildDocumentPath(tt.opts)
54+
require.NoError(t, err)
55+
assert.Equal(t, tt.want, got)
4356
})
4457
}
4558
}
4659

60+
func TestBuildDocumentPath_EmptyUUID(t *testing.T) {
61+
t.Parallel()
62+
_, err := BuildDocumentPath(DocumentPathOptions{})
63+
assert.ErrorIs(t, err, errEmptyUUID)
64+
}
65+
4766
func TestBuildVideoPath(t *testing.T) {
4867
t.Parallel()
4968

@@ -68,20 +87,60 @@ func TestBuildVideoPath(t *testing.T) {
6887
UUID: "abc-123",
6988
Format: "webm",
7089
Size: "640x480",
71-
ResizeMode: "preserve_ratio",
72-
Quality: "best",
90+
ResizeMode: ResizeModePreserveRatio,
91+
Quality: QualityBest,
7392
CutStart: "000:00:05.000",
7493
CutLength: "000:00:15.000",
7594
Thumbs: 10,
7695
},
7796
want: "abc-123/video/-/format/webm/-/size/640x480/preserve_ratio/-/quality/best/-/cut/000:00:05.000/000:00:15.000/-/thumbs~10/",
7897
},
98+
{
99+
name: "size_without_resize_mode",
100+
opts: VideoPathOptions{UUID: "abc-123", Size: "1920x1080"},
101+
want: "abc-123/video/-/format/mp4/-/size/1920x1080/",
102+
},
103+
{
104+
name: "negative_thumbs_ignored",
105+
opts: VideoPathOptions{UUID: "abc-123", Thumbs: -5},
106+
want: "abc-123/video/-/format/mp4/",
107+
},
108+
{
109+
name: "only_quality",
110+
opts: VideoPathOptions{UUID: "abc-123", Quality: QualityLighter},
111+
want: "abc-123/video/-/format/mp4/-/quality/lighter/",
112+
},
113+
}
114+
115+
for _, tt := range tests {
116+
t.Run(tt.name, func(t *testing.T) {
117+
t.Parallel()
118+
got, err := BuildVideoPath(tt.opts)
119+
require.NoError(t, err)
120+
assert.Equal(t, tt.want, got)
121+
})
122+
}
123+
}
124+
125+
func TestBuildVideoPath_Errors(t *testing.T) {
126+
t.Parallel()
127+
128+
tests := []struct {
129+
name string
130+
opts VideoPathOptions
131+
wantErr error
132+
}{
133+
{"empty_uuid", VideoPathOptions{}, errEmptyUUID},
134+
{"resize_mode_without_size", VideoPathOptions{UUID: "abc-123", ResizeMode: "preserve_ratio"}, errResizeModeNoSize},
135+
{"cut_start_only", VideoPathOptions{UUID: "abc-123", CutStart: "000:00:05.000"}, errIncompleteCut},
136+
{"cut_length_only", VideoPathOptions{UUID: "abc-123", CutLength: "000:00:15.000"}, errIncompleteCut},
79137
}
80138

81139
for _, tt := range tests {
82140
t.Run(tt.name, func(t *testing.T) {
83141
t.Parallel()
84-
assert.Equal(t, tt.want, BuildVideoPath(tt.opts))
142+
_, err := BuildVideoPath(tt.opts)
143+
assert.ErrorIs(t, err, tt.wantErr)
85144
})
86145
}
87146
}

‎conversion/service.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,24 @@ const (
3838
ToStoreTrue = "1"
3939
ToStoreFalse = "0"
4040
)
41+
42+
// ResizeMode defines how a video is resized to fit the target dimensions.
43+
type ResizeMode string
44+
45+
const (
46+
ResizeModePreserveRatio ResizeMode = "preserve_ratio"
47+
ResizeModeChangeRatio ResizeMode = "change_ratio"
48+
ResizeModeScaleCrop ResizeMode = "scale_crop"
49+
ResizeModeAddPadding ResizeMode = "add_padding"
50+
)
51+
52+
// Quality defines the output quality for video conversion.
53+
type Quality string
54+
55+
const (
56+
QualityNormal Quality = "normal"
57+
QualityBetter Quality = "better"
58+
QualityBest Quality = "best"
59+
QualityLighter Quality = "lighter"
60+
QualityLightest Quality = "lightest"
61+
)

0 commit comments

Comments
 (0)