Skip to content

[improve][pip]pip-417:Enable SpotBugs FindPublicAttributes Check #24290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions pip/pip-417.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# PIP-417: Enable SpotBugs `FindPublicAttributes` Check

## Background Knowledge

As Pulsar has upgraded the SpotBugs version from 4.7.x to 4.9.x ([PR #24243](https://github.com/apache/pulsar/pull/24243)), the `FindPublicAttributes` check is currently disabled.

To enable the `FindPublicAttributes` check, we need to refactor many classes by changing primitive fields from `public` to `private` to limit external accessibility. I have recently attempted this in [PR #24285](https://github.com/apache/pulsar/pull/24285).

However, such changes may break current public or internal interfaces — particularly in scenarios where a mixed-version Pulsar cluster is running during version upgrades or downgrades. Therefore, it is important to have a community discussion before proceeding.

## Motivation

The goal is to improve code quality and maintainability by enforcing encapsulation. Public fields expose internal state and make refactoring harder, especially in large-scale projects. By enabling the `FindPublicAttributes` check and resolving its violations, we reduce the risk of unintended access to internal data and enhance long-term API stability.

## Goals

- Enable the SpotBugs `FindPublicAttributes` check.
- Refactor fields violating the rule while ensuring backward compatibility.
- Avoid breaking any public or internal interfaces that may be used by Pulsar plugins or mixed-version clusters.

## In Scope

- Convert primitive fields declared as `public` into `private`, where appropriate.
- Use `@Getter` and `@Setter` annotations (e.g., from Lombok) to preserve access when needed.
- Leave public fields unchanged only when their modification would break compatibility, and use `@SuppressFBWarnings` to bypass the check in such cases.

## Public-facing Changes

### Example: `org/apache/pulsar/common/policies/data/PersistentOfflineTopicStats.java`

**Before:**

```java
/** Total number of messages. */
public long totalMessages;

/** Total backlog. */
public long messageBacklog;
```

**After:**

```java
@Getter
@Setter
private long totalMessages;

/** Total number of messages. */

@Getter
@Setter
private long messageBacklog;

/** Total backlog. */
```

These fields are made `private` to conform with the SpotBugs rule, while accessors maintain compatibility.

## Backward & Forward Compatibility

This change may affect public interfaces if users or plugins rely on direct access to public fields. For now, we will:

- Review each field individually before making it private.
- Suppress SpotBugs warnings in cases where direct access must be preserved.
- Ensure that any changes introduced do not break mixed-version Pulsar deployments.

## Links

- Related PR: https://github.com/apache/pulsar/pull/24285
- Related issue: https://github.com/apache/pulsar/issues/24245
- Mailing List discussion thread:
- Mailing List voting thread