Skip to content

Code quality fixes#9348

Merged
josegar74 merged 2 commits into
geonetwork:mainfrom
GeoCat:fix/code-quality-issues
Jun 26, 2026
Merged

Code quality fixes#9348
josegar74 merged 2 commits into
geonetwork:mainfrom
GeoCat:fix/code-quality-issues

Conversation

@josegar74

@josegar74 josegar74 commented Jun 17, 2026

Copy link
Copy Markdown
Member

Includes the following code fixes:

  • String comparison using of == or != instead of equals().
  • Number equality using == or != instead of equals().
  • Expression compared to itself.
  • Long literal ending with lowercase 'l'.
  • Empty finally blocks.
  • Logging placeholders does not match number of arguments in logging call.
  • Serializable classes whose serialVersionUID field is not declared private static final long
  • Use Objects.equals to avoid NullPointerException in comparisons of fields annotated with @nonnull and no initializer
  • Update MetadataLink.hash to avoid NullPointerException

Checklist

  • I have read the contribution guidelines
  • Pull request provided for main branch, backports managed with label
  • Good housekeeping of code, cleaning up comments, tests, and documentation
  • Clean commit history broken into understandable chucks, avoiding big commits with hundreds of files, cautious of reformatting and whitespace changes
  • Clean commit messages, longer verbose messages are encouraged
  • API Changes are identified in commit messages
  • Testing provided for features or enhancements using automatic tests
  • User documentation provided for new features or enhancements in manual
  • Build documentation provided for development instructions in README.md files
  • Library management using pom.xml dependency management. Update build documentation with intended library use and library tutorials or documentation

@josegar74 josegar74 added this to the 4.4.12 milestone Jun 17, 2026
- String comparison using of == or != instead of equals().
- Number equality using == or != instead of equals().
- Expression compared to itself.
- Long literal ending with lowercase 'l'.
- Empty finally blocks.
- Logging placeholders does not match number of arguments in logging call.
- Serializable classes whose serialVersionUID field is not declared private static final long
@josegar74 josegar74 force-pushed the fix/code-quality-issues branch from 2154b58 to 31ba6e3 Compare June 17, 2026 07:52
if (link.getId() != other.link.getId())
return false;
if (metadataId.intValue() != metadataId.intValue())
if (metadataId.intValue() != other.metadataId.intValue())

@juanluisrp juanluisrp Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fixed equals() now calls both metadataId.intValue() and other.metadataId.intValue(), but metadataId is declared as boxed Integer with no @NotNull constraint. A MetadataLink constructed via the default constructor without calling setMetadataId() leaves the field null, so .intValue() throws NullPointerException.

Consider using Objects.equals(metadataId, other.metadataId) instead to handle the null case safely:

if (!Objects.equals(metadataId, other.metadataId))
    return false;

Note: hashCode() has the same risk: result = prime * result + metadataId will also unbox and NPE if metadataId is null.

featuredType == that.featuredType &&
Objects.equals(featuredType, that.featuredType) &&
creatorId == that.creatorId &&
url.equals(that.url) &&

@juanluisrp juanluisrp Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR fixes featuredType on line 181 but the adjacent fields url, creationDate, and creator use bare .equals() without null guards. All three are plain String fields with no @NotNull annotation and no initializer, so a default-constructed LinkDto has them as null. Calling equals() on such an instance throws NullPointerException.

The logo field just below already uses Objects.equals(logo, that.logo). The same pattern should be applied consistently to these three fields:

Objects.equals(url, that.url) &&
Objects.equals(creationDate, that.creationDate) &&
Objects.equals(creator, that.creator) &&

featuredType == that.featuredType &&
Objects.equals(featuredType, that.featuredType) &&
creatorId == that.creatorId &&
url.equals(that.url) &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as LinkDto: url, creationDate, and creator are compared with bare .equals() without null guards. The fields are uninitialized Strings with no @NotNull constraint, so a default-constructed UserSearchDto will throw NullPointerException when equals() is called.

Please apply Objects.equals() consistently here just as was done for featuredType on line 208:

Objects.equals(url, that.url) &&
Objects.equals(creationDate, that.creationDate) &&
Objects.equals(creator, that.creator) &&

- Use Objects.equals to avoid NullPointerException in comparisons of fields annotated with @nonnull and no initializer
- Update MetadataLink.hash to avoid NullPointerException
@josegar74 josegar74 force-pushed the fix/code-quality-issues branch from 6d70001 to 1fd72bc Compare June 18, 2026 10:05
@josegar74

Copy link
Copy Markdown
Member Author

@juanluisrp thanks for the review, I have addressed the issues, please check.

@josegar74 josegar74 merged commit 5a890e1 into geonetwork:main Jun 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants