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.

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

Validate the name of the topic when it's being unsubscribed #89

Merged
merged 1 commit into from
Apr 8, 2018

Conversation

minggangw
Copy link
Member

Before this patch, when the bridge received the operation code 'unsubscribe',
it will not validate the topic name(e.g. validity of the topic or if it existed).

This patch implements that when receiving the 'unsubscribe' op, the bridge will
valide the name of the topic first. If the name itself is invalid, an error will
return. If the topic name doesn't exist, a warning will return.

Fix #87

Before this patch, when the bridge received the operation code 'unsubscribe',
it will not validate the topic name(e.g. validity of the topic or if it existed).

This patch implements that when receiving the 'unsubscribe' op, the bridge will
valide the name of the topic first. If the name itself is invalid, an error will
return. If the topic name doesn't exist, a warning will return.

Fix RobotWebTools#87
@minggangw
Copy link
Member Author

@qiuzhong please verify this PR to see if #87 is fixed. I will merge it if it has been fixed and make a bug fix release of this module. Thanks!

@qiuzhong
Copy link
Collaborator

qiuzhong commented Apr 8, 2018

@minggangw , sure. Meanwhile, I'll uncomment the one test case in suite test-unsubscribe.js to see if it can pass. Then you can release a new version, is that OK?

@minggangw
Copy link
Member Author

No problem.

@qiuzhong
Copy link
Collaborator

qiuzhong commented Apr 8, 2018

@minggangw , I just tested and #87 was fixed by this patch. The uncommented test case can pass as well. Please merge this PR.

@minggangw
Copy link
Member Author

Thanks, will merge it soon.

@minggangw minggangw merged commit 7436a4a into RobotWebTools:develop Apr 8, 2018
@qiuzhong
Copy link
Collaborator

qiuzhong commented Apr 8, 2018

Thanks! I'll uncomment the test case metioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants