JiraAPI Groovy Script conversion

[TheMeinerLP]

In this PR I have converted the JiraAPI script into a Java class. I have reduced code and implemented better error handling.
API and implementation are separated and can be called via Singelton.

Overall, this PR should work simply because the classes and calls have the same names

[NotMyFault]

SpotBugs is not fine with that.

[TheMeinerLP]

I will handle that

[timja]

As a translation this seems ok.

Its not normal Java code, there's no reason that I can see for the abstract class.
It should be just one class in this instance.

It would be good to replace the verbose httpurlconnection with the HttpClient added in Java 11 but that should be looked at after a translation

[gounthar]

And it now builds! 👏

[timja]

Can you please remove all tests that are checking log messages? Test code not the contents of logging.

so e.g. isUserPresent returns true or false based on different responses from Jira using something like wiremock.

[TheMeinerLP]

I would like to keep the tests to simply check intended behaviour. There is no point in just checking return values.

[timja]

I would like to keep the tests to simply check intended behaviour. There is no point in just checking return values.

You could refactor to result types, e.g. an enum for some of the validation which would result in cleaner code.

[TheMeinerLP]

The goal of this PR is the conversion from groovy to java and improvement of the code in minimal.
And this is related to GSoC

[timja]

The goal of this PR is the conversion from groovy to java and improvement of the code in minimal. And this is related to GSoC

I understand that but the tests that you've added are fragile and in my opinion its not a great practice to test specific log messages unless they are an external interface to another system.

[TheMeinerLP]

In what sense fragile ?
Because if I now add Enum and more, that goes against the goal of what the PR should do and what is still planned

[TheMeinerLP]

@timja Based on Alex's feedback I will add enum classes for the return values and use optionals to better catch the errors and add tests. Would you be happy with that?

[timja]

@timja Based on Alex's feedback I will add enum classes for the return values and use optionals to better catch the errors and add tests. Would you be happy with that?

Looking at the code, there's one 'validation' error for the username regex.
The rest look like exceptions.
None of them are actually anything to do with the user being present in Jira and reporting false isn't correct.

If you return an exception instead that would be better and clearer.

I don't know where returning optionals would be useful.

[TheMeinerLP]

Okay, understood, I'll implement it that way.

[timja]

Thanks!