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

Issue 517 - Unify representation of end-user identity between ExperienceEvent and Profile #523

Merged
merged 8 commits into from
Nov 21, 2018

Conversation

kstreeter
Copy link
Collaborator

This PR addresses issue #517.

Currently, EE and Profile have different representations of end user identities. This has caused some confusion by implementors, and in addition the representation in EE cannot carry multiple identities of a single namespace, which is a problem for some use cases.

The new representation uses the newly introduced "map" type to model identities as a map from identity namespace ID to one or more instances of the Identity schema. This makes it possible to carry multiple identities from a single namespace (which was possible with the array representation), while still doing direct lookups for the presence of identities of a specific space (which was not possible with the array representation).

@kstreeter kstreeter added this to the 0.9.7 milestone Sep 27, 2018
{
"xdm:id": "2394509340-30453470347",
"xdm:namespace": {
"xdm:code": "AVID"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the code redundant? I take it this part is optional and all of xdm:namespace could be considered optional?

Choose a reason for hiding this comment

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

If the intention of using an ID is to have a stable reference to the namespace and not have systems use the code in their implementations, then we should remove it. There's nothing stopping any implementation from assuming the code can be used for generating a fully qualified identity.

Choose a reason for hiding this comment

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

This structure allows for specifying different namespace codes mapped to the same namespace key. Is that OK? If not, xdm:namespace should be outside the id object.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the code can change, it makes sense to allow for the current value to (optionally) be captured in the data.

Choose a reason for hiding this comment

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

If we want both id and code how we are planning to get id at the time of ETL. Are we saying ETL ecosystem with integrate with Adobe Service to fetch id?

Copy link

@rkpandey rkpandey Nov 14, 2018

Choose a reason for hiding this comment

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

Also, if keys are going to be represented as "https://data.adobe.io/entities/namespace/4" how are we going to use this in json pointer notation. REF-> https://tools.ietf.org/html/rfc6901

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@rkpandey the mechanism is that external callers use the integration code (as it is the user-facing "handle" for the namespace), and the system transforms to id for storage as appropriate. The interface is the code, while the id is used internally.

There is no incompatibility between URIs and JSON Pointer. JSON Pointer allows for escaping of reserved characters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@harleensahni , @biswas-adobe see new definition that includes only the xdm:id and authenticated state in the map entry.

@harleensahni
Copy link
Contributor

@kstreeter ProfileStitch Identities should also be updated: https://github.com/adobe/xdm/blob/master/schemas/context/profilestitch.schema.json#L26 to use IdentityMap for consistency.

@kstreeter kstreeter modified the milestones: 0.9.7, 0.9.8 Oct 30, 2018
@tcuevas-adobe
Copy link

This breaks open source tools. Json key names need to compliant with open source tools. Using urls/uris in keys breaks these tools.

Copy link

@tcuevas-adobe tcuevas-adobe left a comment

Choose a reason for hiding this comment

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

This breaks open source tools. Json key names need to compliant with open source tools. Using urls/uris in keys breaks these tools.

@cdegroot-adobe
Copy link
Contributor

@kstreeter - we also have another identities[] representation here https://github.com/adobe/xdm/blob/master/schemas/context/profilestitch.schema.json

What do you wan to do with this? considering we are moving away for the identities array?

"xdm:code": "ECID"
}
}
"xdm:identityNap": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Misspelling "identityNap"

@@ -38,9 +38,15 @@
"description": "The time at which this interaction was received by a server."
},
"xdm:endUserIDs": {
"meta:status": "deprecated",
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the plan was to make this a breaking change and just remove it so we're clean.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we are going to do an initial drop of this change with both to help testing. We will drop the older fields when we do the larger schema refactor.

@kstreeter kstreeter merged commit 76f7077 into master Nov 21, 2018
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.

8 participants