Skip to content

Commit 5618a48

Browse files
yasunariwyasunariw
committed
edit context structure and define getters for secrets+config
Secrets are now in their own record field within the context, so that future secret additions don’t pollute the outer context structure. Getters are also defined to reduce boilerplate exception handling.
1 parent e734c2e commit 5618a48

File tree

2 files changed

+75
-71
lines changed

2 files changed

+75
-71
lines changed

‎lib/action.ml‎

Lines changed: 52 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -88,60 +88,54 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
8888
if List.is_empty matched_channel_names then default else matched_channel_names
8989

9090
let partition_status (ctx : Context.t) (n : status_notification) =
91-
match ctx.config with
92-
| None -> action_error "missing configuration file"
93-
| Some cfg ->
94-
let pipeline = n.context in
95-
let current_status = n.state in
96-
let updated_at = n.updated_at in
97-
let get_commit_info () =
98-
let default = Option.to_list cfg.prefix_rules.default_channel in
99-
let () =
100-
Context.refresh_pipeline_status ~pipeline ~branches:n.branches ~status:current_status ~updated_at ctx
101-
in
102-
match List.is_empty n.branches with
103-
| true -> Lwt.return []
104-
| false ->
105-
match cfg.main_branch_name with
106-
| None -> Lwt.return default
107-
| Some main_branch_name ->
108-
(* non-main branch build notifications go to default channel to reduce spam in topic channels *)
109-
match List.exists n.branches ~f:(fun { name } -> String.equal name main_branch_name) with
110-
| false -> Lwt.return default
111-
| true ->
112-
let sha = n.commit.sha in
113-
let repo = n.repository in
114-
( match%lwt Github_api.get_api_commit ~ctx ~repo ~sha with
115-
| Error e -> action_error e
116-
| Ok commit -> Lwt.return @@ partition_commit cfg commit.files
117-
)
118-
in
119-
let res =
120-
match
121-
List.exists cfg.status_rules.status ~f:(fun x ->
122-
match x with
123-
| State s -> Poly.equal s n.state
124-
| HideConsecutiveSuccess -> Poly.equal Success n.state
125-
| _ -> false)
126-
with
127-
| false -> Lwt.return []
128-
| true ->
129-
match List.exists ~f:id [ Rule.Status.hide_cancelled n cfg; Rule.Status.hide_success n ctx ] with
130-
| true -> Lwt.return []
131-
| false ->
132-
match cfg.status_rules.title with
133-
| None -> get_commit_info ()
134-
| Some status_filter ->
135-
match List.exists status_filter ~f:(String.equal n.context) with
136-
| false -> Lwt.return []
137-
| true -> get_commit_info ()
138-
in
139-
res
91+
let cfg = Context.get_config_exn ctx in
92+
let pipeline = n.context in
93+
let current_status = n.state in
94+
let updated_at = n.updated_at in
95+
let get_commit_info () =
96+
let default = Option.to_list cfg.prefix_rules.default_channel in
97+
let () = Context.refresh_pipeline_status ~pipeline ~branches:n.branches ~status:current_status ~updated_at ctx in
98+
match List.is_empty n.branches with
99+
| true -> Lwt.return []
100+
| false ->
101+
match cfg.main_branch_name with
102+
| None -> Lwt.return default
103+
| Some main_branch_name ->
104+
(* non-main branch build notifications go to default channel to reduce spam in topic channels *)
105+
match List.exists n.branches ~f:(fun { name } -> String.equal name main_branch_name) with
106+
| false -> Lwt.return default
107+
| true ->
108+
let sha = n.commit.sha in
109+
let repo = n.repository in
110+
( match%lwt Github_api.get_api_commit ~ctx ~repo ~sha with
111+
| Error e -> action_error e
112+
| Ok commit -> Lwt.return @@ partition_commit cfg commit.files
113+
)
114+
in
115+
let res =
116+
match
117+
List.exists cfg.status_rules.status ~f:(fun x ->
118+
match x with
119+
| State s -> Poly.equal s n.state
120+
| HideConsecutiveSuccess -> Poly.equal Success n.state
121+
| _ -> false)
122+
with
123+
| false -> Lwt.return []
124+
| true ->
125+
match List.exists ~f:id [ Rule.Status.hide_cancelled n cfg; Rule.Status.hide_success n ctx ] with
126+
| true -> Lwt.return []
127+
| false ->
128+
match cfg.status_rules.title with
129+
| None -> get_commit_info ()
130+
| Some status_filter ->
131+
match List.exists status_filter ~f:(String.equal n.context) with
132+
| false -> Lwt.return []
133+
| true -> get_commit_info ()
134+
in
135+
res
140136

