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

Add support to find Types based on prefix of parent nodes #620

Merged
merged 6 commits into from
Jan 28, 2022

Conversation

mszostok
Copy link
Member

@mszostok mszostok commented Jan 28, 2022

Description

Changes proposed in this pull request:

  • Add support to find Types based on prefix of parent nodes

  • Update hub-js development documentation

  • Populate additionalRefs to database, so it can be returned on GraphQL side

  • Add integration tests to test that Types are properly returned:

    • based on full path name
    • based on a prefix of the parent node
    • only child node if full path name specified
    • entries matching or regex (cap.core.type.generic.value|cap.type.platform.cloud-foundry)
    • all entries if there is no filter
    • no entries if prefix is not a regex and there is no Type with such explicit path
  • Fix bug that for current main version (43c7400) where no entries are returned for empty filter:

    gotTypes, err := cli.ListTypeRefRevisionsJSONSchemas(ctx, gqlpublicapi.TypeFilter{
        // no path filter
    })

    Problem: The Go JSONMarshaler marshals nil pathPattern as null which was not handled properly in Cyper query.

Testing

Integration test proves that it works.

To check that manually, you can test it directly with Public Hub:

  1. Run Neo4j:

    docker run -d \
      -p 7687:7687 -p 7474:7474 \
      -e "NEO4J_AUTH=neo4j/okon" \
      -e "NEO4JLABS_PLUGINS=[\"apoc\"]" \
      --name hub-neo4j-instance \
      ghcr.io/capactio/neo4j:4.2.13-apoc
  2. Populate database:

    1. Build populator: make build-tool-populator

    NOTE: ensure that you use newly built populator!

    env APP_JSONPUBLISHADDR=http://$(ipconfig getifaddr en0) APP_JSON_PUBLISH_PORT="8888" APP_MANIFESTS_PATH="manifests" populator register ocf-manifests --source github.com/capactio/hub-manifests/
  3. Run Public Hub:

    cd hub-js; APP_NEO4J_ENDPOINT=bolt://localhost:7687 APP_NEO4J_PASSWORD=okon APP_HUB_MODE=public npm start
  4. Run queries (e.g. use Insomnia):

    query GetExplicitItem {
        types(filter: { pathPattern: "cap.core.type.platform.kubernetes" }) {
    	    prefix
    	    path
    	    revisions {
    		    revision
    		    spec {
    			    additionalRefs
    		    }
    	    }
        }
    }
    
    query GetAllCorePlatforms {
        types(filter: { pathPattern: "cap.core.type.platform.*" }) {
    	    prefix
    	    path
    	    revisions {
    		    revision
    		    spec {
    			    additionalRefs
    		    }
    	    }
        }
    }
    
    query GetAllCoreTypes {
        types(filter: { pathPattern: "cap.core.type.*" }) {
    	    prefix
    	    path
    	    revisions {
    		    revision
    		    spec {
    			    additionalRefs
    		    }
    	    }
        }
    }
    
    query GetNomadPlatform {
        types(filter: { pathPattern: "cap.type.platform.nomad" }) {
    	    prefix
    	    path
    	    revisions {
    		    revision
    		    spec {
    			    additionalRefs
    		    }
    	    }
        }
    }
    
    query GetWithOR {
        types(
    	    filter: {
    		    pathPattern: "(cap.core.type.generic.value|cap.type.platform.nomad)"
    	    }
        ) {
    	    prefix
    	    path
    	    revisions {
    		    revision
    		    spec {
    			    additionalRefs
    		    }
    	    }
        }
    }
    
    query GetAllItems {
        types(filter: { pathPattern: "*" }) {
    	    prefix
    	    path
    	    revisions {
    		    revision
    		    spec {
    			    additionalRefs
    		    }
    	    }
        }
    }
    
    query ListAllTypes {
        types {
    	    prefix
    	    path
    	    revisions {
    		    revision
    		    spec {
    			    additionalRefs
    		    }
    	    }
        }
    }
    

Related issue(s)

@mszostok mszostok added enhancement New feature or request area/hub Relates to Hub area/cli Relates to CLI area/documentation Related to all activities around documentation labels Jan 28, 2022
@mszostok mszostok added this to the 0.7.0 milestone Jan 28, 2022
Copy link
Member

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

Your example queries work really well 👍

It would be great to also support:

query GetAllItems {
    types(filter: { pathPattern: "*" }) {
	    prefix
	    path
	    revisions {
		    revision
		    spec {
			    additionalRefs
		    }
	    }
    }
}

Or, if that not easy to implement, at least return some user-friendly error (that this is not supported/implemented), because currently we have an exception returned:

{
  "data": {},
  "errors": [
    {
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "code": "Neo.ClientError.Procedure.ProcedureCallFailed",
          "name": "Neo4jError",
          "stacktrace": [
            "Neo4jError: Failed to invoke function `apoc.cypher.runFirstColumn`: Caused by: org.neo4j.exceptions.InvalidSemanticsException: Invalid Regex: Dangling meta character '*' near index 0",
            "*",
            "^",
            "",
            "    at captureStacktrace (/app/node_modules/neo4j-driver-core/lib/result.js:239:17)",
            "    at new Result (/app/node_modules/neo4j-driver-core/lib/result.js:59:23)",
            "    at newCompletedResult (/app/node_modules/neo4j-driver-core/lib/transaction.js:375:12)",
            "    at Object.run (/app/node_modules/neo4j-driver-core/lib/transaction.js:229:20)",
            "    at Transaction.run (/app/node_modules/neo4j-driver-core/lib/transaction.js:101:34)",
            "    at _callee3$ (/app/node_modules/neo4j-graphql-js/dist/index.js:226:35)",
            "    at tryCatch (/app/node_modules/regenerator-runtime/runtime.js:63:40)",
            "    at Generator.invoke [as _invoke] (/app/node_modules/regenerator-runtime/runtime.js:294:22)",
            "    at Generator.next (/app/node_modules/regenerator-runtime/runtime.js:119:21)",
            "    at asyncGeneratorStep (/app/node_modules/@babel/runtime-corejs2/helpers/asyncToGenerator.js:5:24)"
          ]
        }
      },
      "message": "Failed to invoke function `apoc.cypher.runFirstColumn`: Caused by: org.neo4j.exceptions.InvalidSemanticsException: Invalid Regex: Dangling meta character '*' near index 0\n*\n^",
      "path": [
        "types"
      ]
    }
  ]
}

@mszostok
Copy link
Member Author

In general, the * is not a valid regex (valid is .*). That's the reason why we got such error. But still it is a good catch and you are right that we should handle that situation, I decided to treat * as a special character and in this case also return all manifests. After f7767fe it works properly 👍

Copy link
Member

@pkosiec pkosiec left a comment

Choose a reason for hiding this comment

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

Works flawlessly 👍

@mszostok mszostok merged commit fb12e9c into capactio:main Jan 28, 2022
@mszostok mszostok deleted the parent-nodes/find branch January 28, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Relates to CLI area/documentation Related to all activities around documentation area/hub Relates to Hub enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants