fix "FATAL: Null value not allowed as an environment variable: gitlabUserEmail"

[runsisi]

as in gitlab-ce-8.3.0-ce, when we try to get info of opened MRs, gitlab will not return us the email address of the author:

curl http://hust/api/v3/projects/1/merge_requests?state=opened\&private_token=CiQcZ9TLJ3qZD6sy-Ftv

[{"id":6,"iid":6,"project_id":1,"title":"mr3.1","description":"mr3.1",
...
"author":{
"name":"luo.runbing",
"username":"luo.runbing",
"id":2,
"state":"active",
"avatar_url":"http://www.gravatar.com/avatar/3c0c2978976bf9b11c965017aec497a9?s=80\u0026d=identicon",
"web_url":"http://hust/u/luo.runbing"},
...}]

which results error like this:

FATAL: Null value not allowed as an environment variable: gitlabUserEmail

Signed-off-by: runsisi [email protected]

[jenkinsadmin]

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

[WonderCsabo]

I can verify this PR is need and works.

[coder-hugo]

The null check should be move to the location where the getters getName and getEmail of the author are accessed. This change will cause that also the environment variable gitlabUserName isn't set although just the email of the author is null.

[markus-mnm]

I agree with coder-hugo's comment.
This approach worked for me (I could'nt test the other PR without this):

diff --git a/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java b/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java
index 1ef50ad..3a27ace 100644
--- a/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java
+++ b/src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java
@@ -396,7 +396,10 @@
         values.put("gitlabActionType", new StringParameterValue("gitlabActionType", "MERGE"));
         if (req.getObjectAttribute().getAuthor() != null) {
            values.put("gitlabUserName", new StringParameterValue("gitlabUserName", req.getObjectAttribute().getAuthor().getName()));
-            values.put("gitlabUserEmail", new StringParameterValue("gitlabUserEmail", req.getObjectAttribute().getAuthor().getEmail()));
+            String email = req.getObjectAttribute().getAuthor().getEmail();
+            if (email!=null) {
+                values.put("gitlabUserEmail", new StringParameterValue("gitlabUserEmail", email));
+            }
         }
         values.put("gitlabMergeRequestTitle", new StringParameterValue("gitlabMergeRequestTitle",  req.getObjectAttribute().getTitle()));
         values.put("gitlabMergeRequestId", new StringParameterValue("gitlabMergeRequestId", req.getObjectAttribute().getIid().toString()));

[demaniak]

So... any eta on this making it's way downstream? I got people heavily invested and depending on a jenkins-gitlab pipeline, and this is sorta...well...breaking us.

[testark]

We're also needing this. Is there any chance this can get merged soon?

[omehegan]

@WonderCsabo can you respond to @coder-hugo's comment? I think that's what we're waiting on.

[WonderCsabo]

@omehegan i am a little bit confused now. I did not get any comment from @coder-hugo . If you are referring to #170 (comment), my reimplementation of this PR (#216), was already created with that comment in mind, actually my PR is the same as this #170 (comment) snippet.

[mfriedenhagen]

@WonderCsabo, so the way to go would be to:

• drop fix "FATAL: Null value not allowed as an environment variable: gitlabUserEmail" #170 and
• pull Fix NPE when author email is not available #216 instead

[WonderCsabo]

Exactly.

[omehegan]

@WonderCsabo sorry, you're right, the original comment was for @runsisi. But it sounds like we should reject this in favor of PR #216. @coder-hugo agree?