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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify fixture resolver #38569

Merged
merged 2 commits into from
Feb 25, 2020
Merged

Conversation

jhawthorn
Copy link
Member

Previously FixtureResolver had a copy-pasted version of the filtering already done by OptimizedFileSystemResolver. This PR replaces this by extracting the two places actual filesystem operations into separate methods and overriding those.

It would be nice to not rely on overriding methods at all, and to extract the actual filtering into a separate, reusable class, but I don't want to do that until some other changes are made to the
filtering.

This should also make FixtureResolver much more accurate to OptimizedFileSystemResolver, by also creating and caching the UnboundTemplate classes, which de-duplicate templates.

Because it's now more accurate, this also needed to fix a bad test which it broke 馃槄.

This test was attempting to test how cache keys work by modifying the
templates and seeing when that cache was fresh. This doesn't actually
work for real Resolvers, only FixtureResolver, and isn't desirable. We
absolutely want to share templates if they resolve to the same file.

Instead, this simplifies the test to only check that we get the correct
template for the locale we request.
@rails-bot rails-bot bot added the actionview label Feb 25, 2020
Previously FixtureResolver had a copy-pasted version of the filtering
already done by OptimizedFileSystemResolver. This PR replaces this by
extracting the two places actual filesystem operations into separate
methods and overriding those.

It would be nice to not rely on overriding methods at all, and to
extract the actual filtering into a separate, reusable class, but I
don't want to do that until some other changes are made to the
filtering.

This should also make FixtureResolver much more accurate to
OptimizedFileSystemResolver, by also creating and caching the
UnboundTemplate classes, which de-duplicate templates.
@jhawthorn jhawthorn marked this pull request as ready for review February 25, 2020 00:45
@jhawthorn jhawthorn merged commit 22d9b3e into rails:master Feb 25, 2020
@jhawthorn jhawthorn deleted the simplify_fixture_resolver branch February 25, 2020 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant