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

feat(adapter-neo4j): use isolated session per read/write transaction #8364

Closed
wants to merge 6 commits into from
Closed

Conversation

GregCKrause
Copy link

☕️ Reasoning

This change utilizes a session-per-transaction approach with the Neo4j adapter, rather than creating and maintaining a single session.

As per Neo4j docs, "multiple sessions are required to execute parallel queries".

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

May affect #5849

@vercel
Copy link

vercel bot commented Aug 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
auth-docs ❌ Failed (Inspect) Sep 2, 2023 4:56pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
next-auth-docs ⬜️ Ignored (Inspect) Visit Preview Sep 2, 2023 4:56pm

@vercel
Copy link

vercel bot commented Aug 20, 2023

@GregCKrause is attempting to deploy a commit to the authjs Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added adapters Changes related to the core code concerning database adapters neo4j @auth/neo4j-adapter labels Aug 20, 2023
@GregCKrause GregCKrause changed the title feat(adapter-neo4j): use isolated sessions per read/write transaction feat(adapter-neo4j): use isolated session per read/write transaction Aug 21, 2023
pnpm-lock.yaml Outdated Show resolved Hide resolved
*
* CREATE INDEX verification_token_composite_index IF NOT EXISTS
* FOR (v:VerificationToken) ON (v.identifier, v.token);
* ```
*/
export function Neo4jAdapter(session: Session): Adapter {
const { read, write } = client(session)
export function Neo4jAdapter(driver: Driver): Adapter {
Copy link
Member

Choose a reason for hiding this comment

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

Changing this signature would mean that this is a breaking change. Would this still be compatible with v4?

Copy link
Author

Choose a reason for hiding this comment

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

Are you referring to v4 of next-auth or the neo4j-driver package?

This would be a breaking change for the Neo4jAdapter, as sessions would no longer be accepted arguments.

Copy link
Member

@balazsorban44 balazsorban44 left a comment

Choose a reason for hiding this comment

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

Thanks! I am trying to understand if this PR should be considered breaking. Can you verify?

@GregCKrause
Copy link
Author

GregCKrause commented Aug 22, 2023

Thanks! I am trying to understand if this PR should be considered breaking. Can you verify?

Hey, @balazsorban44! You raised some good concerns here. I am going to work on the following today:

  1. Revert neo4j-driver 5.7.0 -> 5.11.0 bump (I believe this can be deferred to separate PR).
  2. Use corepack for pnpm per contribution docs.

This does introduce a breaking change to the Neo4jAdapter. Do I need to change the associated commit message to include BREAKING CHANGE, or is there some other action that should be taken?

@w7tf
Copy link

w7tf commented Jan 10, 2024

@GregCKrause, thank you very much for this PR. Sad to see that your hard work didn't make it into the package. However I wanted to let you know that I was able to fix the errors mentioned in #5849 on my side with the edits in this PR. Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters Changes related to the core code concerning database adapters neo4j @auth/neo4j-adapter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants