Skip to content

Set parent ID to Edge response events#69

Merged
kevinlind merged 7 commits intoadobe:dev-v2.1.0from
kevinlind:parentid
Jun 28, 2023
Merged

Set parent ID to Edge response events#69
kevinlind merged 7 commits intoadobe:dev-v2.1.0from
kevinlind:parentid

Conversation

@kevinlind
Copy link
Copy Markdown
Contributor

Description

Set parent ID to response events dispatched from NetworkResponseHandler. Parent ID is taken from events in sentEventsWaitingResponse, so if response request ID or event index does not match an Event in the list the dispatched event will have a null parentID.

Response events for Edge.getLocationHint() already contain the parent ID (via Event.Builder.inResponseToEvent() api), so no code changes needed. Added tests to verify parentId in location hint response.

Updates Mobile Core dependency to 2.2.0, which adds support for chaining events.

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@kevinlind kevinlind requested review from addb and emdobrin June 21, 2023 01:44
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 21, 2023

Codecov Report

❗ No coverage uploaded for pull request base (dev-v2.1.0@038b24e). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff              @@
##             dev-v2.1.0      #69   +/-   ##
=============================================
  Coverage              ?   83.63%           
  Complexity            ?      377           
=============================================
  Files                 ?       29           
  Lines                 ?     1570           
  Branches              ?      223           
=============================================
  Hits                  ?     1313           
  Misses                ?      160           
  Partials              ?       97           
Flag Coverage Δ
functional-tests 66.05% <0.00%> (?)
unit-tests 79.62% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Map<String, String> eventData = FunctionalTestUtils.flattenMap(resultEvents.get(0).getEventData());

assertEquals(10, eventData.size());
assertEquals(11, eventData.size());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

processResponseOnError attached the requestEventId even before, is this change for the requestId? I did not find it in the asserts below

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, for generic errors the requestEventId was not attached to the dispatched response, only the request ID. I changed the flow so generic errors are passed through to dispatchEventErrors like other errors, so the requestEventId is now attached.
https://github.com/adobe/aepsdk-edge-android/pull/69/files#diff-6ef6a1ddfee2bea9afe631a1ecae39d502b48d873db1a4d84b7d918b20bf41f2R282

Copy link
Copy Markdown
Contributor

@emdobrin emdobrin Jun 28, 2023

Choose a reason for hiding this comment

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

looks like this line was just setting the requestId as you mentioned, eventDataResponse.put(EdgeConstants.EventDataKeys.EDGE_REQUEST_ID, requestId);

Sounds good, let's assert for requestId too if we are not doing already.

@kevinlind kevinlind merged commit 8d56bde into adobe:dev-v2.1.0 Jun 28, 2023
@kevinlind kevinlind deleted the parentid branch June 28, 2023 19:11
@kevinlind kevinlind linked an issue Jun 28, 2023 that may be closed by this pull request
kevinlind added a commit that referenced this pull request Jul 10, 2023
Set parent ID to Edge response events (#69)
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.

Set the parentId in all responses

2 participants