Phillip/test fileupload #55
Conversation
Codecov Report
|
README.md
Outdated
| @@ -33,13 +33,180 @@ You can start it over the command line (your container name might be different f | |||
|
|
|||
| ### Registration | |||
| - `/api/register` Register a new user. | |||
|
|
|||
| ```json | |||
http/courseEntryHandler_test.go
Outdated
| @@ -35,6 +37,20 @@ func TestAppServer_PostCourseEntryHandler(t *testing.T) { | |||
| } | |||
| return nil, entry | |||
| } | |||
|
|
|||
| service.StoreCourseEntryFilesFn = func(files [][]byte, id string, date time.Time) (error, []string) { | |||
| if len(files) == 0 { | |||
There was a problem hiding this comment.
Maybe use a switch construct here?
| @@ -26,11 +28,13 @@ func (a *AppServer) initialize() { | |||
| privateChain := Chain(protected, Logger(a.Logger), CORS, NewAuthMiddleware(a.UserService)) | |||
| publicChain := Chain(public, Logger(a.Logger), CORS) | |||
| staticChain := Chain(http.FileServer(http.Dir(a.Static)), Logger(a.Logger), CORS) | |||
| filesChain := Chain(http.StripPrefix("/files", http.FileServer(http.Dir(a.Files))), Logger(a.Logger), CORS) | |||
There was a problem hiding this comment.
Shouldn't it be Chain(http.FileServer(http.Dir(a.Files))), http.StripPrefix("/files")...? The first argument of Chain is the final handler.
There was a problem hiding this comment.
http.StripPrefix takes in a string that will be removed from the beginning from the handlers path
So it needs to take in both arguments
phiphi282
force-pushed
the
phillip/test_fileupload
branch
from
4e85910
to
a407051
Compare
| } | ||
| } | ||
|
|
||
| type Uploader interface { | ||
| UploadFile(file []byte, course string, filename string) (error, string) |
There was a problem hiding this comment.
error should be the last value returned.
| } | ||
| } | ||
|
|
||
| type Uploader interface { |
There was a problem hiding this comment.
What is this Interface used for? Wasn't the file uploaded to this server?
There was a problem hiding this comment.
The interface is used for mocking the upload function in our tests
| @@ -39,6 +48,18 @@ func (cES CourseEntryService) StoreCourseEntry(entry *eduboard.CourseEntry, cfu | |||
| return nil, entry | |||
| } | |||
|
|
|||
| func (cES CourseEntryService) StoreCourseEntryFiles(files [][]byte, id string, date time.Time) (error, []string) { | |||
upload/upload.go
Outdated
| type Uploader struct{} | ||
|
|
||
| func (u *Uploader) UploadFile(file []byte, course string, filename string) (error, string) { | ||
| // check content type |
http/courseEntryHandler.go
Outdated
| @@ -43,9 +43,17 @@ func (a *AppServer) PostCourseEntryHandler() httprouter.Handle { | |||
| return | |||
| } | |||
|
|
|||
| pURLs, err := url.URLifyStrings(request.Pictures) | |||
| err, paths := a.CourseEntryService.StoreCourseEntryFiles(request.Pictures, id, request.Date) | |||
There was a problem hiding this comment.
This seems unusual to me. Aren't files usually uploaded using multipart/form-data? Also, what kind of data is the file content here? Is is Base64 encoded? I did not see any decoding in this PR. As a last thought, there is no limit on file size.
There was a problem hiding this comment.
the json decoder decodes the base64 encoded bytes-file automagically.
The filesize is not limited yet.
courseEntry.go
Outdated
| @@ -46,6 +46,7 @@ type CourseEntryDeleter interface { | |||
|
|
|||
| type CourseEntryService interface { | |||
| StoreCourseEntry(entry *CourseEntry, cfu CourseFindUpdater) (err error, courseEntry *CourseEntry) | |||
| StoreCourseEntryFiles(files [][]byte, id string, date time.Time) (err error, paths []string) | |||
There was a problem hiding this comment.
What do you think about passing a []io.Reader here instead of [][]byte? That would make any storing / uploading etc way more flexible.
added file upload
it works
I would prefer to not make tests for upload.go for now