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

Cannot resolve partials relative to localized templates in app bundle #87

Closed
marcpalmer opened this issue Dec 5, 2014 · 14 comments
Closed

Comments

@marcpalmer
Copy link

I found a surprising problem with the latest GRMustache released to cocoapods (7.3.0) that I don't believe existed in the past i.e. it feels like a regression somewhere, either in the iOS 8 NSBundle APIs or GRMustache.

Here's the scenario:

  • You have a template "Main.mustache" in "/en.lproj", and it includes a partial called "Footer.mustache" which is in the same localisation folder "/en.lproj"
  • You ask GRMustache to load template "Main" using template = [GRMustacheTemplate templateFromResource:name bundle:[NSBundle mainBundle] error:&error];
  • GRMustache loads "Main.mustache" with a URL that includes "/en.lproj"
  • GRMustache tries to load "Footer.mustache" relative to that URL, which includes "/en.lproj", which returns nil

See attached debugger screenshot showing the state of the stack and lldb output where I show that po [_bundle pathForResource:@"EmailShareFooter" ofType:@"mustache"] succeeds but [_bundle pathForResource:@"EmailShareFooter" ofType:@"mustache" inDirectory:relativePath]

It is obviously choking on the relativePath as the relativePath should be blank in this case, but it is '/en.lproj' so iOS is trying to look for <appfolder>/en.lproj/EmailShareFooter.mustache" and while this is correct, seems to fail, I'm guessing its actually looking for/en.lproj/en.lproj/EmailShareFooter.mustache` as the resource name.

screen shot 2014-12-05 at 11 42 58

@groue
Copy link
Owner

groue commented Dec 5, 2014

Hi @marcpalmer,

This regression is a side effect of #78 which has been shipped in v7.1.0.

OK, this deserves a fix.

@marcpalmer
Copy link
Author

Yes, I just found the commit f598e62 that seems to be responsible for this.

It seems quite tricky to solve... any thoughts? We can't know safely which part of the URL NSBundle is the localisation part can we? Isn't the "real" solution to keep the NSURL source of the file around and use that as baseTemplateURL instead of having a baseTemplateID?

@marcpalmer
Copy link
Author

By this I mean, create a new NSURL relative the base NSURL that NSBundle originally returned, instead of doing string manipulations and asking NSBundle for a new URL all over again.

@groue
Copy link
Owner

groue commented Dec 5, 2014

It seems quite tricky to solve [...] We can't know safely which part of the URL NSBundle is the localisation part can we?

I had the same trend of thoughts :-) Yes it's tricky!

We should list all the use cases, given a sample hierarchy of resources:

o nonLocalized.mustache
o nonLocalizedSibling.mustache
o nonLocalizedTemplates
|--o a.mustache
|--o b.mustache
o en.lproj
|--o localized.mustache
|--o localizedSibling.mustache
|--o localizedTemplates
|  |--o a.mustache
|  |--o b.mustache
o fr.lproj
|--o localized.mustache
|--o localizedSibling.mustache
|--o localizedTemplates
|  |--o a.mustache
|  |--o b.mustache

MUST HAVE: Siblings - in the same realm

[GRMustacheTemplate templateFromResource:@"nonLocalized"] should be able to load:

- `{{> nonLocalizedSibling }}` /nonLocalizedSibling.mustache

[GRMustacheTemplate templateFromResource:@"nonLocalizedTemplates/a"] should be able to load:

- `{{> b }}` /nonLocalizedTemplates/b.mustache

[GRMustacheTemplate templateFromResource:@"localized"] should be able to load:

- `{{> localizedSibling }}` /en.lproj/localizedSibling.mustache

[GRMustacheTemplate templateFromResource:@"localizedTemplates/a"] should be able to load:

- `{{> b }}` /en.lproj/localizedTemplates/b.mustache

SHOULD HAVE: Siblings - accross realms (localized vs. non localized)

[GRMustacheTemplate templateFromResource:@"nonLocalized"] should be able to load:

- `{{> localized }}` /en.lproj/localized.mustache

[GRMustacheTemplate templateFromResource:@"localized"] should be able to load:

- `{{> nonLocalized }}` /nonLocalized.mustache

MUST HAVE: Absolute paths - in the same realm (localized vs. non localized)

[GRMustacheTemplate templateFromResource:@"nonLocalized"] should be able to load:

- `{{> /nonLocalizedSibling }}` /nonLocalizedSibling.mustache
- `{{> /nonLocalizedTemplates/b }}` /nonLocalizedTemplates/b.mustache

[GRMustacheTemplate templateFromResource:@"nonLocalizedTemplates/a"] should be able to load:

- `{{> /nonLocalized }}` /nonLocalized.mustache
- `{{> /nonLocalizedTemplates/b }}` /nonLocalizedTemplates/b.mustache

[GRMustacheTemplate templateFromResource:@"localized"] should be able to load:

- `{{> /localizedSibling }}` /en.lproj/localizedSibling.mustache
- `{{> /localizedTemplates/a }}` /en.lproj/localizedTemplates/a.mustache

[GRMustacheTemplate templateFromResource:@"localizedTemplates/a"] should be able to load:

- `{{> /localized }}` /en.lproj/localized.mustache
- `{{> /localizedTemplates/b }}` /en.lproj/localizedTemplates/b.mustache

MUST HAVE: Relative navigation - in the same realm (localized vs. non localized)

[GRMustacheTemplate templateFromResource:@"nonLocalized"] should be able to load:

- `{{> nonLocalizedTemplates/b }}` /nonLocalizedTemplates/b.mustache

[GRMustacheTemplate templateFromResource:@"nonLocalizedTemplates/a"] should be able to load:

- `{{> ../nonLocalized }}` /nonLocalized.mustache
- `{{> /nonLocalized }}` /nonLocalized.mustache

[GRMustacheTemplate templateFromResource:@"localized"] should be able to load:

- `{{> localizedTemplates/a }}` /en.lproj/localizedTemplates/a.mustache

[GRMustacheTemplate templateFromResource:@"localizedTemplates/a"] should be able to load:

- `{{> ../localized }}` /en.lproj/localized.mustache
- `{{> /localized }}` /en.lproj/localized.mustache

SHOULD HAVE: Absolute paths - accross realms (localized vs. non localized)

[GRMustacheTemplate templateFromResource:@"nonLocalized"] should be able to load:

- `{{> /localized }}` /en.lproj/localized.mustache
- `{{> /localizedTemplates/a }}` /en.lproj/localizedTemplates/a.mustache

[GRMustacheTemplate templateFromResource:@"nonLocalizedTemplates/a"] should be able to load:

- `{{> /localized }}` /en.lproj/localized.mustache
- `{{> /localizedTemplates/a }}` /en.lproj/localizedTemplates/a.mustache

[GRMustacheTemplate templateFromResource:@"localized"] should be able to load:

- `{{> /nonLocalized }}` /nonLocalized.mustache
- `{{> /nonLocalizedTemplates/b }}` /nonLocalizedTemplates/b.mustache

[GRMustacheTemplate templateFromResource:@"localizedTemplates/a"] should be able to load:

- `{{> /nonLocalized }}` /nonLocalized.mustache
- `{{> /nonLocalizedTemplates/b }}` /nonLocalizedTemplates/b.mustache

NOT SURE: Relative navigation - accross realms (localized vs. non localized)

[GRMustacheTemplate templateFromResource:@"nonLocalized"] should be able to load:

- `{{> localizedTemplates/a }}` /en.lproj/localizedTemplates/a.mustache

[GRMustacheTemplate templateFromResource:@"nonLocalizedTemplates/a"] should be able to load:

- `{{> ../localized }}` /en.lproj/localized.mustache

[GRMustacheTemplate templateFromResource:@"localized"] should be able to load:

- `{{> nonLocalizedTemplates/b }}` /nonLocalizedTemplates/b.mustache

[GRMustacheTemplate templateFromResource:@"localizedTemplates/a"] should be able to load:

- `{{> ../nonLocalized }}` /nonLocalized.mustache
- `{{> ../nonLocalizedTemplates/b }}` /nonLocalizedTemplates/b.mustache

@groue
Copy link
Owner

groue commented Dec 5, 2014

I should have a thourough review of those cases, decide whether they are valid or not, and turn them into tests. I'm not sure yet of the "MUST HAVE", "SHOULD HAVE", "NOT SURE", mentions as well.

@groue
Copy link
Owner

groue commented Dec 10, 2014

@marcpalmer I can't write any regression test.

Given the bundle:

o /path/to/bundle
|--o en.lproj
|  |--o Main.mustache      "{{> Footer }}"
|  |--o Footer.mustache    "Footer"

The following code runs fine and renders the expected content.

NSBundle *bundle = [NSBundle bundleWithPath:@"/path/to/bundle"];
GRMustacheTemplate *template = [GRMustacheTemplate templateFromResource:@"Main" bundle:bundle error:NULL];
NSString *rendering = [template renderObject:nil error:NULL];
XCTAssertEqualObjects(rendering, @"Footer");

What do I miss?

@marcpalmer
Copy link
Author

Hmmm... the only difference I think is the bundle mechanism, which is likely the problem?

Our real code uses

        template = [GRMustacheTemplate templateFromResource:name bundle:[NSBundle mainBundle] error:&error];

@groue
Copy link
Owner

groue commented Dec 10, 2014

Damn. How should I write the test, then 😣 ?

@marcpalmer
Copy link
Author

I don't know, but I'll try to build you a quick sample project to prove it

@groue
Copy link
Owner

groue commented Dec 10, 2014

Well, why not, thanks. I'm trying to fetch documentation about NSBundle.

@marcpalmer
Copy link
Author

FYI I can't reproduce this in isolation either now. I'm trying to recreate the problem in my working app (after I flattened my templates dir to not use localisations to get past this)... I think I must have had a problem with resource paths. I will let you know - don't work on it any more for now.

@marcpalmer
Copy link
Author

Gwendal - please accept my apologies. I have investigated further and it looks like it must be the case that my footer template was not in the /en.lproj/ folder in the bundle, but the main template was. I have re-added them all to the correct folders and it is working currently.

I am really sorry for wasting your time. I was tricked by the Xcode resource grouping not mapping to the folder location on disk for that file :(

@marcpalmer
Copy link
Author

Although... I'm not sure this is 100% the case as I wonder if it will fall back from a footer in say "fr.lproj" to one in "Base.lproj".

@groue
Copy link
Owner

groue commented Dec 10, 2014

OK Marc, no problem. It happens all the time, and I'm the first to blame the third-party code when it happens 😉.

Yet, I know that my tests are lacking on bundle-based templates. As you say, some nasty edge cases my appear between localized, base-localized, non-localized resources, and today I don't have the slightest idea of the exact behavior of the library beyond the most simple cases!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants