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

Determine in file class if the file content is the same #14687

Merged
merged 4 commits into from Jul 22, 2019
Merged

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Jul 22, 2019

I will need this one in 3.11.0 ideally.

Moved the detection whether the file is up to date to a new method isFileContentSame(). It looks unneeded, but is useful when you have eg a class CdnFile {} and need to update the content of that file not only on one path, but also for other trusted hosts. Then you need to save the content basically again if just one of the files is out of date. This happens only when using a custom directory for tracker files.

eg /matomo/foo/$trustedHost/matomo.js
eg /matomo/foo/trusted.host.one/matomo.js
eg /matomo/foo/trusted.host.two/matomo.js
eg /matomo/foo/trusted.host.three/matomo.js

Then we don't just need to read/update/delete one file, but multiple times for each host.

For the same reason we need to return an array of which files were actually updated in the save() method. We might not just update the file in one path, but actually in multiple paths. And then we need to trigger an event for each updated file.

Ideally we would have this logic better in core somehow, but the way we use it is quite special and usually you wouldn't have a file per trusted host.

@tsteur tsteur added the Needs Review PRs that need a code review label Jul 22, 2019
@tsteur tsteur added this to the 3.11.0 milestone Jul 22, 2019
@tsteur
Copy link
Member Author

tsteur commented Jul 22, 2019

@mattab needing to merge this one to have it in the next release. It's covered by tests and only a very basic change.

@tsteur tsteur merged commit a6b6711 into 3.x-dev Jul 22, 2019
@tsteur tsteur deleted the filesamecontent branch July 22, 2019 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant