Set Device Object To Optional - According to API Design Guidelines by dfischer-tech · Pull Request #298 · camaraproject/DeviceLocation

dfischer-tech · 2025-02-15T07:49:28Z

What type of PR is this?

Add one of the following kinds:

correction

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #297

Changelog input

 release-note
 Update geofencing-subscriptions.yaml to make the Device object optional.

Additional documentation

None

bigludo7

LGTM

maxl2287

Thanks @dfischer-tech for raising up this PR!
Could you please also update the info.description by adding the information about how to identify the device from the access-token:

    # Identifying the device from the access token

    This API requires the API consumer to identify a device as the subject of the API as follows:
    - When the API is invoked using a two-legged access token, the subject will be identified from the optional `device` object, which therefore MUST be provided.
    - When a three-legged access token is used however, this optional identifier MUST NOT be provided, as the subject will be uniquely identified from the access token.

    This approach simplifies API usage for API consumers using a three-legged access token to invoke the API by relying on the information that is associated with the access token and was identified during the authentication process.

(see:

DeviceLocation/code/API_definitions/location-retrieval.yaml

Lines 73 to 77 in 5e81e4e

    
               # Identifying the device from the access token 
        
               This API requires the API consumer to identify a device as the subject of the API as follows: 
        
               - When the API is invoked using a two-legged access token, the subject will be identified from the optional `device` object, which therefore MUST be provided. 
        
               - When a three-legged access token is used however, this optional identifier MUST NOT be provided, as the subject will be uniquely identified from the access token.

)

maxl2287

LGTM

jlurien

If we opted to remove device as required in the request, we would need to remove it as well in the event contents. schemas AreaLeft, AreaEntered, SubscriptionEnds, and probably add some guideline that device must not be included in the event if not explicitly provided in the request

dfischer-tech · 2025-02-18T17:57:36Z

If we opted to remove device as required in the request, we would need to remove it as well in the event contents. schemas AreaLeft, AreaEntered, SubscriptionEnds, and probably add some guideline that device must not be included in the event if not explicitly provided in the request

Hi @jlurien,
Thank you for your review and comment.
The device is set to "optional" to prevent conflicts when it has already been identified via an access token (which would otherwise result in a 422 - UNNECESSARY_IDENTIFIER).
However, the device still needs to be identified; otherwise, a 422 - MISSING_IDENTIFIER error would occur, with the message:

"An identifier is not included in the request and the device or phone number identification cannot be derived from the 3-legged access token."

Therefore, we do not need to explicitly remove it from the event contents (schemas: AreaLeft, AreaEntered, SubscriptionEnds),
because it must be there/identified.

Clarification about the device needed

maxl2287 · 2025-02-18T21:05:30Z

@dfischer-tech

For a better understanding I would like to reference here as well to QualityOnDemand:

