From 594c6d5917b02a185ba4975919874f88ed4ef8cd Mon Sep 17 00:00:00 2001 From: Robur Date: Fri, 5 Nov 2021 12:49:16 +0000 Subject: [PATCH] remove unused queries --- db/builder_db.ml | 65 ---------------------------------------------- db/builder_db.mli | 47 --------------------------------- lib/builder_web.ml | 2 +- lib/model.ml | 12 ++------- lib/model.mli | 8 +----- test/builder_db.ml | 44 +++---------------------------- 6 files changed, 7 insertions(+), 171 deletions(-) diff --git a/db/builder_db.ml b/db/builder_db.ml index 1710053..4472b8e 100644 --- a/db/builder_db.ml +++ b/db/builder_db.ml @@ -66,12 +66,6 @@ module Job = struct (id `job) "SELECT id FROM job WHERE name = ?" - let get_all = - Caqti_request.collect - Caqti_type.unit - Caqti_type.(tup2 (id `job) string) - "SELECT id, name FROM job ORDER BY name ASC" - let get_all_with_section_synopsis = Caqti_request.collect Caqti_type.unit @@ -110,12 +104,6 @@ module Tag = struct Caqti_type.unit "DROP TABLE IF EXISTS tag" - let get = - Caqti_request.find - (id `tag) - Caqti_type.string - "SELECT tag FROM tag WHERE id = ?" - let get_id_by_name = Caqti_request.find Caqti_type.string @@ -195,15 +183,6 @@ module Build_artifact = struct {| SELECT filepath, localpath, sha256, size FROM build_artifact WHERE id = ? |} - let get_by_build = - Caqti_request.find - (Caqti_type.tup2 (id `build) fpath) - (Caqti_type.tup2 (id `build_artifact) file) - {| SELECT id, filepath, localpath, sha256, size - FROM build_artifact - WHERE build = ? AND filepath = ? - |} - let get_by_build_uuid = Caqti_request.find_opt (Caqti_type.tup2 uuid fpath) @@ -313,17 +292,6 @@ module Build = struct Caqti_type.unit {| DROP TABLE IF EXISTS build |} - let get_opt = - Caqti_request.find_opt - (id `build) - t - {| SELECT uuid, start_d, start_ps, finish_d, finish_ps, - result_code, result_msg, - console, script, platform, main_binary, input_id, user, job - FROM build - WHERE id = ? - |} - let get_by_uuid = Caqti_request.find_opt Rep.uuid @@ -347,23 +315,6 @@ module Build = struct ORDER BY start_d DESC, start_ps DESC |} - let get_all_with_main_binary = - Caqti_request.collect - (id `job) - (Caqti_type.tup3 - (id `build) t file_opt) - {| SELECT build.id, build.uuid, - build.start_d, build.start_ps, build.finish_d, build.finish_ps, - build.result_code, build.result_msg, build.console, build.script, - build.platform, build.main_binary, build.input_id, build.user, build.job, - build_artifact.filepath, build_artifact.localpath, build_artifact.sha256, build_artifact.size - FROM build, job - LEFT JOIN build_artifact ON - build.main_binary = build_artifact.id - WHERE job.id = ? AND build.job = job.id - ORDER BY build.start_d DESC, build.start_ps DESC - |} - let get_all_artifact_sha = Caqti_request.collect (id `job) @@ -407,17 +358,6 @@ module Build = struct LIMIT 1 |} - let get_latest_uuid = - Caqti_request.find_opt - (id `job) - Caqti_type.(tup2 (id `build) Rep.uuid) - {| SELECT b.id, b.uuid - FROM build b - WHERE b.job = ? - ORDER BY b.start_d DESC, b.start_ps DESC - LIMIT 1 - |} - let get_latest_successful_uuid = Caqti_request.find_opt (id `job) @@ -606,11 +546,6 @@ module User = struct VALUES (?, ?, ?, ?, ?, ?, ?) |} - let remove = - Caqti_request.exec - (id `user) - "DELETE FROM user WHERE id = ?" - let remove_user = Caqti_request.exec Caqti_type.string diff --git a/db/builder_db.mli b/db/builder_db.mli index e603be6..d2b81a3 100644 --- a/db/builder_db.mli +++ b/db/builder_db.mli @@ -47,18 +47,11 @@ val last_insert_rowid : (unit, 'a id, [< `Many | `One | `Zero > `One ]) Caqti_request.t module Job : sig - val migrate : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val rollback : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val get : ([`job] id, string, [< `Many | `One | `Zero > `One ]) Caqti_request.t val get_id_by_name : (string, [`job] id, [< `Many | `One | `Zero > `One `Zero ]) Caqti_request.t - val get_all : - (unit, [`job] id * string, [ `Many | `One | `Zero ]) Caqti_request.t val get_all_with_section_synopsis : (unit, [`job] id * string * string option * string option, [ `Many | `One | `Zero ]) Caqti_request.t val try_add : @@ -68,12 +61,6 @@ module Job : sig end module Tag : sig - val migrate : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val rollback : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val get : - ([`tag] id, string, [< `Many | `One | `Zero > `One ]) Caqti_request.t val get_id_by_name : (string, [`tag] id, [< `Many | `One | `Zero > `One ]) Caqti_request.t val try_add : @@ -81,10 +68,6 @@ module Tag : sig end module Job_tag : sig - val migrate : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val rollback : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t val add : ([`tag] id * string * [`job] id, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t val update : @@ -94,16 +77,7 @@ module Job_tag : sig end module Build_artifact : sig - val migrate : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val rollback : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val get : ([`build_artifact] id, file, [< `Many | `One | `Zero > `One]) Caqti_request.t - val get_by_build : - ([`build] id * Fpath.t, [`build_artifact] id * file, - [< `Many | `One | `Zero > `One ]) Caqti_request.t - val get_by_build_uuid : (Uuidm.t * Fpath.t, [`build_artifact] id * file, [< `Many | `One | `Zero > `One `Zero ]) @@ -134,20 +108,11 @@ sig job_id : [`job] id; } - val migrate : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val rollback : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - - val get_opt : - ([`build] id, t, [< `Many | `One | `Zero > `One `Zero ]) Caqti_request.t val get_by_uuid : (Uuidm.t, [`build] id * t, [< `Many | `One | `Zero > `One `Zero ]) Caqti_request.t val get_all : ([`job] id, [`build] id * t, [ `Many | `One | `Zero ]) Caqti_request.t - val get_all_with_main_binary : - ([`job] id, [`build] id * t * file option, [ `Many | `One | `Zero ]) Caqti_request.t val get_all_artifact_sha : ([`job] id, Cstruct.t, [ `Many | `One | `Zero ]) Caqti_request.t val get_latest : @@ -155,9 +120,6 @@ sig Caqti_request.t val get_latest_failed : ([`job] id, t, [< `Many | `One | `Zero > `One `Zero ]) Caqti_request.t - val get_latest_uuid : - ([`job] id, [`build] id * Uuidm.t, [< `Many | `One | `Zero > `One `Zero ]) - Caqti_request.t val get_latest_successful_uuid : ([`job] id, Uuidm.t, [< `Many | `One | `Zero > `One `Zero ]) Caqti_request.t @@ -184,10 +146,6 @@ sig end module User : sig - val migrate : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val rollback : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t val get_user : (string, [`user] id * Builder_web_auth.scrypt Builder_web_auth.user_info, [< `Many | `One | `Zero > `One `Zero ]) @@ -197,7 +155,6 @@ module User : sig val add : (Builder_web_auth.scrypt Builder_web_auth.user_info, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val remove : ([`user] id, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t val remove_user : (string, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t val update_user : @@ -206,10 +163,6 @@ module User : sig end module Access_list : sig - val migrate : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t - val rollback : - (unit, unit, [< `Many | `One | `Zero > `Zero ]) Caqti_request.t val get : ([`user] id * [`job] id, [`access_list] id, [< `Many | `One | `Zero > `One ]) Caqti_request.t val add : diff --git a/lib/builder_web.ml b/lib/builder_web.ml index 9e61726..4d27caa 100644 --- a/lib/builder_web.ml +++ b/lib/builder_web.ml @@ -96,7 +96,7 @@ let add_routes datadir = List.fold_right (fun (job_id, job_name, section, synopsis) r -> r >>= fun acc -> - Dream.sql req (Model.build_meta job_id) >>= function + Dream.sql req (Model.build_with_main_binary job_id) >>= function | Some (latest_build, latest_artifact) -> let v = (job_name, synopsis, latest_build, latest_artifact) in let section = Option.value ~default:"Failed" section in diff --git a/lib/model.ml b/lib/model.ml index 6ea37e4..fbe04cb 100644 --- a/lib/model.ml +++ b/lib/model.ml @@ -54,9 +54,9 @@ let build uuid (module Db : CONN) = Db.find_opt Builder_db.Build.get_by_uuid uuid >>= not_found -let build_meta job (module Db : CONN) = +let build_with_main_binary job (module Db : CONN) = Db.find_opt Builder_db.Build.get_latest job >|= - Option.map (fun (_id, meta, file) -> (meta, file)) + Option.map (fun (_id, build, file) -> (build, file)) let build_hash hash (module Db : CONN) = Db.find_opt Builder_db.Build.get_with_jobname_by_hash hash @@ -65,11 +65,6 @@ let build_exists uuid (module Db : CONN) = Db.find_opt Builder_db.Build.get_by_uuid uuid >|= Option.is_some -let latest_build_uuid job_id (module Db : CONN) = - Db.find_opt Builder_db.Build.get_latest_uuid job_id >>= - (* We know there's at least one job when this is called, probably. *) - not_found >|= snd - let latest_successful_build_uuid job_id (module Db : CONN) = Db.find_opt Builder_db.Build.get_latest_successful_uuid job_id @@ -134,9 +129,6 @@ let job_and_readme job (module Db : CONN) = let builds = match x with None -> builds | Some f -> (f, None) :: builds in readme, List.rev builds -let jobs (module Db : CONN) = - Db.collect_list Builder_db.Job.get_all () - let jobs_with_section_synopsis (module Db : CONN) = Db.collect_list Builder_db.Job.get_all_with_section_synopsis () diff --git a/lib/model.mli b/lib/model.mli index f3ccc83..277991b 100644 --- a/lib/model.mli +++ b/lib/model.mli @@ -24,7 +24,7 @@ val build_artifacts : [`build] Builder_db.id -> Caqti_lwt.connection -> val build : Uuidm.t -> Caqti_lwt.connection -> ([`build] Builder_db.id * Builder_db.Build.t, [> error ]) result Lwt.t -val build_meta : [`job] Builder_db.id -> Caqti_lwt.connection -> +val build_with_main_binary : [`job] Builder_db.id -> Caqti_lwt.connection -> ((Builder_db.Build.t * Builder_db.file option) option, [> Caqti_error.call_or_retrieve ]) result Lwt.t val build_hash : Cstruct.t -> Caqti_lwt.connection -> @@ -33,9 +33,6 @@ val build_hash : Cstruct.t -> Caqti_lwt.connection -> val build_exists : Uuidm.t -> Caqti_lwt.connection -> (bool, [> Caqti_error.call_or_retrieve ]) result Lwt.t -val latest_build_uuid : [`job] Builder_db.id -> Caqti_lwt.connection -> - (Uuidm.t, [> error ]) result Lwt.t - val latest_successful_build_uuid : [`job] Builder_db.id -> Caqti_lwt.connection -> (Uuidm.t option, [> Caqti_error.call_or_retrieve ]) result Lwt.t @@ -66,9 +63,6 @@ val job_and_readme : string -> Caqti_lwt.connection -> val job_id : string -> Caqti_lwt.connection -> ([`job] Builder_db.id option, [> Caqti_error.call_or_retrieve ]) result Lwt.t -val jobs : Caqti_lwt.connection -> - (([`job] Builder_db.id * string) list, [> Caqti_error.call_or_retrieve ]) result Lwt.t - val jobs_with_section_synopsis : Caqti_lwt.connection -> (([`job] Builder_db.id * string * string option * string option) list, [> Caqti_error.call_or_retrieve ]) result Lwt.t diff --git a/test/builder_db.ml b/test/builder_db.ml index ddb447d..9748d62 100644 --- a/test/builder_db.ml +++ b/test/builder_db.ml @@ -108,16 +108,6 @@ let test_user_update (module Db : CONN) = let auth_opt = Option.map snd res in Alcotest.(check (option Testable.builder_web_auth)) "update user" auth_opt (Some auth') -let test_user_remove (module Db : CONN) = - Db.find_opt Builder_db.User.get_user username >>= function - | None -> - Alcotest.fail "user not found" - | Some (id, _auth') -> - Db.exec Builder_db.User.remove id >>= fun () -> - Db.find_opt Builder_db.User.get_user username >>| fun res -> - let auth_opt = Option.map snd res in - Alcotest.(check (option Testable.builder_web_auth)) "remove user" auth_opt None - let test_user_auth (module Db : CONN) = Db.find_opt Builder_db.User.get_user username >>| function | None -> @@ -177,10 +167,6 @@ let with_build_db f () = add_test_build user_id conn >>= fun () -> f conn) -let test_job_get_all (module Db : CONN) = - Db.collect_list Builder_db.Job.get_all () >>| fun jobs -> - Alcotest.(check int) "one job" (List.length jobs) 1 - let test_job_get_id_by_name (module Db : CONN) = Db.find_opt Builder_db.Job.get_id_by_name job_name >>= fail_if_none >>| fun _id -> () @@ -196,8 +182,9 @@ let test_job_remove () = Db.exec Builder_db.Job.try_add "test-job" >>= fun () -> Db.find_opt Builder_db.Job.get_id_by_name "test-job" >>= fail_if_none >>= fun id -> Db.exec Builder_db.Job.remove id >>= fun () -> - Db.collect_list Builder_db.Job.get_all () >>| fun jobs -> - Alcotest.(check int) "no jobs" (List.length jobs) 0 + match Db.find Builder_db.Job.get id with + | Error #Caqti_error.call_or_retrieve -> Ok () + | Ok _ -> Alcotest.fail "expected no job" in or_fail r @@ -211,11 +198,6 @@ let test_build_get_all (module Db : CONN) = Db.collect_list Builder_db.Build.get_all job_id >>| fun builds -> Alcotest.(check int) "one build" (List.length builds) 1 -let test_build_get_all_with_main_binary (module Db : CONN) = - Db.find_opt Builder_db.Job.get_id_by_name job_name >>= fail_if_none >>= fun job_id -> - Db.collect_list Builder_db.Build.get_all_with_main_binary job_id >>| fun builds -> - Alcotest.(check int) "one build" (List.length builds) 1 - let uuid' = Uuidm.create `V4 let start' = Option.get (Ptime.of_float_s 3600.) let finish' = Option.get (Ptime.of_float_s 3601.) @@ -243,14 +225,6 @@ let test_build_get_latest (module Db : CONN) = Alcotest.(check (option Testable.file)) "same main binary" main_binary' (Some main_binary); Alcotest.(check Testable.uuid) "same uuid" meta.uuid uuid' -let test_build_get_latest_uuid (module Db : CONN) = - add_second_build (module Db) >>= fun () -> - (* Test *) - Db.find_opt Builder_db.Job.get_id_by_name job_name >>= fail_if_none >>= fun job_id -> - Db.find_opt Builder_db.Build.get_latest_uuid job_id - >>| get_opt "no latest build" >>| fun (_id, latest_uuid) -> - Alcotest.(check Testable.uuid) "same uuid" latest_uuid uuid' - let test_build_get_previous (module Db : CONN) = add_second_build (module Db) >>= fun () -> Db.find_opt Builder_db.Build.get_by_uuid uuid' @@ -286,13 +260,6 @@ let test_artifact_get_by_build_uuid (module Db : CONN) = get_opt "no build" >>| fun (_id, file) -> Alcotest.(check Testable.file) "same file" file main_binary -let test_artifact_get_by_build (module Db : CONN) = - Db.find_opt Builder_db.Build.get_by_uuid uuid >>| - get_opt "no build" >>= fun (id, _build) -> - Db.find Builder_db.Build_artifact.get_by_build - (id, main_binary.filepath)>>| fun (_id, file) -> - Alcotest.(check Testable.file) "same file" file main_binary - (* XXX: This test should fail because main_binary on the corresponding build * references its main_binary. This is not the case now due to foreign key. *) let test_artifact_remove_by_build (module Db : CONN) = @@ -308,7 +275,6 @@ let () = test_case "Get user" `Quick (with_user_db test_user_get_user); test_case "Remove user by name" `Quick (with_user_db test_user_remove_user); test_case "Update user" `Quick (with_user_db test_user_update); - test_case "Remove user" `Quick (with_user_db test_user_remove); ]; "user-auth", [ test_case "User auth success" `Quick (with_user_db test_user_auth); @@ -316,7 +282,6 @@ let () = ]; "job", [ test_case "Add build" `Quick (with_build_db (fun _ -> Ok ())); - test_case "One job" `Quick (with_build_db test_job_get_all); test_case "Get job id" `Quick (with_build_db test_job_get_id_by_name); test_case "Get job" `Quick (with_build_db test_job_get); test_case "Remove job" `Quick test_job_remove; @@ -324,9 +289,7 @@ let () = "build", [ test_case "Get build" `Quick (with_build_db test_build_get_by_uuid); test_case "One build" `Quick (with_build_db test_build_get_all); - test_case "One build (meta data)" `Quick (with_build_db test_build_get_all_with_main_binary); test_case "Get latest build" `Quick (with_build_db test_build_get_latest); - test_case "Get latest build uuid" `Quick (with_build_db test_build_get_latest_uuid); test_case "Get build by hash" `Quick (with_build_db test_build_get_with_jobname_by_hash); test_case "Get previous build" `Quick (with_build_db test_build_get_previous); test_case "Get previous build when first" `Quick (with_build_db test_build_get_previous_none); @@ -334,7 +297,6 @@ let () = "build-artifact", [ test_case "Get all by build" `Quick (with_build_db test_artifact_get_all_by_build); test_case "Get by build uuid" `Quick (with_build_db test_artifact_get_by_build_uuid); - test_case "Get by build" `Quick (with_build_db test_artifact_get_by_build); test_case "Remove by build" `Quick (with_build_db test_artifact_remove_by_build); ]; ]