Simplify attach_canister with early returns (#6029)

# Motivation

The `attach_canister` function in the backend is unnecessarily much
indented.
This can be avoided by inverting 2 `if`s and using early returns.

Make sure to enable "ignore whitespace" when reviewing.

# Changes

1. Change
```
if Self::validate_canister_name(&request.name) {
    ...
} else {
    AttachCanisterResponse::NameTooLong
}
```
to
```
if !Self::validate_canister_name(&request.name) {
    return AttachCanisterResponse::NameTooLong;
}
...
```
2. Change
```
if let Some(mut account) = self.accounts_db.db_get_account(&account_identifier) {
    ...
} else {
    AttachCanisterResponse::AccountNotFound
}
```
to
```
let Some(mut account) = self.accounts_db.db_get_account(&account_identifier) else {
    return AttachCanisterResponse::AccountNotFound;
};
...
```


# Tests

Added a missing test for `AttachCanisterResponse::AccountNotFound`.

# Todos

- [ ] Add entry to changelog (if necessary).
not necessary

rs/backend/src/accounts_store.rs

@@ -625,50 +625,50 @@ impl AccountsStore {
     }
 
     pub fn attach_canister(&mut self, caller: PrincipalId, request: AttachCanisterRequest) -> AttachCanisterResponse {
-        if Self::validate_canister_name(&request.name) {
-            let account_identifier = AccountIdentifier::from(caller).to_vec();
+        if !Self::validate_canister_name(&request.name) {
+            return AttachCanisterResponse::NameTooLong;
+        }
 
-            if let Some(mut account) = self.accounts_db.db_get_account(&account_identifier) {
-                let mut index_to_remove: Option<usize> = None;
-                for (index, c) in account.canisters.iter().enumerate() {
-                    if !request.name.is_empty() && c.name == request.name {
-                        return AttachCanisterResponse::NameAlreadyTaken;
-                    }
-                    // The periodic_task_runner might attach the canister before this call.
-                    // The canister attached by the periodic_task_runner has name `""`
-                    if c.canister_id == request.canister_id {
-                        if c.name.is_empty() && !request.name.is_empty() {
-                            index_to_remove = Some(index);
-                        } else {
-                            return AttachCanisterResponse::CanisterAlreadyAttached;
-                            // Note: It might be nice to tell the user the name of the existing canister.
-                        }
-                    }
-                }
+        let account_identifier = AccountIdentifier::from(caller).to_vec();
 
-                if let Some(index) = index_to_remove {
-                    // Remove the previous attached canister before reattaching.
-                    account.canisters.remove(index);
-                }
+        let Some(mut account) = self.accounts_db.db_get_account(&account_identifier) else {
+            return AttachCanisterResponse::AccountNotFound;
+        };
 
-                if account.canisters.len() >= u8::MAX as usize {
-                    return AttachCanisterResponse::CanisterLimitExceeded;
+        let mut index_to_remove: Option<usize> = None;
+        for (index, c) in account.canisters.iter().enumerate() {
+            if !request.name.is_empty() && c.name == request.name {
+                return AttachCanisterResponse::NameAlreadyTaken;
+            }
+            // The periodic_task_runner might attach the canister before this call.
+            // The canister attached by the periodic_task_runner has name `""`
+            if c.canister_id == request.canister_id {
+                if c.name.is_empty() && !request.name.is_empty() {
+                    index_to_remove = Some(index);
+                } else {
+                    return AttachCanisterResponse::CanisterAlreadyAttached;
+                    // Note: It might be nice to tell the user the name of the existing canister.
                 }
-                account.canisters.push(NamedCanister {
-                    name: request.name,
-                    canister_id: request.canister_id,
-                });
-                account.canisters.sort();
+            }
+        }
 
-                self.accounts_db.db_insert_account(&account_identifier, account);
+        if let Some(index) = index_to_remove {
+            // Remove the previous attached canister before reattaching.
+            account.canisters.remove(index);
+        }
 
-                AttachCanisterResponse::Ok
-            } else {
-                AttachCanisterResponse::AccountNotFound
-            }
-        } else {
-            AttachCanisterResponse::NameTooLong
+        if account.canisters.len() >= u8::MAX as usize {
+            return AttachCanisterResponse::CanisterLimitExceeded;
         }
+        account.canisters.push(NamedCanister {
+            name: request.name,
+            canister_id: request.canister_id,
+        });
+        account.canisters.sort();
+
+        self.accounts_db.db_insert_account(&account_identifier, account);
+
+        AttachCanisterResponse::Ok
     }
 
     pub fn rename_canister(&mut self, caller: PrincipalId, request: RenameCanisterRequest) -> RenameCanisterResponse {

rs/backend/src/accounts_store/tests.rs

@@ -233,6 +233,23 @@ fn attach_canister_name_too_long() {
     assert!(matches!(result2, AttachCanisterResponse::NameTooLong));
 }
 
+#[test]
+fn attach_canister_account_not_found() {
+    let mut store = setup_test_store();
+    let principal = PrincipalId::from_str(TEST_ACCOUNT_3).unwrap();
+    let canister_id = CanisterId::from_str(TEST_ACCOUNT_2).unwrap();
+
+    let result = store.attach_canister(
+        principal,
+        AttachCanisterRequest {
+            name: "".to_string(),
+            canister_id,
+        },
+    );
+
+    assert!(matches!(result, AttachCanisterResponse::AccountNotFound));
+}
+
 #[test]
 fn attach_canister_canister_already_attached() {
     let mut store = setup_test_store();