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(@aws-amplify/api): pass additionalHeaders to graphql function #5001

Conversation

jensbodal
Copy link
Contributor

Additional headers are merged after merging in headers set at config time.
Additional headers will overwrite any existing values and merge the rest.

Issue #, if available: #4981

Description of changes: Existing private _graphql function allowed passing a param called additionalHeaders but was never used. The public graphql function now accepts an optional additionalHeaders param and is passed to private _graphql method. _graphql method was modified to merge in additionalHeaders after merging in previously set headers. This is how I would expect the parameter to function.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link

codecov bot commented Feb 28, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@8f6acca). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5001   +/-   ##
=========================================
  Coverage          ?   76.39%           
=========================================
  Files             ?      175           
  Lines             ?     9670           
  Branches          ?     1982           
=========================================
  Hits              ?     7387           
  Misses            ?     2137           
  Partials          ?      146           
Impacted Files Coverage Δ
...aws-amplify-react/src/Auth/Provider/withGoogle.tsx 84.61% <0.00%> (ø)
packages/analytics/src/trackers/PageViewTracker.ts 88.31% <0.00%> (ø)
...kages/interactions/src/Providers/AWSLexProvider.ts 76.27% <0.00%> (ø)
packages/xr/src/Errors.ts 100.00% <0.00%> (ø)
...pubsub/src/Providers/AWSAppSyncRealTimeProvider.ts 18.55% <0.00%> (ø)
packages/core/src/Util/Retry.ts 94.11% <0.00%> (ø)
packages/storage/src/Storage.ts 91.01% <0.00%> (ø)
packages/core/src/Credentials.ts 33.17% <0.00%> (ø)
packages/core/src/ClientDevice/index.ts 71.42% <0.00%> (ø)
packages/core/src/ClientDevice/browser.ts 9.09% <0.00%> (ø)
... and 165 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f6acca...25eb960. Read the comment docs.

@jensbodal jensbodal force-pushed the issues/4981/graphql-use-additional-headers branch from 4f32753 to 8217b1c Compare February 28, 2020 14:15
Additional headers are merged **after** merging in headers set at config time.
Additional headers will overwrite any existing values and merge the rest.
@jensbodal jensbodal force-pushed the issues/4981/graphql-use-additional-headers branch from 8217b1c to a5df323 Compare February 28, 2020 14:16
@jensbodal jensbodal changed the title feat(@aws-amplify/storage): pass additionalHeaders to graphql function feat(@aws-amplify/api): pass additionalHeaders to graphql function Feb 28, 2020
@manueliglesias manueliglesias self-requested a review March 3, 2020 22:51
Copy link
Contributor

@manueliglesias manueliglesias left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution, looks great! 👍 🎉

@manueliglesias manueliglesias merged commit 44b4faf into aws-amplify:master Mar 3, 2020
jordanranz added a commit that referenced this pull request Mar 27, 2020
* Adding database abstraction for AsyncStorage

* Add storage adapter for React Native using AsyncStorage

* Add separate default adapters for Web and React Native

* Make error messages more meaningful

* Add support for Reachability in React Native

* Add @react-native-community/netinfo to devDependencies

* Pin down the version of crypto-js

* Enable publish from rn-datastore branch

* Preparing release

* chore(release): Publish [ci skip]

 - @aws-amplify/[email protected]
 - [email protected]
 - [email protected]
 - [email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]

* Allow in PubSub to add async custom headers

Allow in PubSub the same capabilities as API for custom headers:
* Async header
* Override headers (Authorization)

Check issue #4928 for explanations.

