particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

api/jobqueue: move build id to url

Browse source 
Imho it makes more sense from REST perspective. Also, in the future there
will be ROUTE for uploading image to image build. As it's not a good idea
transport file inside JSON, all the parameters (compose id and image
build id) need to be inside the URL. Therefore for the sake of consistency,
all these routes should have compose id and image build id in the URL.

There is another solution to embedding multiple values inside http body
which allows file transport - multipart/form-data. I think using form-data
is worth when doing more complex stuff, for our usecase transporting all
the metadata in the URL is more appropriate solution.
This commit is contained in:
Ondřej Budai 2020-02-12 15:18:47 +01:00 • committed by Tom Gundersen
parent 188b24e26e
commit b64bbaa0bb
4 changed files with 21 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

14
internal/jobqueue/api.go
View file

@ -5,6 +5,7 @@ import (
"log"
"net"
"net/http"
"strconv"
"github.com/osbuild/osbuild-composer/internal/store"
@ -31,7 +32,7 @@ func New(logger *log.Logger, store *store.Store) *API {
api.router.NotFound = http.HandlerFunc(notFoundHandler)
api.router.POST("/job-queue/v1/jobs", api.addJobHandler)
api.router.PATCH("/job-queue/v1/jobs/:id", api.updateJobHandler)
api.router.PATCH("/job-queue/v1/jobs/:job_id/builds/:build_id", api.updateJobHandler)
return api
}
@ -114,12 +115,19 @@ func (api *API) updateJobHandler(writer http.ResponseWriter, request *http.Reque
return
}
id, err := uuid.Parse(params.ByName("id"))
id, err := uuid.Parse(params.ByName("job_id"))
if err != nil {
statusResponseError(writer, http.StatusBadRequest, "invalid compose id: "+err.Error())
return
}
imageBuildId, err := strconv.Atoi(params.ByName("build_id"))
if err != nil {
statusResponseError(writer, http.StatusBadRequest, "invalid image build id: "+err.Error())
return
}
var body JobStatus
err = json.NewDecoder(request.Body).Decode(&body)
if err != nil {
@ -127,7 +135,7 @@ func (api *API) updateJobHandler(writer http.ResponseWriter, request *http.Reque
return
}
err = api.store.UpdateImageBuildInCompose(id, body.ImageBuildID, body.Status, body.Result)
err = api.store.UpdateImageBuildInCompose(id, imageBuildId, body.Status, body.Result)
if err != nil {
switch err.(type) {
case *store.NotFoundError:

10
internal/jobqueue/api_test.go
View file

@ -25,11 +25,11 @@ func TestBasic(t *testing.T) {
// Create job with invalid body
{"POST", "/job-queue/v1/jobs", ``, http.StatusBadRequest, `invalid request: EOF`},
// Update job with invalid ID
{"PATCH", "/job-queue/v1/jobs/foo", `{"status":"RUNNING"}`, http.StatusBadRequest, `invalid compose id: invalid UUID length: 3`},
{"PATCH", "/job-queue/v1/jobs/foo/builds/0", `{"status":"RUNNING"}`, http.StatusBadRequest, `invalid compose id: invalid UUID length: 3`},
// Update job that does not exist, with invalid body
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", ``, http.StatusBadRequest, `invalid status: EOF`},
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", ``, http.StatusBadRequest, `invalid status: EOF`},
// Update job that does not exist
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", `{"image_build_id": 0, "status":"RUNNING"}`, http.StatusNotFound, `compose does not exist`},
{"PATCH", "/job-queue/v1/jobs/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/builds/0", `{"status":"RUNNING"}`, http.StatusNotFound, `compose does not exist`},
}
for _, c := range cases {
@ -72,12 +72,12 @@ func testUpdateTransition(t *testing.T, from, to string, expectedStatus int, exp
if from != "WAITING" {
test.SendHTTP(api, false, "POST", "/job-queue/v1/jobs", `{}`)
if from != "RUNNING" {
test.SendHTTP(api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff", `{"status":"`+from+`"}`)
test.SendHTTP(api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff/builds/0", `{"status":"`+from+`"}`)
}
}
}
test.TestNonJsonRoute(t, api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff", `{"status":"`+to+`"}`, expectedStatus, expectedResponse)
test.TestNonJsonRoute(t, api, false, "PATCH", "/job-queue/v1/jobs/ffffffff-ffff-ffff-ffff-ffffffffffff/builds/0", `{"status":"`+to+`"}`, expectedStatus, expectedResponse)
}
func TestUpdate(t *testing.T) {

5
internal/jobqueue/job.go
View file

@ -26,9 +26,8 @@ type Job struct {
}
type JobStatus struct {
Status common.ImageBuildState `json:"status"`
ImageBuildID int `json:"image_build_id"`
Result *common.ComposeResult `json:"result"`
Status common.ImageBuildState `json:"status"`
Result *common.ComposeResult `json:"result"`
}
type TargetsError struct {