github-starred/android
2
0
Fork 0
mirror of https://github.com/bitwarden/android.git synced 2026-07-10 22:33:31 -05:00
Code Issues 2.4k Packages Projects Releases 129 Wiki Activity

[PR #2443] [MERGED] [PM-1402] Refactor PasswordGenerationService alongside PolicyService #3569

New Issue
Closed
opened 2025-11-26 23:34:32 -06:00 by GiteaMirror · 0 comments
GiteaMirror commented 2025-11-26 23:34:32 -06:00
Owner
Copy Link

📋 Pull Request Information

Original PR: https://github.com/bitwarden/android/pull/2443
Author: @fedemkr
Created: 3/24/2023
Status: ✅ Merged
Merged: 5/11/2023
Merged by: @fedemkr

Base: master ← Head: PM-1402-refactor-passgenservice-policy

📝 Commits (9)

  • aa6030c PM-1402 Refactor pass generation service alongside policyservice
  • 474d129 PM-1402 Refactor PasswordGenerationService and PolicyService to have a simpler code and more specific to each class
  • f2b97fa Merge branch 'master' into PM-1402-refactor-passgenservice-policy
  • 498a7a3 PM-1402 Fix format
  • f050d64 Merge branch 'master' into PM-1402-refactor-passgenservice-policy
  • a094d29 PM-1402 Moved policy consts from PolicyService to Policy
  • 976c50e PM-1402 fix crash due to lack of null checking
  • 378b10f PM-1402 fix format
  • 99e8529 PM-1402 removed GetValueOrDefault() given that it was not needed and was changing the behavior

📊 Changes

12 files changed (+282 additions, -352 deletions)

View changed files

📝 src/App/Pages/Accounts/LoginSsoPageViewModel.cs (+2 -3)
📝 src/App/Pages/Generator/GeneratorPageViewModel.cs (+1 -1)
📝 src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs (+7 -7)
📝 src/Core/Abstractions/IPasswordGenerationService.cs (+0 -2)
📝 src/Core/Abstractions/IPolicyService.cs (+1 -2)
📝 src/Core/Models/Domain/PasswordGenerationOptions.cs (+97 -21)
📝 src/Core/Models/Domain/Policy.cs (+32 -0)
📝 src/Core/Services/AuthService.cs (+1 -1)
📝 src/Core/Services/PasswordGenerationService.cs (+37 -246)
📝 src/Core/Services/PolicyService.cs (+102 -67)
📝 src/Core/Services/VaultTimeoutService.cs (+1 -0)
📝 src/Core/Utilities/ServiceContainer.cs (+1 -2)

📄 Description

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Simplify PasswordGenerationService and PolicyService and reduce some unneeded allocations.

Code changes

  • PasswordGenerationOptions: Moved default creation of it to be a static property and also added a fluent method to set the Length which is the common use case. Also added some constants to differentiate the types. And moved logic from the PasswordGenerationService to here to enforce policies for password generation.
  • Policy: Moved methods to get policy data values here from the services. This is the information expert so the methods make more sense to be here. Also simplified how these methods work: GetInt(...), GetString(...), GetBool(...). This also allows not needing a service to get a data value of a policy.
  • PasswordGenerationService:
    • Renamed constants to use uppercase.
    • Simplified the methods to remove unnecessary null checks
    • Removed string allocations using StringBuilder instead.
    • Renamed some variables that IMO were not that explicit at what they were.
    • Simplified GetOptionsAsync() moving GetPasswordGeneratorPolicyOptionsAsync(...) to the PolicyService which IMO is the correct place for it to be. And also removed EnforcePasswordGeneratorPoliciesOnOptionsAsync(...) given that the logic has been moved to PasswordGenerationOptions which is easier to use.
    • Removed unnecessary methods to get policy data given that now they are in Policy directly
  • PolicyService:
    • Simplified filters
    • Removed unnecessary null checks
    • Removed unnecessary methods to get policy data given that now they are in Policy directly

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/bitwarden/android/pull/2443 **Author:** [@fedemkr](https://github.com/fedemkr) **Created:** 3/24/2023 **Status:** ✅ Merged **Merged:** 5/11/2023 **Merged by:** [@fedemkr](https://github.com/fedemkr) **Base:** `master` ← **Head:** `PM-1402-refactor-passgenservice-policy` --- ### 📝 Commits (9) - [`aa6030c`](https://github.com/bitwarden/android/commit/aa6030c3a498991f3491073fc350d6673328d75e) PM-1402 Refactor pass generation service alongside policyservice - [`474d129`](https://github.com/bitwarden/android/commit/474d129eecfea425bc9859c2f734df30b4d6905a) PM-1402 Refactor PasswordGenerationService and PolicyService to have a simpler code and more specific to each class - [`f2b97fa`](https://github.com/bitwarden/android/commit/f2b97fab5552201dc453e22288c6b4859a8c82e8) Merge branch 'master' into PM-1402-refactor-passgenservice-policy - [`498a7a3`](https://github.com/bitwarden/android/commit/498a7a32188210388bcb196b930e471658512688) PM-1402 Fix format - [`f050d64`](https://github.com/bitwarden/android/commit/f050d644659e073fb507a2c4d6218a0e2eca9170) Merge branch 'master' into PM-1402-refactor-passgenservice-policy - [`a094d29`](https://github.com/bitwarden/android/commit/a094d2993c089626610a0f8cbda97970a8055dcb) PM-1402 Moved policy consts from PolicyService to Policy - [`976c50e`](https://github.com/bitwarden/android/commit/976c50e306912aa3de4e02dddb948462a366daa9) PM-1402 fix crash due to lack of null checking - [`378b10f`](https://github.com/bitwarden/android/commit/378b10f621b94b503b51af7ff810cedbd165dc9d) PM-1402 fix format - [`99e8529`](https://github.com/bitwarden/android/commit/99e8529d9984c28cd28fdfd2dbf84923e70f31af) PM-1402 removed GetValueOrDefault() given that it was not needed and was changing the behavior ### 📊 Changes **12 files changed** (+282 additions, -352 deletions) <details> <summary>View changed files</summary> 📝 `src/App/Pages/Accounts/LoginSsoPageViewModel.cs` (+2 -3) 📝 `src/App/Pages/Generator/GeneratorPageViewModel.cs` (+1 -1) 📝 `src/App/Pages/Settings/SettingsPage/SettingsPageViewModel.cs` (+7 -7) 📝 `src/Core/Abstractions/IPasswordGenerationService.cs` (+0 -2) 📝 `src/Core/Abstractions/IPolicyService.cs` (+1 -2) 📝 `src/Core/Models/Domain/PasswordGenerationOptions.cs` (+97 -21) 📝 `src/Core/Models/Domain/Policy.cs` (+32 -0) 📝 `src/Core/Services/AuthService.cs` (+1 -1) 📝 `src/Core/Services/PasswordGenerationService.cs` (+37 -246) 📝 `src/Core/Services/PolicyService.cs` (+102 -67) 📝 `src/Core/Services/VaultTimeoutService.cs` (+1 -0) 📝 `src/Core/Utilities/ServiceContainer.cs` (+1 -2) </details> ### 📄 Description ## Type of change - [ ] Bug fix - [ ] New feature development - [X] Tech debt (refactoring, code cleanup, dependency upgrades, etc) - [ ] Build/deploy pipeline (DevOps) - [ ] Other ## Objective <!--Describe what the purpose of this PR is. For example: what bug you're fixing or what new feature you're adding--> Simplify `PasswordGenerationService` and `PolicyService` and reduce some unneeded allocations. ## Code changes <!--Explain the changes you've made to each file or major component. This should help the reviewer understand your changes--> <!--Also refer to any related changes or PRs in other repositories--> * **PasswordGenerationOptions:** Moved default creation of it to be a static property and also added a fluent method to set the `Length` which is the common use case. Also added some constants to differentiate the types. And moved logic from the `PasswordGenerationService` to here to enforce policies for password generation. * **Policy:** Moved methods to get policy data values here from the services. This is the information expert so the methods make more sense to be here. Also simplified how these methods work: `GetInt(...), GetString(...), GetBool(...)`. This also allows not needing a service to get a data value of a policy. * **PasswordGenerationService:** * Renamed constants to use uppercase. * Simplified the methods to remove unnecessary `null` checks * Removed string allocations using `StringBuilder` instead. * Renamed some variables that IMO were not that explicit at what they were. * Simplified `GetOptionsAsync()` moving `GetPasswordGeneratorPolicyOptionsAsync(...)` to the `PolicyService` which IMO is the correct place for it to be. And also removed `EnforcePasswordGeneratorPoliciesOnOptionsAsync(...)` given that the logic has been moved to `PasswordGenerationOptions` which is easier to use. * Removed unnecessary methods to get policy data given that now they are in `Policy` directly * **PolicyService:** * Simplified filters * Removed unnecessary `null` checks * Removed unnecessary methods to get policy data given that now they are in `Policy` directly ## Before you submit - Please check for formatting errors (`dotnet format --verify-no-changes`) (required) - Please add **unit tests** where it makes sense to do so (encouraged but not required) - If this change requires a **documentation update** - notify the documentation team - If this change has particular **deployment requirements** - notify the DevOps team --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2025-11-26 23:34:32 -06:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
app:authenticator
app:password-manager
bug
bug-passkey
customer-support
duplicate
enhancement
good first issue
help wanted
needs-reply
needs-response
pull-request
Mirrored from GitHub Pull Request
 question
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
GiteaMirror ninjasurge
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/android#3569
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?