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

Unify usage of file helpers #566

Merged
merged 18 commits into from
Feb 13, 2025

Conversation

sdvendramini
Copy link
Member

@sdvendramini sdvendramini commented Jan 31, 2025

Description

Closes #550

The proposal is to unify the use of all functions related to files in a file_handler libray, where the implementation of filesystem-related functions is separated from IO operation.

Proposed Changes

File_helper is created, which is composed of the FilesystemWrapper and FileIO libraries. These contain functions that wrap standard functions and others that perform operations related to files and their directories.

In summary:

  • The IFileIO interface is created and IFileSystem is moved to file_helper.
  • Classes implementing these interfaces are created based on previous developments that were scattered in different locations.
  • Wrappers are added to make the code more testable.
  • Unnecessary functions are removed.
  • Tests are added.

Results and Evidence

  • E2E tests

    Inventory - Network

    1

    Inventory - RPM Package

    2

    Inventory - NPM Package

    3

    Inventory - PYPI Package

    4

    Inventory - Hardware

    5

    Inventory - Process Info

    6

  • Unit tests

    Filesystem tests

    image

    FileIO tests

    image

  • Manual tests for macOS

    Inventory - Hardware
    • main branch

    image

    • PR branch

    image

    Inventory - Package
    • main branch

    image

    • PR branch

    image

Artifacts Affected

Executable files, all platforms.

Configuration Changes

None

Documentation Updates

None

Tests Introduced

Added:

  • file_io_test

Moved:

  • filesystem_test

Fixed:

  • sysInfoPackageLinuxParserRPM_test
  • sysInfoPackagesNPM_test
  • sysInfoPackagesPYPI_test

Review Checklist

  • Code changes reviewed
  • Relevant evidence provided
  • Tests cover the new functionality
  • Configuration changes documented
  • Developer documentation reflects the changes
  • Meets requirements and/or definition of done
  • No unresolved dependencies with other issues
  • ...

@sdvendramini sdvendramini self-assigned this Jan 31, 2025
@sdvendramini sdvendramini linked an issue Jan 31, 2025 that may be closed by this pull request
4 tasks
@sdvendramini sdvendramini force-pushed the enhancement/550-unify-usage-of-file-helpers branch from b8fe1c0 to c1b3dde Compare January 31, 2025 09:07
@sdvendramini sdvendramini mentioned this pull request Jan 31, 2025
4 tasks
@sdvendramini sdvendramini force-pushed the enhancement/550-unify-usage-of-file-helpers branch 16 times, most recently from a5480ee to dc7a627 Compare February 6, 2025 12:06
@sdvendramini sdvendramini force-pushed the enhancement/550-unify-usage-of-file-helpers branch from 5da3c52 to 5e24022 Compare February 11, 2025 08:05
The functions is_regular_file and directory_iterator have been moved to the new
library `file_handler`. Since the other functions in `fileHelper` were
redundant, this file has been removed, and all its usages have been replaced
with the new library. Additionally, the moved functions have been added to the.
filesystem mock used in the centralized configuration tests.
The function is moved to the new file_handler, renamed to expand_absolute_path,
all its usages are replaced, and associated tests are fixed.
The function is moved to the new file_handler, renamed to enumerate_dir,
all its usages are replaced.
The class is created to host the method getRpmInfo. This way, it will be
possible to mock the filesystem of file_handler and thus remove the use of
existsRegular function.
This function has been removed, and its usage has been replaced by functions
already present in the library. This results in simpler code. Additionally, the
necessary changes have been made to ensure that the tests run successfully.
This class and its readLineByLinea method have been moved to the new library,
and the necessary changes have been made to ensure that the tests run
successfully.
These functions have been moved to the new FileIO class in file_handler. Their
usages have been replaced, and new functions have been documented. Additionally,
an interface has been added to allow mocking the functions in future tests.
The implementation and name of this method have been changed to return a vector
instead of an iterator. This makes its use in tests and the way the method is
mocked much simpler. Its usages have been replaced and tests fixed.
The remaining fileHelper files have been removed. The remaining tests for the
expand_absolute_path function have been moved. A call to configure_target has
been added in CMakeLists.txt file, which required some changes to avoid warnings
and compilation errors.
@sdvendramini sdvendramini force-pushed the enhancement/550-unify-usage-of-file-helpers branch from 5e24022 to 3c5b884 Compare February 11, 2025 08:06
sdvendramini and others added 5 commits February 11, 2025 15:27
The enumerate_dir function has been removed and its usages replaced with
list_directory. To do so, some checks have been added in the code and the format
of the returned values has been changed.
Methods have been added to the ifile_io interface to allow testing of the FileIO
class functions. A mock has been added within the test file for internal
testing, and the MockFileIO class has been completed for other tests that
require it as an external dependency.
Pure abstract base class was missing a = 0 in create_ifstream
@sdvendramini sdvendramini force-pushed the enhancement/550-unify-usage-of-file-helpers branch from 1ca7dd8 to 1854779 Compare February 12, 2025 08:24
@sdvendramini sdvendramini marked this pull request as ready for review February 12, 2025 09:54
This class member is added to avoid using std::filesystem in the class
implementation.
@TomasTurina TomasTurina force-pushed the enhancement/550-unify-usage-of-file-helpers branch from 26ee55f to 0bc4293 Compare February 12, 2025 18:48
Copy link
Member

@jr0me jr0me left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cborla
Copy link
Member

cborla commented Feb 13, 2025

Good job!
The changes look good, as it affects several modules, I would like to see some examples of inventory tables for the 3 available OS.

@TomasTurina TomasTurina merged commit beb78f2 into main Feb 13, 2025
5 checks passed
@TomasTurina TomasTurina deleted the enhancement/550-unify-usage-of-file-helpers branch February 13, 2025 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify usage of file helpers
4 participants