MF-1864 - CLI: Could not able to read messages from reader #1865

arvindh123 · 2023-07-21T08:10:18Z

Pull request title should be MF-XXX - description or NOISSUE - description where XXX is ID of issue that this PR relate to.
Please review the CONTRIBUTING.md file for detailed contributing guidelines.

What does this do?

Fixes CLI could not able to read messages from reader
Fixes CLI send messages topic path

Which issue(s) does this PR fix/relate to?

Resolves #1864

List any changes that modify/break current functionality

NA

Have you included tests for your changes?

Yes

Did you document any new/modified functionality?

NA

Notes

codecov · 2023-07-21T08:27:04Z

Codecov Report

Merging #1865 (6c11118) into master (7cccba9) will decrease coverage by 0.02%.
Report is 1 commits behind head on master.
The diff coverage is 9.09%.

@@            Coverage Diff             @@
##           master    #1865      +/-   ##
==========================================
- Coverage   67.36%   67.34%   -0.02%     
==========================================
  Files         119      119              
  Lines        9409     9411       +2     
==========================================
  Hits         6338     6338              
- Misses       2400     2402       +2     
  Partials      671      671

Files Changed	Coverage Δ
pkg/errors/sdk_errors.go	`0.00% <0.00%> (ø)`
pkg/sdk/go/message.go	`42.85% <50.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

arvindh123 · 2023-07-21T11:50:43Z

pkg/sdk/go/message.go

@@ -22,7 +22,7 @@ func (sdk mfSDK) SendMessage(chanName, msg, key string) errors.SDKError {
 		subtopicPart = fmt.Sprintf("/%s", strings.Replace(chanNameParts[1], ".", "/", -1))
 	}

-	url := fmt.Sprintf("%s/channels/%s/messages/%s", sdk.httpAdapterURL, chanID, subtopicPart)
+	url := fmt.Sprintf("%s/channels/%s/messages%s", sdk.httpAdapterURL, chanID, subtopicPart)


In Line#22 leading / is added to subtopic ,So removed here

Why removing here and not in Line#22 ?

If there is no subtopic , then ending / is no needed here, So better to have leading / in subtopics and not in main topic

drasko · 2023-07-22T22:30:21Z

pkg/sdk/go/message.go

@@ -34,7 +34,7 @@ func (sdk mfSDK) ReadMessages(chanName, token string) (MessagesPage, errors.SDKE
 	chanID := chanNameParts[0]
 	subtopicPart := ""
 	if len(chanNameParts) == channelParts {
-		subtopicPart = fmt.Sprintf("?subtopic=%s", strings.Replace(chanNameParts[1], ".", "/", -1))
+		subtopicPart = fmt.Sprintf("?subtopic=%s", chanNameParts[1])


Why do we remove this replacement, which I think comes from differences in NATS and in MQTT how they split the topics?

@drasko,

/ seperater in published topics of MQTT are replaced with . and published into NATS

According to Mainflux CLI help,
To read messages usage: read <channel_id.subtopic> <user_token>

mainflux-cli message read command calls SDK.ReadMessages function to get data from Readers.

I have published a message via mqtt with Topics channels/bb01ec9e-9c2c-429c-b0c6-de329da5c20e/messages/test/subtopic1/subtopic2,
In readers SubTopic are stored with like test.subtopic1.subtopic2

And CLI send message also have same . spereator to publish messages
mainflux-cli message send usage: send <channel_id.subtopic> <JSON_string> <thing_key>
I have published a message via mainflux-cli message send command with this topic bb01ec9e-9c2c-429c-b0c6-de329da5c20e.test.subtopic1.subtopic2,
Messages are sucessfully published and written to database

So, I decied, here replacement of . with / is not required.

Did you test by being subscribed also at MQTT topic at the same time? Do you receive messages in the RT on MQTT subscriber, and not only on Writer (which is NATS subscriber)?

Yes @drasko ,Tested,
Subscribed to MQTT topics and after subscribe receives successfully, then reads from reader

@arvindh123 this does not look good to me, because url below will have mix of / (in the main topic) and . in (subtopic)

@drasko , At present in CLI we can't able to read messages ,

A MQTT message is published
mosquitto_pub -u b639aacc-023d-4837-b3bd-f2016b2d93c5 -P 6a6f92e8-0997-4069-ba99-01b0d7d1427d -t "channels/ace3e631-8d9a-4825-bf0b-1a270297e76e/messages/test/subtopic1/subtopic2" -m <messageconent>

These message pickup by writer and stored them in DB as shown below

id channel subtopic publisher protocol name unit value string_value bool_value data_value sum time update_time

9391e57c-c0c7-492d-b64d-31d7e76e96e8 ace3e631-8d9a-4825-bf0b-1a270297e76e test.subtopic1.subtopic2 6a6f92e8-0997-4069-ba99-01b0d7d1427d http urn:dev:demo:10001BCD:temperature C 28.4 1690527291.310878 0.0

539ac63b-bd1c-4c2e-ba12-bad7cdf91b14 ace3e631-8d9a-4825-bf0b-1a270297e76e test.subtopic1.subtopic2 6a6f92e8-0997-4069-ba99-01b0d7d1427d http urn:dev:demo:10001BCD:humidity V 68.4 1690527291.310878 0.0

if you see in the above table
Subtopics are store in readerlike this test.subtopic1.subtopic2, which is different from topic sent from mqtt, in mqtt topics are sent with separate /

To read these message from reader , We need http api with subtopic filter like this http://localhost/reader?subtopic=test.subtopic1.subtopic2

And In CLI , we need to send command like this mainflux-cli messages read ace3e631-8d9a-4825-bf0b-1a270297e76e.test.subtopic1.subtopic2 `

