这是indexloc提供的服务，不要输入任何密码
Skip to content

[rest] Allow colons in SSE topics filter #1052

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
Merged
merged 1 commit into from
Sep 29, 2019
Merged

[rest] Allow colons in SSE topics filter #1052

merged 1 commit into from
Sep 29, 2019
+3 −1

Conversation

@falmanna
Copy link
Contributor

@falmanna falmanna commented Sep 28, 2019

fixes #1043

@cweitkamp
Copy link
Contributor

cweitkamp commented Sep 28, 2019

Thanks. Can you please sign-off your work?

It would be nice to have some unit tests for the new pattern to cover at least all available core events. May I ask you to add them?

@cweitkamp cweitkamp added bug An unexpected problem or unintended behavior of the Core REST/SSE labels Sep 28, 2019
@falmanna falmanna changed the title [rest] allow colons in sse topics filter [rest] Allow colons in sse topics filter Sep 29, 2019
@falmanna
Copy link
Contributor Author

falmanna commented Sep 29, 2019

I signed off the work and added some tests for the single entity filter.

@cweitkamp
Copy link
Contributor

cweitkamp commented Sep 29, 2019 •
edited
Loading

Unfortunately the test failed https://travis-ci.org/openhab/openhab-core/jobs/591061769#L2130 - It looks like the dash ("-") is not allowed too:

[ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.24 s <<< FAILURE! - in org.eclipse.smarthome.io.rest.sse.internal.util.SseUtilTest
[ERROR] testValidInvalidFilters(org.eclipse.smarthome.io.rest.sse.internal.util.SseUtilTest)  Time elapsed: 0.139 s  <<< FAILURE!
java.lang.AssertionError: 

Expected: is <true>
     but: was <false>
	at org.eclipse.smarthome.io.rest.sse.internal.util.SseUtilTest.testValidInvalidFilters(SseUtilTest.java:56)

[ERROR] Failures: 
[ERROR]   SseUtilTest.testValidInvalidFilters:56 
Expected: is <true>
     but: was <false> 
[ERROR] Tests run: 4, Failures: 1, Errors: 0, Skipped: 0

@falmanna
Copy link
Contributor Author

falmanna commented Sep 29, 2019

That's strange, why did it pass when I ran it locally in Eclipse?

@falmanna
Copy link
Contributor Author

falmanna commented Sep 29, 2019

Okay, apparently I didn't rebuild the bundle before running tests.

Should I add another commit? or modify the previous?

@falmanna
Allow colons and dashes in sse topic filter validator
d3df647 
Signed-off-by: Feras Almanna <firas.almanna3@gmail.com>
cweitkamp
cweitkamp approved these changes Sep 29, 2019
View reviewed changes
Copy link
Contributor

@cweitkamp cweitkamp left a comment

Choose a reason for hiding this comment

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

LGTM. And thank you very much for your first contribution over here.

@cweitkamp cweitkamp merged commit b48f1c1 into openhab:master Sep 29, 2019
@cweitkamp cweitkamp added this to the 2.5 milestone Sep 29, 2019
@cweitkamp cweitkamp mentioned this pull request Sep 29, 2019
Closed
@wborn wborn changed the title [rest] Allow colons in sse topics filter [rest] Allow colons in SSE topics filter Dec 8, 2019
Rosi2143 pushed a commit to Rosi2143/openhab-core that referenced this pull request Dec 26, 2020
@Hilbrand @Confectrician
Updated descriptions of directory structure (openhab#1052)
89503ba 
Removed structure from binding page as it's duplicated on the guidelines page.
Updated and improved the description on the guidelines page with the information that was on the bindings page (that information was improved recently).

Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
@falmanna @splatch
[rest] Allow colons and dashes in sse topic filter validator (openhab…
72a0e30 
…#1052)

Fixes openhab#1043

Signed-off-by: Feras Almanna <firas.almanna3@gmail.com>
GitOrigin-RevId: b48f1c1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cweitkamp cweitkamp cweitkamp approved these changes

Assignees

No one assigned

Labels

bug An unexpected problem or unintended behavior of the Core REST/SSE

Projects

None yet

Milestone

2.5

Development

Successfully merging this pull request may close these issues.

[REST] Subscribing to single thing endpoint not working

2 participants

@falmanna @cweitkamp