[#8203] fix: prevent NPE in CreateTagFailureEvent when TagInfo is null

[PCloud63514]

What changes were proposed in this pull request?

• Add null guard in CreateTagFailureEvent constructor.
• Do not construct a NameIdentifier when TagInfo is null.
• Ensure tagInfo() and identifier() return null consistently if no TagInfo is provided.
• Added a unit test testNullTagInfoDoesNotThrow to verify expected behavior.

Why are the changes needed?

• Previously, passing a null TagInfo caused a NullPointerException in the constructor when creating the NameIdentifier.
• This patch prevents the unexpected NPE and ensures that failure events can still be created safely without tag information.

Fix: #8203

Does this PR introduce any user-facing change?

• No API changes.
• Behavioral change: CreateTagFailureEvent no longer throws NPE if TagInfo is null.
• In such cases, both tagInfo() and identifier() return null.

How was this patch tested?

• Added a new test in CreateTagFailureEventTest verifying null TagInfo does not throw.
• Verified that events with valid TagInfo still construct a proper NameIdentifier.
• Ensured all existing tests continue to pass.

[jerryshao]

@FANNG1 please help to review.

[FANNG1]

@PCloud63514 , thanks for your contribution, CreateTagFailureEvent is generated by Gravitino internally, and the tagInfo couldn't be null, so I prefer not to add an extra check for it, WDYT?

[PCloud63514]

@FANNG1 Thanks for your feedback.
I initially added this to prevent a possible NPE, but on second thought, I realize I didn’t fully consider in what cases TagInfo might not be provided. 😅
From what I can see, there doesn’t seem to be any case where TagInfo would be empty in the current flow.
Could there be something I’m overlooking?

[FANNG1]

@FANNG1 Thanks for your feedback. I initially added this to prevent a possible NPE, but on second thought, I realize I didn’t fully consider in what cases TagInfo might not be provided. 😅 From what I can see, there doesn’t seem to be any case where TagInfo would be empty in the current flow. Could there be something I’m overlooking?

Sorry, I don't have other good first issues for now : (

[PCloud63514]

@FANNG1 I shared these thoughts with the issue author and asked for their advice. I found the phrase “cheap insurance against hard-to-debug NPEs” very meaningful.

Also, since the NameIdentifier constructor internally checks the name, I believe it’s better to rely on the constructor’s responsibility for data integrity rather than causing an early NPE.

[jerryshao]

It is the internal code (not for the users). The developers should guarantee that it will never be null. If there's a null value, something must be wrong; the fix here tries to hide the issue, which I think is not proper.

[PCloud63514]

Thanks everyone for the discussion.
This PR was opened based on the issue reporter’s concern about a potential NPE.
After reviewing the feedback, the general agreement is that a null check is unnecessary.
I’ll proceed to close this PR in line with that consensus.