Fix issue 9947, use isEmpty() for checking emptiness

[RohitBhojak]

What does this PR change?

Refactored Java code to comply with SonarCloud rule S1155:
Replaced instances of collection.size() < 1 with collection.isEmpty(), and collection.size() >= 1 with !collection.isEmpty().

GUI diff

No difference.

• DONE

Documentation

• No documentation needed: only internal and user-invisible changes

• DONE

Test coverage

• No tests done: just fixing sonar cloud issues

• DONE

Links

Issue(s): #9947

• DONE

Changelogs

• No changelog needed (If maintainers agree that this change is minor and does not require a changelog entry)

Re-run a test

If you need to re-run a test, please mark the related checkbox, it will be unchecked automatically once it has re-run:

• Re-run test "changelog_test"
• Re-run test "backend_unittests_pgsql"
• Re-run test "java_pgsql_tests"
• Re-run test "schema_migration_test_pgsql"
• Re-run test "susemanager_unittests"
• Re-run test "javascript_lint"
• Re-run test "spacecmd_unittests"

Before you merge

Check How to branch and merge properly!

[github-actions]

👋 Hello! Thanks for contributing to our project.
Acceptance tests will take some time (aprox. 1h), please be patient ☕

You can see the progress at the end of this page and at https://github.com/uyuni-project/uyuni/pull/9981/checks
Once tests finish, if they fail, you can check 👀 the cucumber report. See the link at the output of the action.
You can also check the artifacts section, which contains the logs at https://github.com/uyuni-project/uyuni/pull/9981/checks.

If you are unsure the failing tests are related to your code, you can check the "reference jobs". These are jobs that run on a scheduled time with code from master. If they fail for the same reason as your build, it means the tests or the infrastructure are broken. If they do not fail, but yours do, it means it is related to your code.

Reference tests:

• https://github.com/uyuni-project/uyuni/actions/workflows/acceptance_tests_secondary_parallel.yml?query=event%3Aschedule

• https://github.com/uyuni-project/uyuni/actions/workflows/acceptance_tests_secondary.yml?query=event%3Aschedule

KNOWN ISSUES

Sometimes the build can fail when pulling new jar files from download.opensuse.org . This is a known limitation. Given this happens rarely, when it does, all you need to do is rerun the test. Sorry for the inconvenience.

For more tips on troubleshooting, see the troubleshooting guide.

Happy hacking!
⚠️ You should not merge if acceptance tests fail to pass. ⚠️

[CDellaGiusta]

The text of the PR is twisted:

• GUI DIFF: remove before and after
• FALSE: No tests needed: already covered by existing tests.
• LINKS: in the original text you are requested to give only reference to the issue and backports (other things are not to be added)
• CHANGELOGS: why "No changelog needed"? You do need a changelog here. Please look at the rules to make one and do it. Then remove all other comments: that is not the place where they are requested

[RohitBhojak]

@CDellaGiusta Thank you for your feedback, I am new to open sourcing and appreciate the guidance. I still have some questions:

• Should I change all the instances of .isEmpty() with CollectionUtils.isEmpty()? Even the ones that I didn't change in my commit?
• What should I put in the test section?
• Does the changelog have to be specific or will it do with

- Fixed SonarCloud Rule S1155 violations (gh#uyuni-project/uyuni#9947)  
  * Used CollectionUtils.isEmpty() for checking emptiness
  * Improved code readability and maintainability

[CDellaGiusta]

* Should I change all the instances of .isEmpty() with CollectionUtils.isEmpty()? Even the ones that I didn't change in my commit?

No. I had a quick chat with some colleagues, and the substitutions you did in the code are ok as you did it.

* What should I put in the test section?

Something like: No tests done: just fixing sonar cloud issues
That is more correct as a sentence

- Fixed SonarCloud Rule S1155 violations (gh#uyuni-project/uyuni#9947)  
  * Used CollectionUtils.isEmpty() for checking emptiness
  * Improved code readability and maintainability

Just the first line is ok, something like "- Fixed SonarCloud Rule S1155 violations", how you did it is in the code, and "Improved code readability and maintainability" is not worth mentioning

Please correct the changelog file, and add it to the commit (squashing it in 1 commit only)
Then correct the PR text:
mention only the link to the issue, since you don't have backports, remove the SonarCloud Rule reference and the "All issues in Uyuni for this rule" reference

Then wait for some other reviewers to approve.

[cbosdo]

Your code changes are fine for me. However I would like you to copy the PR description as the commit message:

Refactored Java code to comply with SonarCloud rule S1155:
Replaced instances of collection.size() < 1 with collection.isEmpty(), and collection.size() >= 1 with !collection.isEmpty().

Github may go away, git history will last for ever!

[RohitBhojak]

@CDellaGiusta I have made the changes that you had suggested, I used osc vc to write the changelog, but the changelog check failed. Is it alright? Kindly review this PR.

[CDellaGiusta]

@CDellaGiusta I have made the changes that you had suggested, I used osc vc to write the changelog, but the changelog check failed. Is it alright? Kindly review this PR.

The check fails because you have to name the changelog correctly.
In the java directory, please create a changelog file in the following format:

<package_name>.changes.<username>.<feature_name>

If not explicitly specified, fetch username and feature_name from git email's
username part and the current branch name.

e.g. spacewalk-java.changes.carlo.fix-api-namespace

Please note that you have to make your own branch (you can't be on master), I guess yours is fix-9947
Your file should contain only these 2 lines:

• Fixed SonarCloud Rule S1155 violations
  (gh#SonarCloud error reduction: java rule S1155: "Collection.isEmpty()" should be used to test for emptiness #9947)

please be aware that the max lenght is 65 and that any must then be aligned on the following line.
The rest of the code looks good, so when all tests pass you'll be ready to get approved.

[mcalmer]

https://github.com/uyuni-project/uyuni/wiki/Contributing#submitting-to-git
And the next section explain the changelog rules.
To create the filename correctly you can use mkchlog tool. The link to the tool is in
https://github.com/uyuni-project/uyuni/wiki/Contributing#changelog-rules-for-packages-in-git

[cbosdo]

Such a PR does not need a changelog entry. Remember that a changelog entry is something read by the end user. I would rather remove the changelog change from that commit and simply click the No changelog needed box in the PR description.

[CDellaGiusta]

Such a PR does not need a changelog entry. Remember that a changelog entry is something read by the end user. I would rather remove the changelog change from that commit and simply click the No changelog needed box in the PR description.

Ok, that's fair enough.

[cbbayburt]

Thanks for the PR, @RohitBhojak. The code looks good. As per the comments above, just revert back spacewalk-java.changes and you're good to go.