iox-#1036 builder pattern for file lock

[elfenpiff]

Pre-Review Checklist for the PR Author

1. Code follows the coding style of CONTRIBUTING.md
2. Tests follow the best practice for testing
3. Changelog updated in the unreleased section including API breaking changes
4. Branch follows the naming format (iox-#123-this-is-a-branch)
5. Commits messages are according to this guideline
   • Commit messages have the issue ID (iox-#123 commit text)
   • Commit messages are signed (git commit -s)
   • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
6. Update the PR title
   • Follow the same conventions as for commit messages
   • Link to the relevant issue
7. Relevant issues are linked
8. Add sensible notes for the reviewer
9. All checks have passed (except task-list-completed)
10. Assign PR to reviewer

Notes for Reviewer

The FileLock had some issues. For instance it never verified that the path prefix was a valid path (syntax) and it hardcoded the permissions. I added those as builder parameters and verified them.
To verify the path I had to add a variation of isValidFilePath called isValidPath which just does not check if the last character is a separator. The tests are also more or less the same with some minor addition (adding a last slash).

Checklist for the PR Reviewer

• Commits are properly organized and messages are according to the guideline
• Code according to our coding style and naming conventions
• Unit tests have been written for new behavior
  • Each unit test case has a unique UUID
• Public API changes are documented via doxygen
• Copyright owner are updated in the changed files
• PR title describes the changes

Post-review Checklist for the PR Author

1. All open points are addressed and tracked via issues

References

• Closes Remove creation design pattern class with in place implementation #1036

[codecov]

Codecov Report

Merging #1418 (7e7be2f) into master (4763bd9) will decrease coverage by 0.02%.
The diff coverage is 63.55%.

❗ Current head 7e7be2f differs from pull request most recent head 5274023. Consider uploading reports for the commit 5274023 to get more accurate results

@@            Coverage Diff             @@
##           master    #1418      +/-   ##
==========================================
- Coverage   78.83%   78.80%   -0.03%     
==========================================
  Files         378      377       -1     
  Lines       14579    14503      -76     
  Branches     2027     2020       -7     
==========================================
- Hits        11493    11429      -64     
  Misses       2432     2432              
+ Partials      654      642      -12     

Flag	Coverage Δ
unittests	78.46% <63.55%> (-0.03%)	⬇️
unittests_timing	14.92% <13.08%> (+0.11%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...eoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp	97.29% <ø> (ø)
...oryx_posh/include/iceoryx_posh/roudi/roudi_app.hpp	100.00% <ø> (ø)
iceoryx_hoofs/source/posix_wrapper/file_lock.cpp	39.72% <41.79%> (-0.69%)	⬇️
...fs/include/iceoryx_hoofs/internal/cxx/helplets.inl	98.27% <100.00%> (+4.52%)	⬆️
.../include/iceoryx_hoofs/posix_wrapper/file_lock.hpp	100.00% <100.00%> (ø)
..._hoofs/source/posix_wrapper/unix_domain_socket.cpp	63.20% <100.00%> (ø)
...posh/roudi/memory/iceoryx_roudi_memory_manager.hpp	22.22% <100.00%> (+9.72%)	⬆️
...oryx_posh/source/runtime/ipc_interface_creator.cpp	50.00% <100.00%> (+7.89%)	⬆️
iceoryx_posh/source/runtime/posh_runtime.cpp	55.26% <0.00%> (-7.90%)	⬇️
...sh/include/iceoryx_posh/internal/popo/wait_set.inl	91.17% <0.00%> (ø)
... and 8 more

[elBoberido]

I did not yet review the new tests for the helplets

[elBoberido]

helplets tests still not reviewed

[elBoberido]

Does it mean that removing :_ from the end makes this path invalid?

[elfenpiff]

Nope, maybe the name is misleading. I just wanted to pack all valid characters in one string and check if the string is valid.

Why did you think that removing :_ would make the path invalid?

[elBoberido]

Because the FilePathsWithEndingDotsAreInvalid test suggest that this is the case since the path would end with a dot.

[elfenpiff]

Ah of course then you are right.

The validation of the file names, paths and entries should work platform independently and therefore I restricted the names a bit to support some more file systems or let's say weird applications.

For instance, NTFS supports file names which end with a dot (blaa.) but the windows file explorer does not. So you can see the file but not open, modify or delete it since it is not supported by the Windows API. Yeah.

[elBoberido]

It's similar with .blaa. You cannot create it but AFAIK open, modify and delete it with the file explorer.

[elBoberido]

Almost there

[elBoberido]

I like this.

Disclaimer: I don't suggest to do this in this PR

It would be great to have this formalized and checked by the CI. A a script ninja could create a script to check for a specific syntax for our deprecation attributes. In combination with the VERSION file, this could tell us that there are deprecated functions for the new release and we need to remove them.

Do you think it might make sense to also tell the user which release introduce the deprecation? Just in case we have something that will be remove in next+2, e.g.
[[deprecated("in 3.0, removed in 6.0, use foo::bar()")]]