Match file paths with spaces in caller regexp

[rwstauner]

When matching only \S+ I get warnings like this on my ci server:

Cannot digest non-existent file: '/tmp/space char20151201-28026-1sux619/some_ruby.rb20151201-28026-g481xg:1:in `<top (required)>''.
Please set `::_cache_digest` of the serializer
if you'd like to cache it.

Matching the trailing space after :\d+:in wasn't strictly necessary, but seemed like it would help it to be more accurate.

[bf4]

duplicate : #1355

[rwstauner]

rats. i searched for the "cache" warning but i guess i didn't search for "spaces".

[bf4]

you solved it somewhat differently. would you like to merge in changes from the other (mine), or have me add some changes? I don't have failing tests for the difference between the two regexen

[bf4]

(As a maintainer, I'd rather someone else get credit...)

[beauby]

Great, I was hoping someone would fix the spaces situation. Thanks 👍

[rwstauner]

After fiddling with some extra tests I think your pattern is more accurate.

My original thinking was that the string after the file path (an error message) could include a substring match.
But in this case you aren't matching errors, you're matching an explicit caller line, so I assume the ending will only be something like <top (required)>'` (or

'`),
in which case the greedy regexp is more correct.

An example that passes for non-greedy and fails for greedy:

def test_serializer_file_path_with_path_in_string
  # $ ruby =(echo $'def foo(s); raise s; end;\nfoo("file:123:in string")')
  # /tmp/zshr2gfAE:1:in `foo': file:123:in string (RuntimeError)
  #         from /tmp/zshr2gfAE:2:in `<main>'
  path = '/Users/git/ember js/ember-crm-backend/app/serializers/lead_serializer.rb'
  caller_line = "#{path}:1:in `foo': file:123:in string (RuntimeError)"
  assert_equal path, caller_line[ActiveModel::Serializer::CALLER_FILE]
end

but as I said, I don't think that's appropriate in this case.

So I adjusted mine, added your test, and added an additional (more convoluted) test to confirm the difference between the two. Use whichever pull req makes sense.

[bf4]

If someone has that in their file path I pity them 😺

[rwstauner]

Yes, quite.

In my case the file path was generated by another process with apparently little sanitation on the naming ;-)

[bf4]

@rwstauner looks good. Could you add a changelog entry as a Fix?

I have a few other comments but nothing big.

I keep thinking there should be a way for the subclass to calculate its own digest outside of the inherited hook, which would be simpler, no? I think the method would just need to be dynamically defined define_singleton_method(:cache_digest) { MD5::Digest.hexdigest(__FILE__) } or something like that, right?

[rwstauner]

Sounds like a good idea, but we'd have to find another way to get the file from where the class is inheriting (the caller) since the value of __FILE__ is based on where the token is present.
And class_eval "..." isn't any better (__FILE__ evaluates to (eval)).

[bf4]

Closes #1355

[rwstauner]

Thanks!