Tweak json field names

Also add "main_binary" boolean field in comparison for consistency with
`job_build` json.

builder-web.opam

@@ -50,10 +50,10 @@ depends: [
   "decompress" {>= "1.5.0"}
   "digestif" {>= "1.2.0"}
   "uuidm" {>= "0.9.9"}
+  "yojson"
   "alcotest" {>= "1.2.0" & with-test}
   "ppx_deriving" {with-test}
   "ppx_deriving_yojson" {with-test}
-  "yojson" {with-test}
 ]
 
 synopsis: "Web interface for builder"

lib/builder_web.ml

@@ -330,18 +330,9 @@ let routes ~datadir ~cachedir ~configdir ~expired_jobs =
      Dream.sql req (Model.latest_successful_build_uuid job_id platform))
     >>= Model.not_found
     |> if_error "Error getting job" >>= fun build ->
-      match Dream.header req "Accept" with
+    Dream.redirect req
-      | Some accept when String.starts_with ~prefix:"application/json" accept ->
+      (Link.Job_build_artifact.make_from_string ~job_name ~build ~artifact ())
-        let json_response =
+    |> Lwt_result.ok
-          `Assoc [
-            "uuid", `String (Uuidm.to_string build);
-          ] |> Yojson.Basic.to_string
-        in
-        Dream.json ~status:`OK json_response |> Lwt_result.ok
-      | _ ->
-        Dream.redirect req
-          (Link.Job_build_artifact.make_from_string ~job_name ~build ~artifact ())
-        |> Lwt_result.ok
   in
 
   let redirect_latest req =
@@ -413,8 +404,8 @@ let routes ~datadir ~cachedir ~configdir ~expired_jobs =
           "job_name", `String job_name;
           "uuid", `String (Uuidm.to_string build.uuid);
           "platform", `String build.platform;
-          "build_start_time", `String (Ptime.to_rfc3339 build.start);
+          "start_time", `String (Ptime.to_rfc3339 build.start);
-          "build_finish_time", `String (Ptime.to_rfc3339 build.finish);
+          "finish_time", `String (Ptime.to_rfc3339 build.finish);
           "main_binary", (match build.main_binary with Some _ -> `Bool true | None ->  `Bool false)
         ] |> Yojson.Basic.to_string
       in
@@ -561,15 +552,12 @@ let routes ~datadir ~cachedir ~configdir ~expired_jobs =
   in
 
 
-  let resolve_artifact id_opt conn =
+  let resolve_artifact_size id_opt conn =
-    let default_file : Builder_db.file = {
-      filepath = Fpath.v "null";
-      sha256 = "0";
-      size = -1;
-    } in
     match id_opt with
-    | None -> Lwt.return_ok default_file
+    | None -> Lwt.return_ok None
-    | Some id -> Model.build_artifact_by_id id conn
+    | Some id ->
+      Model.build_artifact_by_id id conn >|= fun file ->
+      Some file.size
 
   in
 
@@ -593,16 +581,16 @@ let routes ~datadir ~cachedir ~configdir ~expired_jobs =
         Model.build_artifact_data datadir >>= fun build_env_right ->
         Model.build_artifact build_right.Builder_db.Build.uuid (Fpath.v "system-packages") conn >>=
         Model.build_artifact_data datadir >>= fun system_packages_right ->
-        resolve_artifact build_left.Builder_db.Build.main_binary conn >>= fun build_left_file ->
+        resolve_artifact_size build_left.Builder_db.Build.main_binary conn >>= fun build_left_file_size ->
-        resolve_artifact build_right.Builder_db.Build.main_binary conn >>= fun build_right_file ->
+        resolve_artifact_size build_right.Builder_db.Build.main_binary conn >>= fun build_right_file_size ->
         Model.job_name build_left.job_id conn >>= fun job_left ->
         Model.job_name build_right.job_id conn >|= fun job_right ->
