Skip to content
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.

Sign up for GitHub

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

Jump to bottom

14 topics handler #29

Merged
merged 19 commits into from
Feb 9, 2024
Merged

14 topics handler #29

merged 19 commits into from
Feb 9, 2024

Conversation

cybertobbe
Copy link
Contributor

Related Issue(s)

Closes #14

Proposed Changes

We have created a Topic and a TopicTest class. Also updated the pom.xml file with assertJ and Junit5.

  • Change 1: Added tests for exact and non-exact matches between Topic instances.
  • Change 2: Enhanced the Topic class to handle invalid topic names more effectively.
  • Change 3: Introduced tests for exception handling when creating topics with empty strings or names starting with a forward slash.

cybertobbe and others added 4 commits February 4, 2024 20:05
@cybertobbe @altmannk @IngridHammargren
Initial commit, updating pom.xml with junit, assertJ.
b260946 
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
@cybertobbe @IngridHammargren
Test isExactMatch
4c971a5 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Kathrine Altmann <kathrine.altmann@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
@cybertobbe @IngridHammargren
Add test for no match between two topics
440ce89 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
@cybertobbe @IngridHammargren
Add test checking empty Topic when creating a new Topic instance in f…
96bd48e 
…actory method createTopic().

Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
@cybertobbe cybertobbe added the enhancement New feature or request label Feb 5, 2024
@cybertobbe cybertobbe linked an issue Feb 5, 2024 that may be closed by this pull request
Topics handler #14
Closed
ByHatice
ByHatice previously approved these changes Feb 6, 2024
View reviewed changes
Frlundsten
Frlundsten reviewed Feb 6, 2024
View reviewed changes
Copy link
Contributor

@Frlundsten Frlundsten left a comment •
edited
Loading

Choose a reason for hiding this comment

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

To be able to close #14 this class should handle wildcards matching. If not, then maby this could solve its own issue because we need the Topics class either way.

Topics can be an exact match like myhome/first_floor/kitchen/temperature but we can also use wildcards so myhome/first_floor/+/temperature is a valid topic name for subscribing to messages.

myhome/first_floor/kitchen/temperature
myhome/first_floor/livingroom/temperature

we want to subscribe to both with following:
myhome/first_floor/+/temperature

So we need to check that everything after first_floor that ends with temperature should be subscribed to.
I think that using regex might be a solution to this.
Or substrings

@cybertobbe @IngridHammargren @altmannk
Add test for forward slash, add equals and hashcode methods.
e0b059e 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
cybertobbe and others added 2 commits February 6, 2024 11:46
@cybertobbe @IngridHammargren @altmannk
Add test for single level wildcard.
3704895 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
@cybertobbe @IngridHammargren @altmannk
Add test for multilevel wildcard
41899de 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
@cybertobbe @IngridHammargren @altmannk
Add test for multilevel exception
2bccc85 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Ebyrdeu
Ebyrdeu requested changes Feb 6, 2024
View reviewed changes
pom.xml Outdated Show resolved Hide resolved
@cybertobbe @IngridHammargren @altmannk
Add getName() and make name private
947f429 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Frlundsten
Frlundsten requested changes Feb 6, 2024
View reviewed changes
Copy link
Contributor

@Frlundsten Frlundsten left a comment

Choose a reason for hiding this comment

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

Make name variable final and this will look good. Nice job with matchesWildcard method! 🥇

src/main/java/org/fungover/Topic.java Outdated Show resolved Hide resolved
Ebyrdeu
Ebyrdeu reviewed Feb 7, 2024
View reviewed changes
src/main/java/org/fungover/Topic.java Outdated Show resolved Hide resolved
src/main/java/org/fungover/Topic.java Outdated Show resolved Hide resolved
src/main/java/org/fungover/Topic.java Outdated Show resolved Hide resolved
altmannk and others added 6 commits February 8, 2024 18:58
@altmannk
Merge branch 'main' into 14-topics-handler
3861706
@altmannk @IngridHammargren @cybertobbe
Refactor: from class to record, move package to org.fungover.thunder
4868df0 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
@altmannk @IngridHammargren @cybertobbe
Refactor: extract code from create to new method checkTopicNamingVali…
dffb6ff 
…dation

Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
@altmannk @IngridHammargren @cybertobbe
Add method isAValidTopicName for wildcards, change exception message …
fd3a24a 
…in TopicTest

Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
@altmannk @IngridHammargren @cybertobbe
Refactor: remove 'public' from TopicTest
f5adfc2 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
@altmannk @IngridHammargren @cybertobbe
Add additional test to isAValidTopicName
381e3a9 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
altmannk and others added 3 commits February 9, 2024 00:27
@altmannk @IngridHammargren @cybertobbe
Add test for special char '$'
193f59d 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
@altmannk @IngridHammargren @cybertobbe
Add test for isValidForPublishing method
c3e732d 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
@altmannk @IngridHammargren @cybertobbe
Add test for isValidForSubscription method
f4bd51a 
Co-authored-by: Ingrid Hammargren <ingrid.hammargren@iths.se>
Co-authored-by: Tobias Sjöström <tobias.sjostrom@iths.se>
Co-authored-by: Katherine Altmann <katherine.altmann@iths.se>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Feb 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
96.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@marcusju23 marcusju23 added this pull request to the merge queue Feb 9, 2024
Merged via the queue into main with commit 561254d Feb 9, 2024
2 checks passed
@marcusju23 marcusju23 deleted the 14-topics-handler branch February 9, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IngridHammargren IngridHammargren IngridHammargren left review comments

@altmannk altmannk altmannk left review comments

@TrungMa1 TrungMa1 TrungMa1 left review comments

@Frlundsten Frlundsten Frlundsten approved these changes

@Ebyrdeu Ebyrdeu Ebyrdeu approved these changes

@ByHatice ByHatice ByHatice left review comments

@kappsegla kappsegla Awaiting requested review from kappsegla

@Emmelie83 Emmelie83 Awaiting requested review from Emmelie83

Assignees

@altmannk altmannk

@cybertobbe cybertobbe

@IngridHammargren IngridHammargren

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Topics handler
9 participants
@cybertobbe @kappsegla @Ebyrdeu @Frlundsten @TrungMa1 @altmannk @ByHatice @IngridHammargren @marcusju23