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 subclasses not inheriting headers set in the superclass #383

Merged
merged 7 commits into from
Jan 24, 2024

Conversation

meanphil
Copy link
Contributor

@meanphil meanphil commented Mar 1, 2022

This fixes #376 - the headers weren't being shared because the InheritedHash class only works for individual key lookups.

But when the headers InheritedHash was merged into the request headers inside the Connection class it didn't consider
the @parent_hash.

The implementation of InheritedHash worked fine when look up individual keys, but when
it was merged into the request headers inside the Connection class, it didn't consider
the @parent_hash.
@meanphil
Copy link
Contributor Author

meanphil commented Mar 1, 2022

The inspection methods probably aren't necessary, but were useful at the time...

@fwininger
Copy link
Contributor

@meanphil whaou ! Thanks for this PR. I have the same issue

@fwininger
Copy link
Contributor

@meanphil can you add some unit tests ?

@rails-bot
Copy link

rails-bot bot commented Jun 1, 2022

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.

If it is an issue and you can still reproduce this error on the main branch,
please reply with all of the information you have about it in order to keep the issue open.

If it is a pull request and you are still interested on having it merged please make sure it can be merged clearly.

Thank you for all your contributions.

@rails-bot rails-bot bot added the stale label Jun 1, 2022
@rails-bot rails-bot bot closed this Jun 8, 2022
@rafaelfranca rafaelfranca reopened this Feb 8, 2023
@rails-bot rails-bot bot removed the stale label Feb 8, 2023
@rafaelfranca
Copy link
Member

Sorry for the delay. Can someone add a test for the problem?

@meanphil
Copy link
Contributor Author

I've added some tests, hopefully correctly!

@jpheos
Copy link

jpheos commented Feb 27, 2023

It's exactly the fix that I was looking for. Thank you @meanphil

@meanphil
Copy link
Contributor Author

@rafaelfranca any problem with this being merged now?

@rafaelfranca rafaelfranca merged commit 1517cad into rails:main Jan 24, 2024
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.

Active Resource 6 breaking change: can't share common headers anymore
4 participants