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

GraphQL: node set dependencies, full node graph #115

Merged
merged 75 commits into from
Jul 13, 2022

Conversation

MarkusHorstmann
Copy link
Collaborator

@MarkusHorstmann MarkusHorstmann commented May 5, 2022

This PR uses a NodeSet model (from the CESMII Profile Designer) and the widely used GraphQL "Hot Chocolate" library to expose a fully resolved object graph of all nodes in the nodesets. It is deployed in the CESMII Stage Cloud Library for easier evaluation.

Open issues:

  • Compatibility with current graphql schema

Done.

Decided to use the NuGet packages.

  • Should invalid models continue to be allowed? (Currently there are 5 such models)

Models are allowed but marked as having validation errors.

Try:

# Interfaces in all nodesets
query MyQuery {
  interfaces {
    nodes {
      browseName
    }
  }
}
# DeviceType and its first 3 super  types
query MyQuery {
  objectTypes(nodeId: "nsu=http://opcfoundation.org/UA/DI/;i=1002") {
    nodes {
      browseName nodeId
      properties {
        browseName nodeId
      }
      superType {
        browseName nodeId
        superType {
          browseName nodeId
          superType {
            browseName nodeId
          }
        }
      }
    }
  }
}
# Object types that contain "Vendor" or "Server" in their browseName
query MyQuery {
  objectTypes(where: {or: [{browseName: {contains: "Vendor"}}, {browseName: {contains: "Server"}}]}) {
    nodes {
      browseName
    }
  }
}
# Node sets, their required models, and the actual models/versions found in the cloud library
query MyQuery {
  nodeSets {
    nodes {
      modelUri
      publicationDate
      identifier
      requiredModels {
        modelUri
        publicationDate
        version
        model {
          modelUri
          publicationDate
          version
          identifier
        }
      }
    }
  }
}

@MarkusHorstmann MarkusHorstmann changed the title GraphQL: node dependencies, full node graph GraphQL: node set dependencies, full node graph May 5, 2022
@MarkusHorstmann MarkusHorstmann marked this pull request as ready for review May 10, 2022 04:14
@BoBiene
Copy link
Collaborator

BoBiene commented May 11, 2022

as requested in #106 we need API stability.
There is a released product using the API (https://www.opc-router.com/opc-router-4-27-update/)

@malessainray
Copy link
Collaborator

as requested in #106 we need API stability. There is a released product using the API (https://www.opc-router.com/opc-router-4-27-update/)

I strongly suggest converting this pullrequest to a draft as long as API stability is not ensured. Taking care of #48 first could ease the reviewing burden regarding API compatibility.

Copy link
Collaborator

@BoBiene BoBiene left a comment

Choose a reason for hiding this comment

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

Compatibility with current graphql schema

@BoBiene BoBiene self-requested a review May 11, 2022 18:22
@BoBiene BoBiene marked this pull request as draft May 11, 2022 18:23
@malessainray
Copy link
Collaborator

@MarkusHorstmann is the https://cloudlib-back-stage.azurewebsites.net/graphiql running the same version as in this pr?

Yes, essentially. Identical in graphql and data index: cesmii/CESMII-CloudLibrary@opcContrib/mhdev...cesmii:cesmii/develop

Sadly I cannot login or register there for the live of me. Please get in touch with me so that we can perform tests against the staging deployment.

@MarkusHorstmann
Copy link
Collaborator Author

Sadly I cannot login or register there for the live of me. Please get in touch with me so that we can perform tests against the staging deployment.

Your logins should now work. Please try again. There was an issue with the mailer.

@malessainray
Copy link
Collaborator

These changes break our client side implementation in shipping product of dependency resolution for nodesets. With the instance hosted at https://uacloudlibrary.opcfoundation.org/ it works fine. I will follow up with more details on what specifically changed.

@malessainray
Copy link
Collaborator

malessainray commented Jun 29, 2022

The from the method Opc.Ua.CloudLib.Client.UACloudLibClient.GetNamespacesAsync() returned tuple values for the identifiers all have a leading space. This in turn results in failing to download the nodeset using the exact identifier as in no place any string trimming is performed. The stable instance does not return those leading spaces.
image

@MarkusHorstmann
Copy link
Collaborator Author

The from the method Opc.Ua.CloudLib.Client.UACloudLibClient.GetNamespacesAsync() returned tuple values for the identifiers all have a leading space. This in turn results in failing to download the nodeset using the exact identifier as in no place any string trimming is performed. The stable instance does not return those leading spaces.

Fixed and added a test.

@malessainray
Copy link
Collaborator

Fixed and added a test.

@MarkusHorstmann when will the latest changes in this pullrequest be deployed to the staging instance for integration testing?

@MarkusHorstmann
Copy link
Collaborator Author

MarkusHorstmann commented Jul 7, 2022

@MarkusHorstmann when will the latest changes in this pullrequest be deployed to the staging instance for integration testing?

@malessainray It is now deployed at https://cloudlib-back-stage.azurewebsites.net/.

@MarkusHorstmann MarkusHorstmann dismissed BoBiene’s stale review July 8, 2022 17:15

The PR now retains full compability. Please re-review.

@malessainray
Copy link
Collaborator

malessainray commented Jul 12, 2022

I could no longer spot incompatibilities with our released product.

UACloudLibraryServer/AWSFileStorage.cs Outdated Show resolved Hide resolved
UACloudLibraryServer/Controllers/InfoModelController.cs Outdated Show resolved Hide resolved
@barnstee barnstee merged commit 7316e89 into OPCFoundation:main Jul 13, 2022
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.

4 participants