Since in present code . got replace by / in readers SDK
The request sent to reader is like this http://localhost/reader?subtopic=test/subtopic1/subtopic2
Because of this request we are not getting any response.

drasko · 2023-07-28T00:53:58Z

pkg/sdk/go/message.go

@@ -34,7 +34,7 @@ func (sdk mfSDK) ReadMessages(chanName, token string) (MessagesPage, errors.SDKE
 	chanID := chanNameParts[0]
 	subtopicPart := ""
 	if len(chanNameParts) == channelParts {
-		subtopicPart = fmt.Sprintf("?subtopic=%s", strings.Replace(chanNameParts[1], ".", "/", -1))
+		subtopicPart = fmt.Sprintf("?subtopic=%s", chanNameParts[1])


@arvindh123 this does not look good to me, because url below will have mix of / (in the main topic) and . in (subtopic)

Signed-off-by: Arvindh <arvindh91@gmail.com>

drasko

LGTM

Signed-off-by: Arvindh <arvindh91@gmail.com> Co-authored-by: Drasko DRASKOVIC <drasko.draskovic@gmail.com> Signed-off-by: WashingtonKK <washingtonkigan@gmail.com>

Signed-off-by: Arvindh <arvindh91@gmail.com> Co-authored-by: Drasko DRASKOVIC <drasko.draskovic@gmail.com>

arvindh123 requested a review from a team as a code owner July 21, 2023 08:10

arvindh123 commented Jul 21, 2023

View reviewed changes

arvindh123 changed the title ~~MF-1864 - CLI Could not able to read messages from reader~~ MF-1864 - CLI: Could not able to read messages from reader Jul 21, 2023

ianmuchyri approved these changes Jul 21, 2023

View reviewed changes

drasko requested changes Jul 22, 2023

View reviewed changes

arvindh123 force-pushed the MF-1864 branch 2 times, most recently from d9dfa2d to ed23945 Compare July 26, 2023 15:20

arvindh123 requested a review from drasko July 27, 2023 08:47

SammyOina approved these changes Jul 27, 2023

View reviewed changes

drasko requested changes Jul 28, 2023

View reviewed changes

fix: cli message send and reader topic

2fcc660

Signed-off-by: Arvindh <arvindh91@gmail.com>

arvindh123 force-pushed the MF-1864 branch from ed23945 to 2fcc660 Compare July 28, 2023 07:12

drasko approved these changes Jul 28, 2023

View reviewed changes

drasko added 2 commits July 28, 2023 14:46

Merge branch 'master' into MF-1864

593d2b6

Merge branch 'master' into MF-1864

6c11118

drasko merged commit 57d47fe into absmach:master Jul 28, 2023
1 of 3 checks passed

dborovcanin deleted the MF-1864 branch August 1, 2023 21:04

rodneyosodo pushed a commit to rodneyosodo/magistrala that referenced this pull request Aug 3, 2023

fix: cli message send and reader topic (absmach#1865)

9758487

Signed-off-by: Arvindh <arvindh91@gmail.com> Co-authored-by: Drasko DRASKOVIC <drasko.draskovic@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MF-1864 - CLI: Could not able to read messages from reader #1865

MF-1864 - CLI: Could not able to read messages from reader #1865

arvindh123 commented Jul 21, 2023

codecov bot commented Jul 21, 2023 •

edited

Loading

arvindh123 Jul 21, 2023 •

edited

Loading

drasko Jul 22, 2023

arvindh123 Jul 24, 2023 •

edited

Loading

drasko Jul 24, 2023

arvindh123 Jul 24, 2023

drasko Jul 28, 2023

arvindh123 Jul 28, 2023

drasko Jul 28, 2023

drasko left a comment

id	channel	subtopic	publisher	protocol	name	unit	value	string_value	bool_value	data_value	sum	time	update_time
9391e57c-c0c7-492d-b64d-31d7e76e96e8	ace3e631-8d9a-4825-bf0b-1a270297e76e	test.subtopic1.subtopic2	6a6f92e8-0997-4069-ba99-01b0d7d1427d	http	urn:dev:demo:10001BCD:temperature	C	28.4					1690527291.310878	0.0
539ac63b-bd1c-4c2e-ba12-bad7cdf91b14	ace3e631-8d9a-4825-bf0b-1a270297e76e	test.subtopic1.subtopic2	6a6f92e8-0997-4069-ba99-01b0d7d1427d	http	urn:dev:demo:10001BCD:humidity	V	68.4					1690527291.310878	0.0

MF-1864 - CLI: Could not able to read messages from reader #1865

MF-1864 - CLI: Could not able to read messages from reader #1865

Conversation

arvindh123 commented Jul 21, 2023

What does this do?

Which issue(s) does this PR fix/relate to?

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

codecov bot commented Jul 21, 2023 • edited Loading

Codecov Report

arvindh123 Jul 21, 2023 • edited Loading

Choose a reason for hiding this comment

Why removing here and not in Line#22 ?

drasko Jul 22, 2023

Choose a reason for hiding this comment

arvindh123 Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

drasko Jul 24, 2023

Choose a reason for hiding this comment

arvindh123 Jul 24, 2023

Choose a reason for hiding this comment

drasko Jul 28, 2023

Choose a reason for hiding this comment

arvindh123 Jul 28, 2023

Choose a reason for hiding this comment

drasko Jul 28, 2023

Choose a reason for hiding this comment

drasko left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 21, 2023 •

edited

Loading

arvindh123 Jul 21, 2023 •

edited

Loading

Why removing here and not in `Line#22` ?

arvindh123 Jul 24, 2023 •

edited

Loading