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

Target "prefetchContent" API #11

Merged
merged 15 commits into from
Feb 19, 2021
Merged

Target "prefetchContent" API #11

merged 15 commits into from
Feb 19, 2021

Conversation

yangyansong-adbe
Copy link
Contributor

No description provided.

@yangyansong-adbe yangyansong-adbe changed the title WIP: skeleton setup for prefetch API WIP: setup skeleton code for prefetch API Nov 19, 2020
@yangyansong-adbe yangyansong-adbe changed the title WIP: setup skeleton code for prefetch API WIP: Target "prefetchContent" API Dec 2, 2020
Copy link
Contributor

@cdhoffmann cdhoffmann left a comment

Choose a reason for hiding this comment

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

Still reviewing but some stuff to get started

AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryResponse.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target+PublicAPI.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@cdhoffmann cdhoffmann left a comment

Choose a reason for hiding this comment

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

Changes look good 👍 LMK when tests are done so I can take a look again.

AEPTarget/Sources/TargetProduct+Internal.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetProduct+Internal.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetParameters.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetParameters+Internal.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetConstants.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
@swarna04
Copy link
Contributor

Provided feedback here @yangyansong-adbe, expect a few more comments. Please also provide test cases and documentation.

@yangyansong-adbe yangyansong-adbe changed the title WIP: Target "prefetchContent" API Target "prefetchContent" API Feb 8, 2021
@yangyansong-adbe yangyansong-adbe linked an issue Feb 8, 2021 that may be closed by this pull request
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
super.init()
}

