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

Fix resolving backend.context in cypher #662

Merged
merged 2 commits into from
Mar 17, 2022

Conversation

mszostok
Copy link
Member

@mszostok mszostok commented Mar 14, 2022

Description

Changes proposed in this pull request:

  • Fix resolving backend.context in cypher. Currently, the context was always taken from the user input even if the backend was not sent. This against our contract, in case of empty backend we should use the old backend.context

Update scenario:

  • if backend is not sent, use old settings
  • if backend was sent, use new backend.context value (if it's null then persist null which means, zero backend.context property)

Testing

  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. Run storage server:

    APP_LOGGER_LEVEL="debug" APP_LOGGER_DEV_MODE=true APP_SUPPORTED_PROVIDERS="dotenv" go run ./cmd/secret-storage-backend/main.go
  3. Run Local Hub:

    cd hub-js; APP_NEO4J_ENDPOINT=bolt://localhost:7687 APP_NEO4J_PASSWORD=okon APP_HUB_MODE=local npm run dev; cd ..
  4. Run test:

    GRPC_SECRET_STORAGE_BACKEND_ADDR="0.0.0.0:50051" go test ./pkg/hub/client/local/ -v -count 1

You can also manually test that using GraphQL playground.

@mszostok mszostok added bug Something isn't working area/hub Relates to Hub labels Mar 14, 2022
@mszostok mszostok added this to the 0.7.0 milestone Mar 14, 2022
@pkosiec pkosiec self-assigned this Mar 14, 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.

LGTM with one improvement suggestion. If that's not something easy to do, let's skip it and merge this PR 👍

hub-js/graphql/local/schema.graphql Show resolved Hide resolved
@mszostok mszostok enabled auto-merge (squash) March 17, 2022 08:36
@mszostok mszostok merged commit 6972bf2 into capactio:main Mar 17, 2022
@mszostok mszostok deleted the fix-context-store branch March 25, 2022 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hub Relates to Hub bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants