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

owningSystemUrl in IServerInfo is not available in JS SDK's ServerInfo #1108

Open
arjanvanzutphen opened this issue Jun 12, 2023 · 3 comments
Assignees
Labels

Comments

@arjanvanzutphen
Copy link

arjanvanzutphen commented Jun 12, 2023

Describe the bug

The property 'owningSystemUrl ' is not available in JS SDK's ServerInfo but is defined in IServerInfo.

Should owningSystemUrl be removed or does it serve another purpose?

Reproduction

Run this snippet from the website: https://developers.arcgis.com/arcgis-rest-js/api-reference/arcgis-rest-request/ArcGISIdentityManager#fromCredential

require(["esri/id"], (esriId) => {
  const credential = esriId.findCredential("https://www.arcgis.com/sharing/rest");
  const serverInfo = esriId.findServerInfo("https://www.arcgis.com/sharing/rest");

  const manager = ArcGISIdentityManager.fromCredential(credential, serverInfo);
});

TypeScript will notice that 'owningSystemUrl' is not available in ServerInfo

Logs

webpack compiled with 1 warning
ERROR in src/logic/GroupLogic.ts:16:68
TS2345: Argument of type 'ServerInfo' is not assignable to parameter of type 'IServerInfo'.
  Property 'owningSystemUrl' is missing in type 'ServerInfo' but required in type 'IServerInfo'.
    14 |   );
    15 |
  > 16 |   const manager = ArcGISIdentityManager.fromCredential(credential, serverInfo);
       |                                                                    ^^^^^^^^^^
    17 |   debugger;
    18 |   try {
    19 |     const resultGroupUsers = await getGroupUsers(groupId, {

System Info

"@esri/arcgis-rest-request": "4.2.0",

Additional Information

No response

@gavinr-maps
Copy link
Contributor

Hi Arjan, thanks for logging this.

This snippet is actually old - using the JS API v3 syntx (for example, should be esri/identity/IdentityManager instead of esri/id). Could you please send a PR to update this snippet to the latest JS API? This can be found in https://github.com/Esri/arcgis-rest-js/tree/main/demos/jsapi-integration I think.

After that's updated, can you please provide an updated replication case.

@gavinr-maps
Copy link
Contributor

Snippet (non-)issue

After further looking into this, I think that snippet is actually acceptable:

I think the Credential that is returned from findCredential or getCredential should both be able to be passed into the ArcGIS REST JS ArcGISIdentityManager.fromCredential function.

So I don't think the snippet is the issue.

Replication case

I was able to create a simple replication case for this issue in a StackBlitz:
https://stackblitz.com/edit/vitejs-vite-ikrxa2?file=src%2Fmain.ts,src%2Fstyle.css,src%2Fmap.ts&terminal=dev

image

That ^ is the original issue (I think).

Given that, I do agree with @arjanvanzutphen that owningSystemUrl should be removed (or set to optional?) here. Note that this property does not seem to be included anymore: for example compare our fetch mock vs the actual response:
https://sampleserver6.arcgisonline.com/arcgis/rest/info/?f=json

{
  "currentVersion": 10.91,
  "fullVersion": "10.9.1",
  "soapUrl": "https://sampleserver6.arcgisonline.com/arcgis/services",
  "secureSoapUrl": null,
  "authInfo": {
    "isTokenBasedSecurity": true,
    "tokenServicesUrl": "https://sampleserver6.arcgisonline.com/arcgis/tokens/",
    "shortLivedTokenValidity": 60
  }
}

@gavinr-maps
Copy link
Contributor

Discussed with @patrickarlt, we propose to remove this line:

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

No branches or pull requests

2 participants