public func onRegistered() {
registerListener(type: EventType.target, source: EventSource.requestContent, listener: handle)
registerListener(type: EventType.target, source: EventSource.requestContent) { event in
if event.isPrefetchEvent {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do this in the a new requestContentHandler method/callback. Otherwise this callback will become big

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can do it when handling other target request events.

AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetConstants.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetConstants.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetConstants.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/TargetState.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/Target.swift Show resolved Hide resolved
AEPTarget/Sources/Target.swift Outdated Show resolved Hide resolved
@@ -16,7 +16,15 @@ import Foundation

@objc(AEPMobileTarget)
public class Target: NSObject, Extension {
internal static let LOG_TAG = "Target"
static let LOG_TAG = "Target"
Copy link
Contributor

Choose a reason for hiding this comment

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

Not resolved!


import Foundation

class TargetError: Error, CustomStringConvertible {
Copy link
Contributor

Choose a reason for hiding this comment

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

Misread this one! Ignore my comment here.

AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved

private static func generateTargetContext() -> TargetContext? {
let deviceType: DeviceType = systemInfoService.getDeviceType() == AEPServices.DeviceType.PHONE ? .phone : .tablet
let mobilePlatform = MobilePlatform(deviceName: systemInfoService.getDeviceName(), deviceType: deviceType, platformType: .ios)
Copy link
Contributor

Choose a reason for hiding this comment

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

Use systemInfoService.getCanonicalPlatformName() for platformType. Let's not hardcode here since we do not want to manage this in Target code for the supported platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enum:"android" "ios"
Only activities with the specified mobile platform type will be evaluated.

I think we don't need to do it as Target server side only supports above 2 types, and we will never send android type in Swift extension.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, that's what I've been trying to point out that we should avoid trying to hardcode the value on each platform when we can read the value from systemInfoService like we were doing before TargetRequestBuilder.cpp#L506

AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
AEPTarget/Sources/DeliveryRequest.swift Outdated Show resolved Hide resolved
@yangyansong-adbe yangyansong-adbe merged commit 1fe717b into dev Feb 19, 2021
@yangyansong-adbe yangyansong-adbe deleted the Prefetch branch April 9, 2021 19:39
yangyansong-adbe added a commit that referenced this pull request Apr 9, 2021
* Project skeleton (#8)

* cleanup project (#9)

cleanup project

* Swift Class placeholder  (#10)

* Placeholder for Swift classes

* review feedback

* disable Github action for now

* Circleci project setup (#23)

* Add .circleci/config.yml

* set up CircleCI

* no message

* no message

* no message

* Target "prefetchContent" API (#11)

* wip: skeleton setup for prefetch API

* more prefetch API details

* clean up code

* rearrange code

* AEPError -> Error

* fix test fails

* review feedback

* fix review comments

* add more docs

* format

* update CI scripts

* disable Github Action

* review feedback

* review feedback

* wip: skeleton setup for prefetch API

* more prefetch API details

* clean up code

* fix test fails

* review feedback

* fix review comments

* add more docs

* update CI scripts

* disable Github Action

* review feedback

* display notification tracking api

* Updating with Notification clicked api.

* Rebasing from prefetch branch

* Removing AEPIdentity from podspec

* Fixing compile time errors

* Changes based on comments

* fixes based on review comments

* Adding public api tests for displayed and clicked location

* Create swift.yml

* Preview manager test progress. Need to update once uiutil protocol is set up.

* Add comment for preview manager protocol

* Fix spacing

* Swift format

* Add comments for ui delegate extensions

* Set up changes with UIService, mockable ui elements.

* Fix typos and change log tag

* Make sure preview token is not empty

* update createFullscreenMessage api calls from dev.

* add onshowfailure to fullscreenmessage delegate extension

* Remove invalid string from log.

* Removing unnecessary github actions

* Reset Experience API

* Updating the pr based on review comments

* remove the usage of CustomIdentity (#41)

* remove usage of CustomIdentity

* add function docs

* cleanup code

* fix review comments

* remove AEPIdentity dependency for AEPTarget target

* use the latest valid configuration (#46)

* Adding configuration response handling

* Updating the tests

* Fixes based on comments for tests

* Batch API feature

* Adding pair id for getting the response back to customer

* Adding A4T functionality

* Adding few unit tests

* - Update api to handle response content event
- Updating unit tests

* Updating based on previous pr comments

* Updating based on comments

* Using thread safe dictionary

* Fixing the tests

* Adding Objective c app

* ObjC App UI

* attempt to fix UI mishap

* objc app and changes in sdk for it

* Updates based on comments

* Adding clear prefetch cache implementation

* Removing signal

* add release scripts

* add integration test - prefetch (#49)

* add integration test - prefetch

* clean up

* Adding Assurance to target demo apps

* release automation (#56)

* Data migration (#52)

* migration stored V5 data to new format

* no message

* fix review comments

* Preview manager test progress. Need to update once uiutil protocol is set up.

* Setup preview manager target class usage.

* wip: skeleton setup for prefetch API

* more prefetch API details

* clean up code

* fix test fails

* review feedback

* fix review comments

* add more docs

* Mock previewmanager and initial test for Target preview events

* Updated

* Removed extra files

* Updating tests

* Updating tests

* Updating preview api

* Updating demo app to send a generic os event

* Rebasing after conflicts

* Updating tests

* add tests for prefetch  (#61)

* add tests

* add tests and fix format

* Seperating functioonal tests with unit tests

* clean up project (#67)

* retrieve configuration and update target state (#54)

* retrieve configuration and update target state (option 1)

* lint

* store configuration shared state in TargetState object

* fix tests fail

* fix lint

* add tests for TargetState

* remove duplicated function

* improve the readability of code (#73)

* Add functional test and updates on the request processing (#74)

* Adding more functional tests

* Adding more tests

* Add more integration tests (#69)

* more tests

* enable "testRetrieveLocationContent"

* not use empty target server url (#75)

* Adding more tests

* using env id from configuration (#76)

* Checking objc annotation for all the public apis

* Refactor the code to update session timestamp in one place  (#77)

* refactoring

* increase the timeout

* adding missed objc annotation

* V4 data migration (#63)

* migration stored V5 data to new format

* no message

* data migration for V4

* manually merges the missing code

* revert changes

* no message

* fix review comments

* add some trace logs

* increase timeout in tests

* clean up Prefetch code and add more tests (#79)

* not allow dispatching multiple error response events for prefetch API

* add missing key ("prefetchresult") in prefetch response event

* Update AEPTarget.podspec

* Update Makefile

* Updating the test app to use analytics

* clean up tests code (#83)

* Speed up CI times  (#84)

* no message

* no message

* no message

* no message

* no message

* no message

* no message

* no message

* add functional tests for loadRequest (#85)

* Extension class should not call MobileCore.dispatch (#86)

* Add docs for public APIs (#62)

* add docs for prefetch API

* revert changes for the demo apps

* add syntax

* Update AEPTarget.md

* Update AEPTarget.md

* add docs for public APIs

* revert changes for demo apps

* update readme

* no message

* no message

* Update LICENSE

* Update COPYRIGHT

* add API description for "Viaual preview"

* Migrates all functional test cases from ACPTarget project (prefetch/retrieve) (#87)

* add functional tests for loadRequest

* no message

* no message

* add tests for prefetch API

* import SwiftJSON library to the test code

* Migrates all functional test cases from ACPTarget project (session/edgeHost/reset ...)  (#88)

* add functional tests for loadRequest

* no message

* no message

* add tests for prefetch API

* import SwiftJSON library to the test code

* more tests

* no message

* allow set nil for public API - setThirdPartyId (#89)

* add tests for "LocationClicked" API (#90)

* add tests for "LocationDisplayed" API (#91)

* add more tests (#92)

* GA release (#81)

* update podspec for GA release

* clean up project

* revert format

* fix format

* update public API signature (#93)

* update public API signature

* update API comments

* review comments

* update test app

* update all public APIs

* doc

* update tests

* clean up API docs

* Update codecov.yml

Co-authored-by: Christopher Hoffmann <cdhoffmann@users.noreply.github.com>
Co-authored-by: ravinsingh jain <ravjain@adobe.com>
Co-authored-by: ravjain-adb <61328814+ravjain-adb@users.noreply.github.com>
Co-authored-by: Christopher Hoffmann <choffman@adobe.com>
Co-authored-by: pravin <pravinpk0707@gmail.com>
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.

[Prefetch] API implementation
4 participants