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

MF-919 - Mainflux message updates #924

Merged
merged 24 commits into from
Nov 5, 2019
Merged

Conversation

dborovcanin
Copy link
Collaborator

What does this do?

This pull request removes RawMessage, replacing it with Message. As a consequence of this, different message types are supported using the message Transformer. The concept is similar to what used to be Normalizer service but more generic and cleaner. For more information, please read the corresponding issue (#919).

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

Resolves #919.

List any changes that modify/break current functionality

The code is reorganized in such a way that RawMessage is renamed to Message. What used to be Message is moved to trasnformer/senml package because it represents SenML package.

Have you included tests for your changes?

Yes.

Did you document any new/modified functionality?

Yes.

@codecov-io
Copy link

codecov-io commented Oct 31, 2019

Codecov Report

Merging #924 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #924   +/-   ##
=======================================
  Coverage   82.47%   82.47%           
=======================================
  Files          75       75           
  Lines        5244     5244           
=======================================
  Hits         4325     4325           
  Misses        659      659           
  Partials      260      260

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7dc836...d7c505b. Read the comment docs.

@dborovcanin dborovcanin marked this pull request as ready for review November 1, 2019 17:53
@dborovcanin dborovcanin requested a review from a team as a code owner November 1, 2019 17:53
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

This looks good to me, misses only VerneMQ changes.

Please add them and I will approve.

Makefile Outdated
@@ -2,7 +2,7 @@
# SPDX-License-Identifier: Apache-2.0

BUILD_DIR = build
SERVICES = users things http ws coap lora normalizer influxdb-writer influxdb-reader mongodb-writer mongodb-reader cassandra-writer cassandra-reader postgres-writer postgres-reader cli bootstrap opcua
SERVICES = users things http ws coap lora influxdb-writer influxdb-reader mongodb-writer mongodb-reader cassandra-writer cassandra-reader postgres-writer postgres-reader cli bootstrap opcua
Copy link
Contributor

Choose a reason for hiding this comment

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

If all the addons are added here, probably it would be good to add transformer here as well, so that CI can check it and build the docker (for DoeckerHub usage).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If all the addons are added here, probably it would be good to add transformer here as well, so that CI can check it and build the docker (for DoeckerHub usage).

Normalizer is removed as a service and we're now using Transformers as packages imported by Message consumers to transform messages. Since there is no standalone Transformer service, we don't have it as an add-on, either.

Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

I would be good to rename also RawMessage record into something like Message or similar.

@@ -123,7 +123,7 @@ auth_on_publish(UserName, {_MountPoint, _ClientId} = SubscriberId, QoS, Topic, P
contentType => ContentType,
payload => Payload
},
mfx_nats:publish(NatsSubject, message_pb:encode_msg(RawMessage, raw_message)),
mfx_nats:publish(NatsSubject, message_pb:encode_msg(RawMessage, message)),
Copy link
Contributor

Choose a reason for hiding this comment

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

We want to keep RawMessage here?

@@ -75,7 +75,7 @@ func main() {
defer session.Close()

repo := newService(session, logger)
norm := normalizer.New()
norm := senml.New()
Copy link
Contributor

Choose a reason for hiding this comment

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

This norm doesn't make sense anymore

anovakovic01
anovakovic01 previously approved these changes Nov 4, 2019
Copy link
Contributor

@anovakovic01 anovakovic01 left a comment

Choose a reason for hiding this comment

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

LGTM!

manuio
manuio previously approved these changes Nov 4, 2019
Copy link
Contributor

@manuio manuio left a comment

Choose a reason for hiding this comment

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

LGTM

nmarcetic
nmarcetic previously approved these changes Nov 4, 2019
anovakovic01
anovakovic01 previously approved these changes Nov 5, 2019
Copy link
Contributor

@anovakovic01 anovakovic01 left a comment

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@anovakovic01 anovakovic01 left a comment

Choose a reason for hiding this comment

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

LGTM!

@nmarcetic nmarcetic merged commit f50335a into absmach:master Nov 5, 2019
manuio pushed a commit that referenced this pull request Oct 12, 2020
* Remove RawMessage

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Remove Normalizer

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Update tests

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Replace normalizer with senml-transformer

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Rename Transformer interface and package

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Update docs

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Remove SenML transformer service

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Remove SenML Protobuf support

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Fix readers

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Fix writers tests

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Refactor tests and remove normalizer

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Update docs

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Reanme Service interface to Transformer

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Use msg instead of rawmsg

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Remove rawMsg from Aedes code

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Fix VerneMQ files

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Remove RawMessage code

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Fix missing subtopic return

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Remove remaining RawMessage reference

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Fix formatting

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Fix readers and writers tests

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Rename SenML transformer variables

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>

* Fix readers and writers tests constants

Signed-off-by: Dušan Borovčanin <dusan.borovcanin@mainflux.com>
@dborovcanin dborovcanin deleted the MF-919 branch February 17, 2021 09:06
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.

Consider Mainflux message updates
6 participants