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

Change Identity schema identifier to unstructured string from URI #419

Closed
kstreeter opened this issue Jun 24, 2018 · 11 comments
Closed

Change Identity schema identifier to unstructured string from URI #419

kstreeter opened this issue Jun 24, 2018 · 11 comments
Labels
v0.9.3 Scheduled for v0.9.3

Comments

@kstreeter
Copy link
Collaborator

kstreeter commented Jun 24, 2018

After working with the current definition of Identity it has been decided that the current definition of Identity.@id is too restrictive due to the need to ensure that all characters in a contributed identity are URI safe. The preference is to define this field in a less structured way, and let implementations manage the details of the representation.

Since @id is defined as a URI, the suggestion is to change this field from @id to xdm:id and designate it as a string versus a URI. This is consistent to how we have represented unstructured identifiers in other places in XDM.

What are the schemas that are affected by the issue

Identity, EndUserIds, Profile, ExperienceEvent (and their extensions)

What are examples of products that are impacted by the issue

Analytics, Campaign, Ad Cloud, Target

@jbeckert
Copy link
Contributor

Is this explicitly only for Identity? Meaning all other usages of @id remain unchanged with the same "type": "string" and "format": "uri"?

@jbeckert
Copy link
Contributor

Does xdm:id for Identity need a "pattern" property to reject ids with prohibited characters, e.g. characters that would mess up the usage of the value in URL path components?

@lrosenthol
Copy link
Collaborator

Please don't use xdm:id - it will cause too much confusion with @id.

@fmeschbe
Copy link
Collaborator

@jbeckert:

Is this explicitly only for Identity?

Yes, all others keep the @id property

Does xdm:id for Identity need a "pattern" property to reject ids with prohibited characters

That is a good call-out. We should indeed to this at least in the description if not formalizing with a pattern. Your intent would be for this to be the "URL-safe" characters, correct ?

For example we could amend the description to state, that only characters should be chosen for the xdm:id which do not need to be percent-encoded when used in an URL path.

Or we could (in addition) declare this explicit pattern:


pattern = "^[a-zA-Z0-9._~!$&'()*+,;=:@-]+$"

This is the full ability of RFC 3986 pchar except percent-encoded which would be forbidden:

pchar         = unreserved / pct-encoded / sub-delims / ":" / "@"
unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
sub-delims    = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "="

I think it is debatable to support all those characters and we might want to actually limit to just the unreserved set.

@fmeschbe
Copy link
Collaborator

fmeschbe commented Jul 13, 2018

@lrosenthol I see your point. Unfortunately we cannot change this at this point.

@lrosenthol
Copy link
Collaborator

You actually need both - since @id is required in all cases. So in this case, you'd have both @id (which si the URI for the schema) plus xdm:id which is the identity. Which makes this even worse.

I will not be accepting any PR that contains xdm:id.

@fmeschbe
Copy link
Collaborator

Created #432 to deal with the character set limitation issue and to thus unblock this issue for release.

@fmeschbe
Copy link
Collaborator

Merged #420 and thus closing this issue for now.

@fmeschbe fmeschbe added the v0.9.3 Scheduled for v0.9.3 label Jul 13, 2018
@lrosenthol lrosenthol reopened this Jul 13, 2018
@lrosenthol
Copy link
Collaborator

@fmeschbe Please UNMERGE #420 ASAP. We are NOT in agreement to accept it.

@fmeschbe
Copy link
Collaborator

@lrosenthol working on it.

@fmeschbe
Copy link
Collaborator

After some discussion, I am stating:

  • We want to cut another internal drop v0.9.3 on July 16, the latest
  • We cannot get consensus in this timeframe
  • Teams/developers are waiting for the v0.9.3 drop
  • The Identity schema is marked stabilizing, so changes have to be expected
  • More discussion is needed around the need or not for the @id property and how we want to define it

Gathering this, the plan is:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v0.9.3 Scheduled for v0.9.3
Projects
None yet
Development

No branches or pull requests

4 participants