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

IBX-7485: Skipped files with corrupted filenames when loading and deleting #400

Merged
merged 3 commits into from
Jan 31, 2024

Conversation

barw4
Copy link
Member

@barw4 barw4 commented Jan 5, 2024

Proof of concept PR

Question Answer
JIRA issue IBX-7485
Type bug
Target Ibexa version v3.3
BC breaks no

This PR try/catches the CorruptedPathDetected exception originating from the flysystem/league package. Files will be treated as missing.

Another idea (Filesystem override #399):

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (master for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ezsystems/engineering-team).

@barw4 barw4 added Bug Something isn't working Ready for review labels Jan 5, 2024
@barw4 barw4 self-assigned this Jan 5, 2024
@barw4 barw4 changed the title IBX-7485: Skipped files with corrupted filenames when loading and del… IBX-7485: Skipped files with corrupted filenames when loading and deleting Jan 5, 2024
@barw4 barw4 requested a review from a team January 5, 2024 13:16
@barw4 barw4 marked this pull request as ready for review January 5, 2024 14:37
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

+1 to this over #399 but please work on integration coverage for this case.

Paths stored in the database can be corrupted by performing a query directly on a current connection. Every integration test case contains getDatabaseConnection() method.

@barw4 barw4 requested review from alongosz and a team January 30, 2024 00:45
@barw4 barw4 requested review from alongosz and a team January 30, 2024 14:28
Copy link

sonarcloud bot commented Jan 30, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@alongosz alongosz requested a review from a team January 30, 2024 14:50

$contentService->deleteContent($content->getVersionInfo()->getContentInfo());

// Expect no League\Flysystem\CorruptedPathDetected thrown
Copy link
Contributor

Choose a reason for hiding this comment

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

Assert no assertions are made here (unless I'm mistaken?).

Copy link
Member

Choose a reason for hiding this comment

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

Assert no assertions are made here (unless I'm mistaken?).

Either there are some implicit assertions performed in the background or we don't have strict config. We'll see how it behaves on a merge up.

Copy link

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

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

QA approved on IbexaDXP 3.3 commerce.

@alongosz alongosz merged commit 6e5fa6f into 1.3 Jan 31, 2024
24 checks passed
@alongosz alongosz deleted the ibx-7845-skip-corrupted-files-loading branch January 31, 2024 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working QA approved
Development

Successfully merging this pull request may close these issues.

7 participants