-        (job_left, job_right, build_left, build_right, build_left_file,
+        (job_left, job_right, build_left, build_right, build_left_file_size,
-         build_right_file, switch_left, build_env_left, system_packages_left,
+         build_right_file_size, switch_left, build_env_left, system_packages_left,
          switch_right, build_env_right, system_packages_right))
     |> if_error "Internal server error"
-    >>= fun (job_left, job_right, build_left, build_right, build_left_file,
+    >>= fun (job_left, job_right, build_left, build_right, build_left_file_size,
-              build_right_file, switch_left, build_env_left, system_packages_left,
+              build_right_file_size, switch_left, build_env_left, system_packages_left,
              switch_right, build_env_right, system_packages_right) ->
     let env_diff = Utils.compare_env build_env_left build_env_right
     and pkg_diff = Utils.compare_pkgs system_packages_left system_packages_right
@@ -610,33 +598,36 @@ let routes ~datadir ~cachedir ~configdir ~expired_jobs =
     let switch_left = OpamFile.SwitchExport.read_from_string switch_left
     and switch_right = OpamFile.SwitchExport.read_from_string switch_right in
     let opam_diff = Opamdiff.compare switch_left switch_right in
-    (job_left, job_right, build_left, build_right, build_left_file, build_right_file, env_diff, pkg_diff, opam_diff)
+    (job_left, job_right, build_left, build_right, build_left_file_size, build_right_file_size, env_diff, pkg_diff, opam_diff)
     |> Lwt.return_ok
 
   in
 
   let compare_builds req =
     process_comparison req >>= fun
-    (job_left, job_right, build_left, build_right, build_left_file, build_right_file, env_diff, pkg_diff, opam_diff) ->
+    (job_left, job_right, build_left, build_right, build_left_file_size, build_right_file_size, env_diff, pkg_diff, opam_diff) ->
       match Dream.header req "Accept" with
         | Some accept when String.starts_with ~prefix:"application/json" accept ->
+          let file_size_json = Option.fold ~none:`Null ~some:(fun size -> `Int size) in
           let json_response =
             `Assoc [
               "left", `Assoc [
-                "job", `String job_left;
+                "job_name", `String job_left;
-                "build", `String (Uuidm.to_string build_left.uuid);
+                "uuid", `String (Uuidm.to_string build_left.uuid);
                 "platform", `String build_left.platform;
-                "build_start_time", `String (Ptime.to_rfc3339 build_left.start);
+                "start_time", `String (Ptime.to_rfc3339 build_left.start);
-                "build_finish_time", `String (Ptime.to_rfc3339 build_left.finish);
+                "finish_time", `String (Ptime.to_rfc3339 build_left.finish);
-                "build_size", `Int (build_left_file.size)
+                "main_binary", `Bool (Option.is_some build_left_file_size);
+                "main_binary_size", file_size_json build_left_file_size;
               ];
               "right", `Assoc [
-                "job", `String job_right;
+                "job_name", `String job_right;
                 "build", `String (Uuidm.to_string build_right.uuid);
                 "platform", `String build_right.platform;
-                "build_start_time", `String (Ptime.to_rfc3339 build_right.start);
+                "start_time", `String (Ptime.to_rfc3339 build_right.start);
-                "build_finish_time", `String (Ptime.to_rfc3339 build_right.finish);
+                "finish_time", `String (Ptime.to_rfc3339 build_right.finish);
-                "build_size", `Int (build_right_file.size)
+                "main_binary", `Bool (Option.is_some build_right_file_size);
+                "main_binary_size", file_size_json build_right_file_size;
               ];
               "env_diff", Utils.diff_map_to_json env_diff;
               "package_diff", Utils.diff_map_to_json pkg_diff;

opamdiff/dune

@@ -1,3 +1,3 @@
 (library
  (name opamdiff)
- (libraries opam-core opam-format))
+ (libraries opam-core opam-format yojson))