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

Extend MsappArchive APIs #675

Merged
merged 3 commits into from
Jun 25, 2024
Merged

Conversation

joem-msft
Copy link
Contributor

This PR moves some new APIs used in the DocSvr code into the MsappArchive and IMsappArchive APIs in this repo.

  • DoesEntryExist - allows callers to determine whether an entry exists w/o needing to inspect internal canonical dictionary.
  • TryGenerateUniqueEntryPath - exposes logic for generating a unique entry, in case similar entries already exist. This function is similar to the private method GetSafeEntryPath, but this method only has a single responsibility, in order to make it more usable for different scenarios.
  • MsappArchive.TryMakeSafeForEntryPathSegment is a new static method that has a single responsibility, allowing it's usage to be composed with other functions depending on the scenario. It is similar to encoding a path segment of a URI. Namely, the char set it allows is anything that could be a valid file name or directory name.
  • GetSafeEntryPath is modified to use the new TryGenerateUniqueEntryPath and TryMakeSafeForEntryPathSegment methods.
  • The GetEntry, GetRequiredEntry methods were updated to utilize the TryGetEntry method.

Validation:

  • Added additional unit tests for the three new methods
  • Updated existing test output due to '.' chars now being a valid file/directory name.

@joem-msft joem-msft requested review from a team as code owners June 21, 2024 05:02
Copy link
Contributor

@mizrael mizrael left a comment

Choose a reason for hiding this comment

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

do we really need to add these methods to the interface? can't we have them exposed as extension methods instead?

@joem-msft joem-msft force-pushed the users/joemay/MsappArchive-improvements branch 3 times, most recently from 1f22180 to 0ccc194 Compare June 22, 2024 02:11
@petrochuk petrochuk dismissed their stale review June 24, 2024 12:58

not needed

src/Persistence/MsApp/MsappArchive.cs Outdated Show resolved Hide resolved
src/Persistence/MsApp/MsappArchive.cs Outdated Show resolved Hide resolved
@joem-msft joem-msft force-pushed the users/joemay/MsappArchive-improvements branch from 0ccc194 to 3418f8c Compare June 25, 2024 00:04
@joem-msft joem-msft force-pushed the users/joemay/MsappArchive-improvements branch from 3418f8c to 6525724 Compare June 25, 2024 00:14
@joem-msft joem-msft enabled auto-merge (squash) June 25, 2024 17:45
@joem-msft joem-msft merged commit d135c78 into master Jun 25, 2024
4 checks passed
@joem-msft joem-msft deleted the users/joemay/MsappArchive-improvements branch June 25, 2024 18:03
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.

5 participants