From cb43734b7b0ea9c02b93956bdb2e5a91cf03ed9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reynir=20Bj=C3=B6rnsson?= Date: Thu, 7 Jul 2022 17:36:24 +0200 Subject: [PATCH] Refactor and address #127 --- lib/builder_web.ml | 33 +++++++++++++++++++++++++-------- lib/link.ml | 25 ++++++++----------------- lib/views.ml | 8 ++++---- test/router.ml | 20 +++++++++++--------- 4 files changed, 48 insertions(+), 38 deletions(-) diff --git a/lib/builder_web.ml b/lib/builder_web.ml index adb2f8a..24e5126 100644 --- a/lib/builder_web.ml +++ b/lib/builder_web.ml @@ -302,11 +302,7 @@ let routes ~datadir ~cachedir ~configdir = |> string_of_html |> Dream.html |> Lwt_result.ok in - let redirect_latest req = - let job_name = Dream.param req "job" in - let platform = Dream.query req "platform" in - (* FIXME *) - let artifact = begin[@alert "-deprecated"] Dream.path req |> String.concat "/" end in + let redirect_latest req ~job_name ~platform ~artifact = (Dream.sql req (Model.job_id job_name) >>= Model.not_found >>= fun job_id -> Dream.sql req (Model.latest_successful_build_uuid job_id platform)) >>= Model.not_found @@ -316,13 +312,32 @@ let routes ~datadir ~cachedir ~configdir = |> Lwt_result.ok in + let redirect_latest req = + let job_name = Dream.param req "job" in + let platform = Dream.query req "platform" in + let artifact = + (* FIXME Dream.path deprecated *) + let path = begin[@alert "-deprecated"] Dream.path req end in + if path = [] then + "" (* redirect without trailing slash *) + else + "/" ^ (List.map Uri.pct_encode path |> String.concat "/") + in + redirect_latest req ~job_name ~platform ~artifact + + and redirect_latest_no_slash req = + let job_name = Dream.param req "job" in + let platform = Dream.query req "platform" in + redirect_latest req ~job_name ~platform ~artifact:"" + in + let redirect_main_binary req = let job_name = Dream.param req "job" and build = Dream.param req "build" in get_uuid build >>= fun uuid -> Dream.sql req (main_binary_of_uuid uuid) >>= fun main_binary -> - let filepath = main_binary.Builder_db.filepath in - Link.Job_build_f.make ~job_name ~build:uuid ~filepath () + let artifact = `File main_binary.Builder_db.filepath in + Link.Job_build_artifact.make ~job_name ~build:uuid ~artifact () |> Dream.redirect req |> Lwt_result.ok in @@ -597,6 +612,7 @@ let routes ~datadir ~cachedir ~configdir = `Get, "/job/:job/build", (w redirect_parent); `Get, "/job/:job/failed", (w job_with_failed); `Get, "/job/:job/build/latest/**", (w redirect_latest); + `Get, "/job/:job/build/latest", (w redirect_latest_no_slash); `Get, "/job/:job/build/:build", (w job_build); `Get, "/job/:job/build/:build/f/**", (w job_build_file); `Get, "/job/:job/build/:build/main-binary", (w redirect_main_binary); @@ -619,7 +635,8 @@ let to_dream_route = function let to_dream_routes l = List.map to_dream_route l let routeprefix_ignorelist_when_removing_trailing_slash = [ - "/job/:job/build/:build/f" + "/job/:job/build/:build/f"; + "/job/:job/build/latest"; ] module Middleware = struct diff --git a/lib/link.ml b/lib/link.ml index 4ee0781..bd990d7 100644 --- a/lib/link.ml +++ b/lib/link.ml @@ -52,15 +52,16 @@ end module Job_build_artifact = struct let encode_artifact = function - | `Main_binary -> "main-binary" - | `Viz_treemap -> "viztreemap" - | `Viz_dependencies -> "vizdependencies" - | `Script -> "script" - | `Console -> "console" - | `All_targz -> "all.tar.gz" + | `Main_binary -> "/main-binary" + | `Viz_treemap -> "/viztreemap" + | `Viz_dependencies -> "/vizdependencies" + | `Script -> "/script" + | `Console -> "/console" + | `All_targz -> "/all.tar.gz" + | `File f -> "/f/" ^ Fmt.to_to_string fpath_url_pp f let make_from_string ~job_name ~build ~artifact () = - Fmt.str "/job/%a/build/%a/%s" + Fmt.str "/job/%a/build/%a%s" pctencode job_name Uuidm.pp build artifact @@ -71,16 +72,6 @@ module Job_build_artifact = struct end -module Job_build_f = struct - - let make ~job_name ~build ~filepath () = - Fmt.str "/job/%a/build/%a/f/%a" - pctencode job_name - Uuidm.pp build - fpath_url_pp filepath - -end - module Compare_builds = struct let make ~left ~right () = diff --git a/lib/views.ml b/lib/views.ml index d3ffd5a..75f5d05 100644 --- a/lib/views.ml +++ b/lib/views.ml @@ -191,10 +191,10 @@ let artifact ~file:{ Builder_db.filepath; localpath = _; sha256; size } = let artifact_link = - Link.Job_build_f.make + Link.Job_build_artifact.make ~job_name ~build:build.Builder_db.Build.uuid - ~filepath () + ~artifact:(`File filepath) () in [ H.a ~a:H.[a_href artifact_link] [ @@ -476,10 +476,10 @@ module Job_build = struct let (`Hex sha256_hex) = Hex.of_cstruct file.sha256 in [ H.dt [ - H.a ~a:H.[a_href @@ Link.Job_build_f.make + H.a ~a:H.[a_href @@ Link.Job_build_artifact.make ~job_name ~build:build_uuid - ~filepath:file.filepath () + ~artifact:(`File file.filepath) () ] [H.code [txtf "%a" Fpath.pp file.filepath]] ]; H.dd ([ diff --git a/test/router.ml b/test/router.ml index 8a1a6fd..3be03cd 100644 --- a/test/router.ml +++ b/test/router.ml @@ -110,7 +110,7 @@ let () = test_case "Link.Job_build_artifact.make_from_string" `Quick begin let job_name = "test" in let build = Uuidm.v `V4 in - let artifact = "all.tar.gz" in + let artifact = "" in test_link `GET @@ Builder_web.Link.Job_build_artifact.make_from_string ~job_name ~build ~artifact () @@ -123,10 +123,11 @@ let () = `Script; `Console; `All_targz; + `File Fpath.(v "some" / "path"); ] |> List.map (fun artifact -> let descr = - Fmt.str "Link..artifact..: %s" @@ + Fmt.str "Job_build_artifact.make: %s" @@ Builder_web.Link.Job_build_artifact.encode_artifact artifact in test_case descr `Quick begin @@ -134,13 +135,6 @@ let () = end ) ) @ Alcotest.[ - test_case "Link.Job_build_f.make" `Quick begin - let job_name = "test" in - let build = Uuidm.v `V4 in - let filepath = Fpath.(v "some" / "path") in - test_link `GET @@ - Builder_web.Link.Job_build_f.make ~job_name ~build ~filepath () - end; test_case "Link.Compare_builds.make" `Quick begin let left = Uuidm.v `V4 in let right = Uuidm.v `V4 in @@ -152,4 +146,12 @@ let () = Builder_web.Link.Failed_builds.make ~count:2 ~start:1 () end; ]; + (* this doesn't actually test the redirects, unfortunately *) + "Latest", List.map (fun p -> Alcotest.(test_case ("…"^p) `Quick begin + test_link `GET @@ "/job/test/build/latest" ^ p end)) + [ + "/f/bin/unikernel.hvt"; + "/"; + ""; + ] ]