* chore(deps-dev): bump codecov from 1.0.1 to 3.6.5 (#4951)

Bumps [codecov](https://github.com/codecov/codecov-node) from 1.0.1 to 3.6.5.
- [Release notes](https://github.com/codecov/codecov-node/releases)
- [Commits](https://github.com/codecov/codecov-node/commits)

Signed-off-by: dependabot[bot] <[email protected]>

* (fix:@aws-amplify/pubsub) Fix for unsubscription new subscription race condition (#4956)

* Fix for unsubscription new subscription race condition

* Update packages/pubsub/src/Providers/AWSAppSyncRealTimeProvider.ts

* Fix to initialize only once and handle disconnection from network  (#4921)

* initialize only once

* Fix onerror handler for WebSocket

* Change log level for unsubscribe message

* Add regions and fix typo

* Handle disconnection

* chore(deps): bump nokogiri from 1.10.4 to 1.10.8 in /docs (#4974)

Bumps [nokogiri](https://github.com/sparklemotion/nokogiri) from 1.10.4 to 1.10.8.
- [Release notes](https://github.com/sparklemotion/nokogiri/releases)
- [Changelog](https://github.com/sparklemotion/nokogiri/blob/master/CHANGELOG.md)
- [Commits](sparklemotion/nokogiri@v1.10.4...v1.10.8)

Signed-off-by: dependabot[bot] <[email protected]>

* Use the db instance property of the Adapter class for all database operations (#4995)

Use the db instance property of the Adapter class for all database operations

* Removing unused code

* Remove rn-datastore branch publish config

* Preparing release

* chore(release): Publish [ci skip]

 - [email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - [email protected]
 - [email protected]
 - [email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]
 - @aws-amplify/[email protected]

* fix(core): move react-native dependency from dev

* fix(core): revert dep addition. Add to peer and devDeps

* ci: Add React Native integration testing with Detox (#5007)

* Make deploy wait on integ_rn_ios_storage (#5009)

* feat(@aws-amplify/api): pass additionalHeaders to graphql function (#5001)

Additional headers are merged **after** merging in headers set at config time.
Additional headers will overwrite any existing values and merge the rest.

* chore(@aws-amplify/api): fix non-breaking typo in function (#5034)

* fix(@aws-amplify/datastore): Fix query and delete types (#5032)

Fixes #4827

* Fix(@aws-amplify/interactions) fixes 4750 to properly use the spread operator (#4806)

* fix(@aws-amplify/datastore) Adding socket disconnection detection (#5086)

* Export necessary providers and types from the package root to avoid importing from generated paths (#5085)

* fix(aws-amplify-react): Fix Federated icons when using React Bo… (#5073)

* Explicitly set box-sizing, as CSS resets override to border-box

* Explicitly set line-height, as CSS resets override line-height

* Upgrade aws-sdk clients which resolves react native issues (#5124)

* Export necessary providers and types from the package root to avoid importing from generated paths

* Upgrade aws-sdk clients which resolves react native issues

* Enable integration tests on modularization branch (#5125)

* chore: Fix setup-dev script (#5035)

Let `unlink-all` succeed always

Co-authored-by: Iglesias <[email protected]>

* fix(@aws-amplify/datastore): Storage should be re-initialized after DataStore.clear() (#5083)

* fix(@aws-amplify/datastore): Fix #5076 storage not re-initialized after DataStore.clear()

* Adding comments

Co-authored-by: Manuel Iglesias <[email protected]>
Co-authored-by: Ashish Nanda <[email protected]>

* fix(aws-amplify-react): BREAKING - Remove "import '@aws-amplify… (#5138)

Importing non-JS files relies on a bundler & breaks SSR.

It works with CRA because CRA *builds all dependencies*, just-in-case.

* feat(@aws-amplify/datastore): Support non-@model types in DataStore (#5128)

* Upgrade immer

* Return sooner if predicates are empty

* Fix bug when trying to delete a model instance that is not persisted

* Support non-@model types

* Generate datastore coverage report and RN integ test

* Fix tslint error

* Remove  unit test case for onGetPost

* Remove unused code

* Rename instance initializer to initializeInstance

* Rename SchemaType to SchemaNonModel

* Rename types to nonModels in schema.js

* Rename type to nonModel

* Make nonModels optional in schema.js

* Remove generic constraint from createTypeClass

* Rename ModelOrTypeConstructorMap to TypeConstructorMap

* Rename createModelAndTypeClassses to createTypeClasses

* Rename createTypeClass to createNonModelClass

* chore: remove datastore form RN integ tests (#5139)

* Fix export type of datastore storage in unit tests

* test(@aws-amplify/datastore): Fix unit tests

* [RN] Migrate zen-observable to zen-observable-ts and fix zen-push import (#5155)

* Migrate zen-observable to zen-observable-ts and fix zen-push import

* Fix unit tests

* Pin aws-sdk versions and rename presignRequest to presign (#5171)

* Migrate zen-observable to zen-observable-ts and fix zen-push import

* Fix unit tests

* Pin aws-sdk versions and rename presignRequest to presign

* Upgrade aws-sdk clients from alpha to beta (#5209)

* Migrate zen-observable to zen-observable-ts and fix zen-push import

* Fix unit tests

* Pin aws-sdk versions and rename presignRequest to presign

* Upgrade aws-sdk clients from alpha to beta

* Update expiration type to Date as per change in aws-sdk

* Fix unit tests as per expiration type change

* Fix jest failures, update snapshot

* Update storage changelog

Co-authored-by: Ashish Nanda <[email protected]>
Co-authored-by: Rodriguez Elorza <[email protected]>
Co-authored-by: aws-amplify-bot <[email protected]>
Co-authored-by: Sebastien Schwartz <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Manuel Iglesias <[email protected]>
Co-authored-by: Ivan Artemiev <[email protected]>
Co-authored-by: Jens Bodal <[email protected]>
Co-authored-by: Ashika <[email protected]>
Co-authored-by: Manuel Iglesias <[email protected]>
Co-authored-by: Praveen Gupta <[email protected]>
Co-authored-by: Eric Clemmons <[email protected]>
Co-authored-by: Iglesias <[email protected]>
Co-authored-by: Yirako <[email protected]>
Co-authored-by: [email protected] <[email protected]>
@bensie
Copy link
Contributor

bensie commented May 3, 2020

@manueliglesias @jordanranz It looks like the merge of #5215 accidentally stomped on this change so additionalHeaders are no longer working. Can we get the change added back?

@bensie
Copy link
Contributor

bensie commented May 3, 2020

After more digging I see that API.graphql(...) is deprecated in favor of GraphQLAPI.graphql(...) and the latter supports additionalHeaders (though not for subscriptions - see #5576). This still appears to be an accidental merge error, but switching to the new GraphQLAPI is pretty straight forward so it's not critical to get this fixed. That said, the inline documentation still indicates that additionalHeaders are supported on API.graphql(...), so at the very least a docs cleanup is probably warranted.

@github-actions
Copy link

This pull request has been automatically locked since there hasn't been any recent activity after it was closed. Please open a new issue for related bugs.

Looking for a help forum? We recommend joining the Amplify Community Discord server *-help channels or Discussions for those types of questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants