Propagate cache stats from ets operations and fix dialyzer issues

[matyasmarkovics]

Functional

• Remove the special case of operate_cache/1 where {increase_stat, StatName} would be used to bump-up the cache-statistic for the named operation.
• Ensure instead that the 3-tuple: {increase_stat, StatName, Increment} is used for every cache-operation. The actual Increment then can be propagated from the ets operations (or just defined as 1 for a singular change).

Type-checking

• Define cache-operation function type
• Use arguments known at compile-time when applying cache-operation function(s) (more efficient and better written).
• Fix dialyzer issues within the erl_cache_server module
• Fix no-local-return of decorator: cache_pt/3
• Fix incorrect type-descriptions in the erl_cache module

Non-code

• Ignore test artefacts

[matyasmarkovics]

@kkuzmin @motobob and @raghavkarol Please review these changes and provide feedback where necessary. Much appreciated.

[raghavkarol]

Looks OK when eyeballing, but since I don't maintain I can not say if the tests cover all the code touched affected here and am wary of the side effects.

I suggest that instead of merging this to master we leave this change on a branch, pin AE to use that branch and then after some time merge to master.

[matyasmarkovics]

Let me see, maybe I can get test coverage.

[raghavkarol]

Let me see, maybe I can get test coverage.

+1, even with test coverage I would not be completely confident merging this to master right away.

[matyasmarkovics]

module coverage %
erl_cache 88%
erl_cache_app 100%
erl_cache_decorator 88%
erl_cache_driver 0%
erl_cache_key_generator 100%
erl_cache_server 95%
erl_cache_server_sup 100%
erl_cache_sup 100%
Total 89%

[motobob]

@carlisom @rmpalomino please review and merge

[rmpalomino]

It doesn't look like this introduces any breaking changes. I'm not a fan of the way the function type specs were changed, now there is a combination of foo(term(), term()) -> term() and foo(Arg1, Arg2) -> Result when Arg1 :: term()..., I would prefer it to be done the original way since you didn't change the other existing type specs.

[rmpalomino]

Why was this changed?

[matyasmarkovics]

To make use of operate_cache/5.

[matyasmarkovics]

I'm not a fan of the way the function type specs were changed, now there is a combination of foo(term(), term()) -> term() and foo(Arg1, Arg2) -> Result when Arg1 :: term()...,

The way of "type-spec"-ing you've listed first becomes unreadable very quickly and also it would raise the question of where to inject line-breaks. I challenge you to list at least 3 developers who agree on where to put line-breaks :), I think that problem has a NP complexity. This project is old enough to have no code-formatting tools enabled. So in order to preserve readability, my only option was to introduce type-args in the type-spec.

I would prefer it to be done the original way since you didn't change the other existing type specs.

Honestly that would be a tall order. It would be similar to reformatting the whole code-base with erlfmt. Lots of changes to review in both cases.

[matyasmarkovics]

FYI: While I can see that the PR has been approved, I cannot do anything else. - I have no permission the merge this. :|

[carlisom]

@matyasmarkovics, has this been tested pinned in AE as @raghavkarol suggested?