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

Feature: Use new db context handling #43

Merged
merged 3 commits into from
Apr 5, 2023

Conversation

cap10morgan
Copy link
Contributor

fluree/db#437 introduces a new :defaultContext key in the create API that doesn't merge with conn default contexts unless you explicitly request that via a ["" {...}] structure.

This updates http-api-gateway to work with that. All of the tests now pass in this branch pointed at the fix/context branch of db (plus one other small change in the history query test; :latest => "latest").

@cap10morgan cap10morgan requested a review from a team April 4, 2023 19:23
(let [first-txn (if (map? txn)
txn
(first txn))]
(cond-> {}
(-> first-txn keys first keyword?) (assoc :context-type :keyword)
(-> first-txn keys first string?) (assoc :context-type :string)
context (assoc :context context))))
defaultContext (assoc :defaultContext defaultContext))))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this causes us to ignore context on the /transact endpoint too, since this fn is called for both. Perhaps the support for defaultContext here should be strictly additive?

Copy link
Contributor

@mpoffald mpoffald Apr 5, 2023

Choose a reason for hiding this comment

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

🤔 wait now that I look closer, maybe that's not really a thing. I may be mixed up about the various places context can be supplied and how they should work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, and/or I might be mixed up too / instead. Let me make sure there is test coverage of that real quick.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, looking over the current fluree.db.json-ld.api/stage code in db, I don't think you can send a context in the opts of a transaction. Just a context-type. The context would go inside the transaction's JSON-LD data, I believe (i.e. in the map under the :txn key).

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, that sounds right. False alarm! 😅

Copy link
Contributor

@mpoffald mpoffald left a comment

Choose a reason for hiding this comment

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

✨ 🦡

@cap10morgan cap10morgan merged commit a7d2b7b into main Apr 5, 2023
@cap10morgan cap10morgan deleted the feature/use-new-db-context-handling branch April 5, 2023 16:57
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.

2 participants