Note that the device object is defined as optional and will only to be returned if provided in createSession.
(https://github.com/camaraproject/QualityOnDemand/blob/r2.1/code/API_definitions/quality-on-demand.yaml#L502)

Means that we only would like to provide information about the device, if this was explicitly provided by the user.
Or in other words: only when a 2-legged-token was used and the device is required to be in the request-body then we have to add the device also in the:

$.config.subscriptionDetail of the response
$.data in the cloud-event

If we are dealing with a 3-legged-token I aggree with @jlurien that we also have to remove the mandatory device-object from the CloudEvent $.data

DeviceStatus is doing it the same to just provide the subscriptionId as an identifier:

    BasicDeviceEventData:
      description: Event detail structure for basic device events
      type: object
      required:
        - subscriptionId
      properties:
        device:
          $ref: "#/components/schemas/Device"
        subscriptionId:
          $ref: "#/components/schemas/SubscriptionId"

https://github.com/camaraproject/DeviceStatus/blob/r2.1/code/API_definitions/device-roaming-status-subscriptions.yaml#L934-L943

dfischer-tech · 2025-02-19T05:40:23Z

@jlurien @maxl2287
Thank you for the notice – understood and accepted.

maxl2287

LGTM

bigludo7

LGTM

jlurien

@dfischer-tech , @maxl2287 The changes are OK, but we have to revert versions to wip and /vwip in the URL before merging (as in #303). At the same time, issue #302 could be fixed in this PR

maxl2287 · 2025-02-21T13:49:54Z

@jlurien @bigludo7
Let's set back the versions via #305 first so we have a seperate PR for it.

jlurien · 2025-02-24T12:21:36Z

@jlurien @bigludo7 Let's set back the versions via #305 first so we have a seperate PR for it.

@dfischer-tech @maxl2287 already merged

maxl2287 · 2025-02-25T09:01:14Z

@jlurien @bigludo7 PR is now synced

jlurien

LGTM

feat: update geofencing-subscriptions.yaml

5e81e4e

dfischer-tech requested review from bigludo7, jlurien and maxl2287 as code owners February 15, 2025 07:49

dfischer-tech changed the title ~~feat: update geofencing-subscriptions.yaml~~ Set Device Object To Optional - According to API Design Guidelines Feb 15, 2025

bigludo7 previously approved these changes Feb 16, 2025

View reviewed changes

maxl2287 requested changes Feb 17, 2025

View reviewed changes

maxl2287 added the Spring25 Meta-release Spring25 label Feb 17, 2025

maxl2287 assigned dfischer-tech Feb 17, 2025

fix: add info how to identify the device from the access-token

a06407e

dfischer-tech dismissed bigludo7’s stale review via a06407e February 17, 2025 08:36

Update geofencing-subscriptions.yaml

9a02962

maxl2287 previously approved these changes Feb 18, 2025

View reviewed changes

jlurien reviewed Feb 18, 2025

View reviewed changes

maxl2287 self-requested a review February 18, 2025 20:58

feat: update device to optional for events

baed61d

fix: Linter issues

fab0ec1

maxl2287 requested a review from jlurien February 19, 2025 10:27

maxl2287 approved these changes Feb 19, 2025

View reviewed changes

This was referenced Feb 19, 2025

Release PR for Spring25 M4 milestone #301

Closed

Missing error 422 with code MISSING_IDENTIFIER in Geofencing Subscriptions API #300

Closed

jlurien mentioned this pull request Feb 20, 2025

Align Geofencing-subscriptions on Commonalities r2.3 #302

Closed

3 tasks

bigludo7 approved these changes Feb 20, 2025

View reviewed changes

jlurien approved these changes Feb 21, 2025

View reviewed changes

jlurien self-requested a review February 21, 2025 08:40

Merge remote-tracking branch 'upstream/main'

9e490ab

jlurien approved these changes Feb 25, 2025

View reviewed changes

jlurien merged commit 33bf007 into camaraproject:main Feb 25, 2025
2 checks passed

	# Identifying the device from the access token

	This API requires the API consumer to identify a device as the subject of the API as follows:
	- When the API is invoked using a two-legged access token, the subject will be identified from the optional `device` object, which therefore MUST be provided.
	- When a three-legged access token is used however, this optional identifier MUST NOT be provided, as the subject will be uniquely identified from the access token.

Conversation

dfischer-tech commented Feb 15, 2025

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Changelog input

Additional documentation

Uh oh!

bigludo7 left a comment

Choose a reason for hiding this comment

Uh oh!

maxl2287 left a comment • 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!

maxl2287 left a comment

Choose a reason for hiding this comment

Uh oh!

jlurien left a comment • 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!

dfischer-tech commented Feb 18, 2025

Uh oh!

maxl2287 commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dfischer-tech commented Feb 19, 2025

Uh oh!

maxl2287 left a comment

Choose a reason for hiding this comment

Uh oh!

bigludo7 left a comment

Choose a reason for hiding this comment

Uh oh!

jlurien left a comment • 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!

maxl2287 commented Feb 21, 2025

Uh oh!

jlurien commented Feb 24, 2025

Uh oh!

maxl2287 commented Feb 25, 2025

Uh oh!

jlurien left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

maxl2287 left a comment •

edited

Loading

jlurien left a comment •

edited

Loading

maxl2287 commented Feb 18, 2025 •

edited

Loading

jlurien left a comment •

edited

Loading