141137
let partition_commit_comment (ctx : Context.t) n =
142-
match ctx.config with
143-
| None -> action_error "missing configuration file"
144-
| Some cfg ->
138+
let cfg = Context.get_config_exn ctx in
145139
match n.comment.commit_id with
146140
| None -> action_error "unable to find commit id for this commit comment event"
147141
| Some sha ->
@@ -160,9 +154,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
160154
)
161155

162156
let generate_notifications (ctx : Context.t) req =
163-
match ctx.config with
164-
| None -> action_error "missing configuration file"
165-
| Some cfg ->
157+
let cfg = Context.get_config_exn ctx in
166158
match req with
167159
| Github.Push n ->
168160
partition_push cfg n |> List.map ~f:(fun (webhook, n) -> webhook, generate_push_notification n) |> Lwt.return
@@ -231,7 +223,8 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
231223

232224
let process_github_notification (ctx : Context.t) headers body =
233225
try%lwt
234-
match Github.parse_exn ~secret:ctx.gh_hook_token headers body with
226+
let secrets = Context.get_secrets_exn ctx in
227+
match Github.parse_exn ~secret:secrets.gh_hook_token headers body with
235228
| exception exn -> Exn_lwt.fail ~exn "failed to parse payload"
236229
| payload ->
237230
( match%lwt refresh_config_of_context ctx payload with
@@ -251,4 +244,7 @@ module Action (Github_api : Api.Github) (Slack_api : Api.Slack) = struct
251244
| Action_error msg ->
252245
log#error "%s" msg;
253246
Lwt.return_unit
247+
| Context.Context_error msg ->
248+
log#error "%s" msg;
249+
Lwt.return_unit
254250
end

‎lib/context.ml‎

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ open Base
22
open Common
33
open Devkit
44

5+
exception Context_error of string
6+
7+
let context_error msg = raise (Context_error msg)
8+
59
type t = {
610
config_filename : string;
711
secrets_filepath : string;
812
state_filepath : string option;
9-
gh_token : string option;
10-
gh_hook_token : string option;
11-
slack_hooks : Slack.channel_hook StringMap.t;
13+
mutable secrets : Config_t.secrets option;
1214
mutable config : Config.t option;
1315
mutable state : State_t.state;
1416
}
@@ -18,9 +20,7 @@ let default : t =
1820
config_filename = "notabot.json";
1921
secrets_filepath = "secrets.json";
2022
state_filepath = None;
21-
gh_token = None;
22-
gh_hook_token = None;
23-
slack_hooks = StringMap.empty;
23+
secrets = None;
2424
config = None;
2525
state = State.empty;
2626
}
@@ -30,7 +30,21 @@ let make ?config_filename ?secrets_filepath ?state_filepath () =
3030
let secrets_filepath = Option.value secrets_filepath ~default:default.secrets_filepath in
3131
{ default with config_filename; secrets_filepath; state_filepath }
3232

33-
let hook_of_channel ctx channel_name = Map.find ctx.slack_hooks channel_name
33+
let get_secrets_exn ctx =
34+
match ctx.secrets with
35+
| None -> context_error "secrets is uninitialized"
36+
| Some secrets -> secrets
37+
38+
let get_config_exn ctx =
39+
match ctx.config with
40+
| None -> context_error "config is uninitialized"
41+
| Some config -> config
42+
43+
let hook_of_channel ctx channel_name =
44+
let secrets = get_secrets_exn ctx in
45+
match List.find secrets.slack_hooks ~f:(fun webhook -> String.equal webhook.channel channel_name) with
46+
| Some hook -> Some hook.url
47+
| None -> None
3448

3549
let refresh_pipeline_status ctx ~pipeline ~(branches : Github_t.branch list) ~status ~updated_at =
3650
ctx.state <- State.refresh_pipeline_status ctx.state ~pipeline ~branches ~status ~updated_at
@@ -41,14 +55,8 @@ let refresh_secrets ctx =
4155
let path = ctx.secrets_filepath in
4256
match%lwt get_local_file path with
4357
| Ok res ->
44-
let secrets = Config_j.secrets_of_string res in
45-
let slack_hooks =
46-
List.fold_left secrets.slack_hooks ~init:StringMap.empty ~f:(fun m hook ->
47-
Map.set m ~key:hook.channel ~data:hook.url)
48-
in
49-
let gh_token = secrets.gh_token in
50-
let gh_hook_token = secrets.gh_hook_token in
51-
Lwt.return @@ Ok { ctx with slack_hooks; gh_token; gh_hook_token }
58+
ctx.secrets <- Some (Config_j.secrets_of_string res);
59+
Lwt.return @@ Ok ctx
5260
| Error e ->
5361
log#error "error while getting local file %s: %s" path e;
5462
Lwt.return @@ fmt_error "failed to get secrets from file %s" path

0 commit comments

Comments
 (0)