Skip to content
New issue

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

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

Already on GitHub? # to your account

[fix][common] TopicName: Throw IllegalArgumentException if localName is whitespace only #23691

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

pdolif
Copy link
Contributor

@pdolif pdolif commented Dec 8, 2024

Fixes #23690

Motivation

When creating a consumer with the topic name only being a whitespace we get:

java.lang.IllegalArgumentException: topicNames cannot have blank topic

That's why I expect TopicName.get(" ") to throw an exception too.

Modifications

Throw exception if localName is blank.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added a test with " " as the topic name.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository: pdolif#5

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 8, 2024
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@pdolif
Copy link
Contributor Author

pdolif commented Dec 13, 2024

/pulsarbot rerun-failure-checks

@codecov-commenter
Copy link

codecov-commenter commented Dec 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.75%. Comparing base (bbc6224) to head (8b3da9b).
Report is 797 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23691      +/-   ##
============================================
+ Coverage     73.57%   73.75%   +0.17%     
- Complexity    32624    34285    +1661     
============================================
  Files          1877     1945      +68     
  Lines        139502   150723   +11221     
  Branches      15299    17033    +1734     
============================================
+ Hits         102638   111160    +8522     
- Misses        28908    30971    +2063     
- Partials       7956     8592     +636     
Flag Coverage Δ
inttests 27.25% <0.00%> (+2.66%) ⬆️
systests 24.64% <0.00%> (+0.31%) ⬆️
unittests 73.14% <100.00%> (+0.29%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ava/org/apache/pulsar/common/naming/TopicName.java 95.45% <100.00%> (-0.61%) ⬇️

... and 679 files with indirect coverage changes

@Technoboy- Technoboy- merged commit 034791f into apache:master Dec 17, 2024
102 of 106 checks passed
lhotari pushed a commit that referenced this pull request Dec 17, 2024
lhotari pushed a commit that referenced this pull request Dec 17, 2024
lhotari pushed a commit that referenced this pull request Dec 17, 2024
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 19, 2024
…is whitespace only (apache#23691)

(cherry picked from commit 034791f)
(cherry picked from commit 0e4dbd1)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Dec 23, 2024
…is whitespace only (apache#23691)

(cherry picked from commit 034791f)
(cherry picked from commit 0e4dbd1)
hanmz pushed a commit to hanmz/pulsar that referenced this pull request Feb 12, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] TopicName does not throw IllegalArgumentException if localName is whitespace only
5 participants