Skip to content

MailboxesAPI.getMailbox and getMessage return wrappers around null instead of null #92

Description

@dmccoystephenson

MailboxesAPI.getMailbox(Player) and MailboxesAPI.getMessage(int) (src/main/java/dansplugins/mailboxes/externalapi/MailboxesAPI.java) unconditionally wrap the lookup result:

public M_Mailbox getMailbox(Player player) {
    return new M_Mailbox(persistentData.getMailbox(player));
}

public M_Message getMessage(int ID) {
    return new M_Message(persistentData.getMessage(ID));
}

PersistentData.getMailbox and PersistentData.getMessage both return null when nothing is found, so a non-null M_Mailbox / M_Message is handed back whose delegate field is null. Every accessor on those wrappers delegates without a guard, so the failure surfaces to the integrating plugin as a NullPointerException at an arbitrary later call site rather than as a null return at the lookup.

This makes the idiomatic guard shown in API.md ineffective — if (message != null) is always true. API.md has been updated to describe the current behavior accurately, but the behavior itself is worth reconsidering, because it is the cross-plugin integration contract.

Suggested resolution

null should be returned when the underlying lookup finds nothing, so that integrating plugins can guard the lookup. This is a breaking change for any caller that currently relies on a non-null return, so it should be paired with an API version bump (currently v0.0.3) and a CHANGELOG.md entry.

This issue body was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions