Fix for MongoDBHandling on ObjectToEvent map handling #472

SantoshNC68 · 2020-08-23T13:42:40Z

Applicable Issues

NA

Description of the Change

Made code changes to handle the updation of events in Object to Event mapper.
Also updated the logic to not store duplicate eventIds.

Alternate Designs

Benefits

Performance improvements in storing the ObjectToEvent mapping relation without having to query multiple times.

Possible Drawbacks

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: @SantoshNC68 , @raja-maragani

e-pettersson-ericsson

Mostly comments on javadoc. Are there no updates needed in the tests for these changes?

src/main/java/com/ericsson/ei/handlers/EventToObjectMapHandler.java

src/main/java/com/ericsson/ei/handlers/MongoDBHandler.java

SantoshNC68 · 2020-08-24T07:30:39Z

Fixed comments from Emelie in 74d6c4b

src/main/java/com/ericsson/ei/handlers/EventToObjectMapHandler.java

e-pettersson-ericsson

It looks like the tests still needs to be updated, but otherwise I commented mostly on logging and general naming.

e-pettersson-ericsson · 2020-08-28T08:09:29Z

src/main/java/com/ericsson/ei/handlers/EventToObjectMapHandler.java


    public boolean isEventInEventObjectMap(String eventId) {
-        String condition = "{\"_id\" : \"" + eventId + "\"}";
+        String condition = "{\"objects\": { \"$in\" : [\"" + eventId + "\"]} }";


At the top of this class a variable is defined for this key called 'listPropertyName' - use it here also. However, given that you've changed the structure of this document to look like:

``` "_id": "aggregated-object-id", "objects": ["event-id", "event-id"] ```

the key of the list should perhaps be called "events" instead of "objects"? It would look somehting like this instead:

``` "_id": "aggregated-object-id", "events": ["event-id", "event-id"] ```

At the top of this class a variable is defined for this key called 'listPropertyName' - use it here also. However, given that you've changed the structure of this document to look like:

"_id": "aggregated-object-id",
"objects": ["event-id", "event-id"]

the key of the list should perhaps be called "events" instead of "objects"? It would look somehting like this instead:

"_id": "aggregated-object-id",
"events": ["event-id", "event-id"]

changed "events" instead of "objects"

"events" instead of "objects" changes will come in future, this changes will be release ASAP, could you please review the current changes

I see another PR open, which duplicates this one. Please update the changes in this PR and close the other one (then we keep the review history also) It's possible for @SantoshNC68 to allow @durga-vasaadi as collaborator on your fork, and this way you can both push to this one PR.

Yes I have done that and we have worked together, but because of the issues on the travis we have another PR. We will close that.

Also we will not make changes for events in this PR but will do it another PR as we need to see the impact of that.

src/main/java/com/ericsson/ei/handlers/MongoDBHandler.java

src/main/java/com/ericsson/ei/jmespath/JmesPathInterface.java

e-pettersson-ericsson · 2020-08-28T08:13:59Z

src/main/java/com/ericsson/ei/jsonmerge/MergeHandler.java

        try {
            // lock and get the AggregatedObject
            String aggregatedObject = getAggregatedObject(id, true);
+            LOGGER.debug("AGGREGATED OBJECT : " + aggregatedObject);


Do you really want to log the entire aggregation twice? They can be very big, so it might clutter the log.

removed logger

src/main/java/com/ericsson/ei/handlers/EventToObjectMapHandler.java

tobiasake · 2020-08-28T08:42:33Z

What is the plan of these latest changes. Are the plan to merge this to Master ? Or just have it in this 2.0 maintenance branch and never have it on Master.
There is an internal ticket about redo the whole latest 2.x releases due to the issue of queueing up event/aggregations in Java memory instead of letting those event wait in RabbitMq instead until EI can process them instead of having this waiting in Memory which is not persistent if Queue is full or EI crash, all this event/aggregation processing is lost.

As it is now, it can't be merge directly to master. You need to implement same thing one more time, but for Master branch on PR based on Master branch code base.

SantoshNC68 · 2020-09-01T06:37:46Z

What is the plan of these latest changes. Are the plan to merge this to Master ? Or just have it in this 2.0 maintenance branch and never have it on Master.
There is an internal ticket about redo the whole latest 2.x releases due to the issue of queueing up event/aggregations in Java memory instead of letting those event wait in RabbitMq instead until EI can process them instead of having this waiting in Memory which is not persistent if Queue is full or EI crash, all this event/aggregation processing is lost.

As it is now, it can't be merge directly to master. You need to implement same thing one more time, but for Master branch on PR based on Master branch code base.

Yes they will need to be updated in the master branch also and we will make it as a separate PR for master branch.

src/functionaltests/java/com/ericsson/ei/utils/DataBaseManager.java

e-pettersson-ericsson · 2020-09-01T12:51:05Z

src/functionaltests/java/com/ericsson/ei/utils/DataBaseManager.java

            for (String expectedID : new ArrayList<>(checklist)) {
-                if (expectedID.equals(document.get("_id").toString())) {
-                    checklist.remove(expectedID);
+            	Document objects = (Document) document.get("objects")) 


Shouldn't this be "events"?

Travis complains a lot about compilation errors, which can be discovered earlier if you run the maven commands in your local environment.

It should be events. Just wanted to check why the travis has been failing.

src/main/java/com/ericsson/ei/handlers/EventToObjectMapHandler.java

src/main/java/com/ericsson/ei/handlers/MongoDBHandler.java

src/main/java/com/ericsson/ei/jmespath/JmesPathInterface.java

src/main/java/com/ericsson/ei/waitlist/WaitListStorageHandler.java

e-pettersson-ericsson

I haven't been able to test this locally due to my Java environment, but since I know it's been tested in a real environment and I realize time is of the essence for this fix, I'll approve it. But I hope the remaining comments will be fixed in an upcoming PR as soon as possible. And that these changes are also made on master if the same issue occurs there.

SantoshNC68 added 3 commits August 23, 2020 12:16

Added fixes to MongoDB connections

bb9edc0

Updated the Mongo query handling to check the object mapping

f520587

Updated java doc for the newly created methods

9f7a0df

SantoshNC68 changed the title ~~Maintainance~~ Fix for MongoDBHandling on ObjectToEvent map handling Aug 23, 2020

e-pettersson-ericsson reviewed Aug 24, 2020

View reviewed changes

Fixed review comments

74d6c4b

SantoshNC68 added 4 commits August 25, 2020 13:06

Added to debug logs

ab8d08f

Added code changes to look in the objects instead of the eventId

84cea22

removed local changes in application.properties

6a6894e

removed local changes in application.properties

8fd5639

raja-maragani reviewed Aug 27, 2020

View reviewed changes

src/main/java/com/ericsson/ei/handlers/EventToObjectMapHandler.java Outdated Show resolved Hide resolved

Updated the logger to change from ID to condition

2b47e2c

e-pettersson-ericsson reviewed Aug 28, 2020

View reviewed changes

SantoshNC68 force-pushed the maintainance branch from ea444f2 to 2b47e2c Compare September 1, 2020 05:44

Added test case and updated comments

a0b4b42

SantoshNC68 and others added 12 commits September 1, 2020 08:51

Added a new collection name for event object map test

3a1753b

Added a new collection name for event object map test

ec68d4b

Added a new collection name for event object map test

319971a

Added a new collection name for event object map test

f462b80

Added a new collection name for event object map test

1199734

Added loggers for testing

e7e2318

logs for failing tescases

93f4395

Updated DatabaseManager to check expectedEventIds in objects

1acd190

Updated DatabaseManager to check expectedEventIds

81da686

Updated DatabaseManager

863659b

Fixes in Database Manager

dbec64e

Fixes in Database Manager

dba08e6

e-pettersson-ericsson reviewed Sep 1, 2020

View reviewed changes

SantoshNC68 added 4 commits September 1, 2020 15:17

Fixes in Database Manager

3728090

Removed the sysouts

1248e62

Modified login in Database Manager

abe9240

Updated to check events in objects list

55f5eab

e-pettersson-ericsson reviewed Sep 1, 2020

View reviewed changes

src/main/java/com/ericsson/ei/waitlist/WaitListStorageHandler.java Outdated Show resolved Hide resolved

Added other comments

f0f3f85

e-pettersson-ericsson approved these changes Sep 1, 2020

View reviewed changes

raja-maragani approved these changes Sep 1, 2020

View reviewed changes

SantoshNC68 merged commit ce574ff into eiffel-community:2.0.0-maintenance Sep 1, 2020

Fix for MongoDBHandling on ObjectToEvent map handling #472

Fix for MongoDBHandling on ObjectToEvent map handling #472

Uh oh!

Conversation

SantoshNC68 commented Aug 23, 2020

Applicable Issues

Description of the Change

Alternate Designs

Benefits

Possible Drawbacks

Sign-off

Uh oh!

e-pettersson-ericsson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SantoshNC68 commented Aug 24, 2020

Uh oh!

Uh oh!

e-pettersson-ericsson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durga-vasaadi Sep 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tobiasake commented Aug 28, 2020

Uh oh!

SantoshNC68 commented Sep 1, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

e-pettersson-ericsson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

durga-vasaadi Sep 1, 2020 •

edited

Loading