2
\$\begingroup\$

I've written a C# API using .NET 9 with the goal of managing a personal investment portfolio – that means,

  • adding funds
  • adding or removing transactions (for ETFs, ETCs, and usual funds)
  • retrieving statistics and analyze your portfolio.

I'd like to share the code for the portfolio management part. The user management / Identity code is implemented in a separate class library and is not included in this question, as it would make the post unnecessarily large.

I'm looking for feedback focused on CPU performance (bottlenecks, unnecessary computations) and best practices (mainly for learning purposes).

Note: Currently, the portfolios are stored in a JSON file because I don't have a local database setup. I'm aware that a real-world implementation would require a proper database.

Let's also assume that external users will really store their transactions using my API.

Porject structure

PortfolioController.cs

using Microsoft.AspNetCore.Mvc;
using FundAndEtfApi.Models;
using FundAndEtfApi.Services;
using Microsoft.AspNetCore.Authorization;

namespace FundAndEtfApi.Controllers
{
    [Authorize]
    [ApiController]
    [Route("api/[controller]")]
    public class PortfolioController : ControllerBase
    {
        private readonly PortfolioService _portfolioService;

        public PortfolioController(PortfolioService portfolioService)
        {
            this._portfolioService = portfolioService;
        }

        /// <summary>
        /// Gets all portfolio data
        /// </summary>
        [HttpGet]
        [Route("getPortfolio")]
        public async Task<ActionResult<Portfolio>> GetPortfolio()
        {
            Portfolio? portfolio = await _portfolioService.GetPortfolio(false);
            return Ok(portfolio);
        }

        /// <summary>
        /// Adds a newly purchased fund that you don't have, yet.
        /// </summary>
        /// <param name="newFund"></param>
        /// <returns></returns>
        [HttpPost]
        [Route("addFund")]
        public async Task<IActionResult> AddFund([FromBody] Fund newFund)
        {
            try
            {
                await _portfolioService.AddFund(newFund, false);
                return Created();
            }
            catch (ArgumentException ex)
            {
                return BadRequest(ex.Message);
            }
            catch (InvalidOperationException ex)
            {
                return BadRequest(ex.Message);
            }
        }

        [HttpPatch("{isin}/UpdateFundName")]
        public async Task<IActionResult> UpdateFundName(string isin, [FromBody] string newName)
        {
            try
            {
                bool updated = await _portfolioService.UpdateFundName(isin, newName, calledFromTest: false);
                return updated ? Ok("Fund name updated.") : Problem();
            }
            catch (ArgumentException ex)
            {
                return BadRequest(ex.Message);
            }
            catch (InvalidOperationException ex)
            {
                return BadRequest(ex.Message);
            }
        }

        [HttpDelete("{isin}/sellFund")]
        public async Task<IActionResult> RemoveFund(string isin)
        {
            try
            {
                bool removed = await _portfolioService.RemoveFundByIsinAndArchiveIt(isin, false);

                if (!removed)
                    return Problem($"Fund with ISIN \"{isin}\" was found but could not be deleted.");
            }
            catch (ArgumentException ex)
            {
                return BadRequest(ex.Message);
            }
            catch (InvalidOperationException ex)
            {
                return BadRequest(ex.Message);
            }

            return Ok();
        }

        [HttpGet("archivedFunds")]
        public async Task<IActionResult> GetArchivedFunds()
        {
            List<Fund> archivedFunds = await _portfolioService.GetDeletedFunds();
            return Ok(archivedFunds);
        }

        /// <summary>
        /// Adds a transaction (i.e., a purchase) to a fund that you already have.
        /// </summary>
        /// <param name="isin"></param>
        /// <param name="newTransaction"></param>
        /// <returns></returns>
        [HttpPost("{isin}/addTransaction")]
        public async Task<ActionResult<Guid>> AddTransaction(string isin, [FromBody] FundTransactionDto newTransaction)
        {
            try
            {
                Guid transactionGuid = await _portfolioService.AddTransactionToFund(isin, newTransaction, false);
                return Ok(transactionGuid);
            }
            catch (ArgumentException ex)
            {
                return BadRequest(ex.Message);
            }
            catch (InvalidOperationException ex)
            {
                return BadRequest(ex.Message);
            }
        }

        [HttpDelete("{isin}/transactions/{transactionId}/remove")]
        public async Task<IActionResult> DeleteTransaction(string isin, Guid transactionId)
        {
            try
            {
                bool removed = await _portfolioService.RemoveTransactionFromFund(isin, transactionId, false);
                if (!removed)
                    return NotFound("Transaction was found but could not be deleted.");

                return NoContent();
            }
            catch (ArgumentException ex)
            {
                return BadRequest(ex.Message);
            }
            catch (InvalidOperationException ex)
            {
                return BadRequest(ex.Message);
            }
        }

        [HttpGet("total-costs")]
        public async Task<IActionResult> GetTotalCosts()
        {
            try
            {
                decimal totalCosts = await _portfolioService.GetTotalCosts(false);
                return Ok(totalCosts);
            }
            catch (ArgumentException ex)
            {
                return BadRequest(ex.Message);
            }
        }

        [HttpGet("statistics")]
        public async Task<IActionResult> GetStatistics()
        {
            try
            {
                Models.Statistics.PortfolioStatistics stats = await _portfolioService.GetPortfolioStatistics(false);
                return Ok(stats);
            }
            catch (ArgumentException ex)
            {
                return BadRequest(ex.Message);
            }
        }
    }
}

Fund.cs

using Newtonsoft.Json;
using Newtonsoft.Json.Converters;

namespace FundAndEtfApi.Models
{
    public sealed class Fund
    {
        [JsonProperty("isin")]
        public string ISIN { get; private set; }

        [JsonProperty("name")]
        public string Name { get; internal set; }

        [JsonProperty("transactions")]
        public List<FundTransaction> Transactions { get; private set; }

        [JsonProperty("deletedAt")]
        public DateTime? DeletedAt { get; internal set; }

        /// <summary>
        /// Type of fund (how it is structured and managed)
        /// </summary>
        public enum Types
        {
            /// <summary>
            /// A professional fund management team selects the securities and reviews them regularly.
            /// </summary>
            ActiveFund,
            IndexFund,
            /// <summary>
            /// Exchange-Traded Fund, belongs to Index fund
            /// </summary>
            ETF,
            /// <summary>
            /// Exchange-Traded Commodity, not a fund
            /// </summary>
            ETC
        }

        [JsonProperty("type")]
        public Types Type { get; internal set; }

        public enum Sector
        {
            AI,
            Automotive,
            CommunicationServices,
            ConsumerDiscretionary, // nicht notwendige Konsumgüter wie z.B. Autos, Mode, Freizeit, Bekleidung, Reise
            ConsumerStaples, // Lebensmittel, Drogerieartikel
            Energy,
            Financials,
            GreenEnergy,
            HealthCare,
            Industrials, // Maschinenbau, Bauwesen, Logistik
            InformationTechnology,
            Materials, // Rohstoffe, auch Dünger
            RealEstate, // Immobilien
            Technology,
            Telecommunication,
            Utilities // Versorger (Gas, Wasser, Strom)
        }

        [JsonProperty("sectors")]
        public List<Sector> Sectors { get; private set; }

        /// <summary>
        /// ISO 3166-1
        /// </summary>
        public enum Country
        {
            Afghanistan,
            Albania,
            Algeria,
            Andorra,
            Angola,
            Antigua_and_Barbuda,
            Argentina,
            Armenia,
            Australia,
            Austria,
            Azerbaijan,
            Bahamas,
            Bahrain,
            Bangladesh,
            Barbados,
            Belarus,
            Belgium,
            Belize,
            Benin,
            Bhutan,
            Bolivia,
            Bosnia_and_Herzegovina,
            Botswana,
            Brazil,
            Brunei,
            Bulgaria,
            Burkina_Faso,
            Burundi,
            Cabo_Verde,
            Cambodia,
            Cameroon,
            Canada,
            Central_African_Republic,
            Chad,
            Chile,
            China,
            Colombia,
            Comoros,
            Congo_Brazzaville,
            Costa_Rica,
            Croatia,
            Cuba,
            Cyprus,
            Czech_Republic,
            Democratic_Republic_of_the_Congo,
            Denmark,
            Djibouti,
            Dominica,
            Dominican_Republic,
            Ecuador,
            Egypt,
            El_Salvador,
            Equatorial_Guinea,
            Eritrea,
            Estonia,
            Eswatini,
            Ethiopia,
            Fiji,
            Finland,
            France,
            Gabon,
            Gambia,
            Georgia,
            Germany,
            Ghana,
            Greece,
            Grenada,
            Guatemala,
            Guinea,
            Guinea_Bissau,
            Guyana,
            Haiti,
            Honduras,
            Hungary,
            Iceland,
            India,
            Indonesia,
            Iran,
            Iraq,
            Ireland,
            Israel,
            Italy,
            Jamaica,
            Japan,
            Jordan,
            Kazakhstan,
            Kenya,
            Kiribati,
            Kuwait,
            Kyrgyzstan,
            Laos,
            Latvia,
            Lebanon,
            Lesotho,
            Liberia,
            Libya,
            Liechtenstein,
            Lithuania,
            Luxembourg,
            Madagascar,
            Malawi,
            Malaysia,
            Maldives,
            Mali,
            Malta,
            Marshall_Islands,
            Mauritania,
            Mauritius,
            Mexico,
            Micronesia,
            Moldova,
            Monaco,
            Mongolia,
            Montenegro,
            Morocco,
            Mozambique,
            Myanmar,
            Namibia,
            Nauru,
            Nepal,
            Netherlands,
            New_Zealand,
            Nicaragua,
            Niger,
            Nigeria,
            North_Korea,
            North_Macedonia,
            Norway,
            Oman,
            Pakistan,
            Palau,
            Palestine_State,
            Panama,
            Papua_New_Guinea,
            Paraguay,
            Peru,
            Philippines,
            Poland,
            Portugal,
            Qatar,
            Romania,
            Russia,
            Rwanda,
            Saint_Kitts_and_Nevis,
            Saint_Lucia,
            Saint_Vincent_and_the_Grenadines,
            Samoa,
            San_Marino,
            Sao_Tome_and_Principe,
            Saudi_Arabia,
            Senegal,
            Serbia,
            Seychelles,
            Sierra_Leone,
            Singapore,
            Slovakia,
            Slovenia,
            Solomon_Islands,
            Somalia,
            South_Africa,
            South_Korea,
            South_Sudan,
            Spain,
            Sri_Lanka,
            Sudan,
            Suriname,
            Sweden,
            Switzerland,
            Syria,
            Taiwan,
            Tajikistan,
            Tanzania,
            Thailand,
            Timor_Leste,
            Togo,
            Tonga,
            Trinidad_and_Tobago,
            Tunisia,
            Turkey,
            Turkmenistan,
            Tuvalu,
            Uganda,
            Ukraine,
            United_Arab_Emirates,
            United_Kingdom,
            United_States,
            Uruguay,
            Uzbekistan,
            Vanuatu,
            Vatican_City,
            Venezuela,
            Vietnam,
            Yemen,
            Zambia,
            Zimbabwe
        }

        [JsonProperty("mainCountry")]
        [JsonConverter(typeof(StringEnumConverter))]
        public Country? MainCountry { get; private set; }

        [JsonProperty("mainCountry2")]
        /// <summary>
        /// If the Country enum does not include the region, for example Jersey.
        /// </summary>
        public string? MainCountry2 { get; private set; }

        [JsonConstructor]
        public Fund(string isin,
                    string name,
                    List<FundTransaction> transactions,
                    DateTime? deletedAt,
                    Types type,
                    List<Sector> sectors,
                    Country? mainCountry,
                    string? mainCountry2)
        {
            if (string.IsNullOrEmpty(isin))
            {
                throw new ArgumentException("ISIN was null / empty.", nameof(isin));
            }
            if (string.IsNullOrEmpty(name))
            {
                throw new ArgumentException("Fund name was null / empty.", nameof(name));
            }
            if (transactions is null)
            {
                throw new ArgumentException($"List of {nameof(FundTransaction)} was null.", nameof(transactions));
            }
            if (string.IsNullOrWhiteSpace(mainCountry2) && mainCountry is null)
            {
                throw new ArgumentException($"At least one of {nameof(mainCountry)} or {nameof(mainCountry2)} must be provided.");
            }

            this.ISIN = isin;
            this.Name = name;
            this.Transactions = transactions;
            this.DeletedAt = deletedAt;
            this.Type = type;
            this.Sectors = sectors;
            this.MainCountry = mainCountry;
            this.MainCountry2 = mainCountry2;
        }

        /// <summary>
        /// only for tests
        /// </summary>
        /// <param name="transactions"></param>
        public void SetTransactions(List<FundTransaction> transactions)
        {
            this.Transactions = transactions;
        }
    }
}

FundTransaction.cs

using Newtonsoft.Json;

namespace FundAndEtfApi.Models
{
    [System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
    public sealed class FundTransaction
    {
        [JsonProperty("date")]
        public DateTime Date { get; private set; }

        [JsonProperty("units")]
        public decimal Units { get; private set; }

        [JsonProperty("pricePerUnit")]
        public decimal PricePerUnit { get; private set; }

        [JsonProperty("fee")]
        public decimal Fee { get; private set; }

        [JsonProperty("totalCost")]
        public decimal TotalCost { get; private set; }

        [JsonProperty("id")]
        public Guid Id { get; private set; }

        [JsonConstructor]
        public FundTransaction(DateTime date, decimal units, decimal pricePerUnit, decimal fee, Guid id)
        {
            this.Date = date;
            this.Units = units;
            this.PricePerUnit = pricePerUnit;
            this.Fee = fee;
            this.TotalCost = (Units * PricePerUnit) + Fee;
            this.Id = id;
        }

        internal void ChangeIdIfEmpty(Guid id)
        {
            if (this.Id == Guid.Empty)
            {
                this.Id = id;
            }
        }
    }
}

FundTransactionDto.cs

namespace FundAndEtfApi.Models
{
    /// <summary>
    /// Made extra so that the actual FundTransaction object with Guid is not required.
    /// </summary>
    public sealed class FundTransactionDto
    {
        public DateTime Date { get; private set; }

        public decimal Units { get; private set; }

        public decimal PricePerUnit { get; private set; }

        public decimal Fee { get; private set; }

        public decimal TotalCost { get; private set; }


        public FundTransactionDto(DateTime date, decimal units, decimal pricePerUnit, decimal fee)
        {
            this.Date = date;
            this.Units = units;
            this.PricePerUnit = pricePerUnit;
            this.Fee = fee;
            this.TotalCost = (Units * PricePerUnit) + Fee;
        }
    }
}

Portfolio.cs

using Newtonsoft.Json;
using System;

namespace FundAndEtfApi.Models
{
    public sealed class Portfolio
    {
        [JsonProperty("funds")]
        public List<Fund> Funds { get; private set; }
        [JsonProperty("userID")]
        public Guid UserId { get; private set; }

        [JsonConstructor]
        public Portfolio(List<Fund> funds, Guid userID)
        {
            this.Funds = funds;
            this.UserId = userID;
        }

        public Portfolio()
        {
            this.Funds = new List<Fund>();
            this.UserId = Guid.Empty;
        }
    }
}

PortfolioService.cs

using FundAndEtfApi.Models;
using FundAndEtfApi.Models.Statistics;
using Newtonsoft.Json;
using Newtonsoft.Json.Converters;
using System.Linq;

namespace FundAndEtfApi.Services
{
    public sealed class PortfolioService
    {
        private const string _filePathPortfolio = "portfolio.json";
        private const string _filePathDeletedFunds = "deletedFunds.json";

        private readonly JsonSerializerSettings _jsonSettings = new JsonSerializerSettings
        {
            Formatting = Formatting.Indented,
            Converters = { new StringEnumConverter() }
        };

        private readonly IHttpContextAccessor _httpContextAccessor;

        private List<Portfolio> _allPortfolios;
        private static readonly object lockObject = new object();
        private static readonly char[] trimChars = new char[] { ' ', '\r', '\n', '\t', Convert.ToChar(160) };

        public PortfolioService(IHttpContextAccessor httpContextAccessor)
        {
            this._httpContextAccessor = httpContextAccessor;
            this._allPortfolios = new List<Portfolio>();
        }

        public async Task<Portfolio> GetPortfolio(bool calledFromTests)
        {
            string path;
            if (calledFromTests)
            {
                path = "testportfolio.json";
            }
            else
            {
                path = _filePathPortfolio;
            }

            if (!File.Exists(path))
                return new Portfolio();

            string json = await File.ReadAllTextAsync(path);
            _allPortfolios = JsonConvert.DeserializeObject<List<Portfolio>>(json) ?? new List<Portfolio>();

            Guid guid = Tools.GetCurrentUserId(_httpContextAccessor);

            Portfolio? foundPortfolioForUser = _allPortfolios.FirstOrDefault(p => p.UserId == guid);
            if (foundPortfolioForUser is null)
            {
                return new Portfolio();
            }

            foreach (Fund f in foundPortfolioForUser.Funds)
            {
                f.Transactions.Sort((x, y) => x.Date.CompareTo(y.Date));
            }

            return foundPortfolioForUser;
        }

        public async Task<bool> AddFund(Fund newFund, bool calledFromTest)
        {
            if (newFund is null)
            {
                throw new ArgumentException($"{nameof(newFund)} is null!");
            }
            if (string.IsNullOrWhiteSpace(newFund.ISIN) || newFund.ISIN.Length != 12)
            {
                throw new ArgumentException("ISIN must be exactly 12 characters long.");
            }
            if (newFund.Transactions.Count <= 0)
            {
                throw new ArgumentException("Must contain at least one transaction.");
            }
            if (!Tools.IsRequestSafe(newFund.ISIN))
            {
                throw new InvalidOperationException($"Forbidden: \"{newFund.ISIN}\"!");
            }
            if (!Tools.IsRequestSafe(newFund.Name))
            {
                throw new InvalidOperationException($"Forbidden: \"{newFund.Name}\"!");
            }

            Portfolio? portfolio = await GetPortfolio(calledFromTest);

            /* In Swagger, there are intentionally empty guids. Replace empty GUIDs with unique GUIDs. */
            foreach (FundTransaction t in newFund.Transactions)
            {
                if (t.Id == Guid.Empty)
                {
                    IEnumerable<Guid> allTranscationGuids = GetAllTransactionGuids();
                    Guid transactionGuid = Tools.GenerateUniqueGuid(allTranscationGuids);
                    t.ChangeIdIfEmpty(transactionGuid);
                }
            }

            bool existingFund = portfolio.Funds.Any(f => f.ISIN == newFund.ISIN);
            if (existingFund)
            {
                throw new ArgumentException("This fund already exists in the portfolio.");
            }

            newFund.Name = newFund.Name.TrimEnd(trimChars);
            if (newFund.Name.Length > 100)
            {
                newFund.Name = newFund.Name.Substring(0, 100);
            }

            portfolio.Funds.Add(newFund);

            if (!calledFromTest)
            {
                SavePortfolio(portfolio, calledFromTest);
            }

            return true;
        }

        private void SavePortfolio(Portfolio portfolio, bool calledFromTest)
        {
            lock (lockObject)
            {
                if (_allPortfolios is null)
                {
                    return;
                }

                string path;
                if (calledFromTest)
                {
                    path = "testportfolio.json";
                }
                else
                {
                    path = _filePathPortfolio;
                }

                // Remove previous version for the current user (if existent. Does not crash if not existent)
                _allPortfolios.RemoveAll(p => p.UserId == portfolio.UserId);

                // save new version
                _allPortfolios.Add(portfolio);

                string updatedJson = JsonConvert.SerializeObject(_allPortfolios, _jsonSettings);
                File.WriteAllText(path, updatedJson);
            }
        }

        public async Task<Guid> AddTransactionToFund(string existingIsin, FundTransactionDto fundTransactionDto, bool calledFromTest)
        {
            if (fundTransactionDto is null)
            {
                throw new ArgumentException($"{nameof(FundTransactionDto)} is null!");
            }
            if (string.IsNullOrEmpty(existingIsin))
            {
                throw new ArgumentException("The given ISIN is null or empty.");
            }
            if (existingIsin.Length != 12)
            {
                throw new ArgumentException("ISIN must be exactly 12 characters long.");
            }
            if (fundTransactionDto.Fee < 0.0M)
            {
                throw new ArgumentException($"{nameof(fundTransactionDto.Fee)} cannot be less than zero.");
            }
            if (fundTransactionDto.PricePerUnit < 0.0M)
            {
                throw new ArgumentException($"{nameof(fundTransactionDto.PricePerUnit)} cannot be less than zero.");
            }
            if (fundTransactionDto.Units <= 0.0M)
            {
                throw new ArgumentException($"{nameof(fundTransactionDto.Units)} cannot be less than or equal to zero.");
            }
            if (!Tools.IsRequestSafe(existingIsin))
            {
                throw new InvalidOperationException($"Forbidden: \"{existingIsin}\"!");
            }

            Portfolio? portfolio = await GetPortfolio(calledFromTest);

            Fund? existingFund = portfolio.Funds.FirstOrDefault(f => f.ISIN == existingIsin);

            if (existingFund is null)
            {
                throw new ArgumentException($"No fund with ISIN \"{existingIsin}\" exists.");
            }

            IEnumerable<Guid> alltranscationGuids = GetAllTransactionGuids();
            Guid transactionGuid = Tools.GenerateUniqueGuid(alltranscationGuids);

            FundTransaction newTransaction = new FundTransaction(fundTransactionDto.Date,
                                                                 fundTransactionDto.Units,
                                                                 fundTransactionDto.PricePerUnit,
                                                                 fundTransactionDto.Fee,
                                                                 transactionGuid);
            existingFund.Transactions.Add(newTransaction);

            if (!calledFromTest)
            {
                SavePortfolio(portfolio, calledFromTest);
            }

            return transactionGuid;
        }

        private IEnumerable<Guid> GetAllTransactionGuids()
        {
            return _allPortfolios
                .SelectMany(p => p.Funds)
                .SelectMany(f => f.Transactions)
                .Select(t => t.Id);
        }

        public async Task<bool> RemoveFundByIsinAndArchiveIt(string isin, bool calledFromTest)
        {
            if (string.IsNullOrEmpty(isin))
            {
                throw new ArgumentException("Given ISIN must not be null or empty.");
            }
            if (isin.Length != 12)
            {
                throw new ArgumentException("ISIN must be exactly 12 characters long.");
            }
            if (!Tools.IsRequestSafe(isin))
            {
                throw new InvalidOperationException($"Forbidden: \"{isin}\"!");
            }

            Portfolio portfolio = await GetPortfolio(calledFromTest);

            Fund? fund = portfolio.Funds.FirstOrDefault(f => f.ISIN == isin);
            if (fund is null)
            {
                throw new ArgumentException($"ISIN \"{isin}\" not found.");
            }

            SaveSoldFundToSeparateFile(fund);

            bool deleted = portfolio.Funds.Remove(fund);
            if (deleted)
            {
                SavePortfolio(portfolio, calledFromTest);
            }

            return deleted;
        }

        private void SaveSoldFundToSeparateFile(Fund fund)
        {
            List<Fund> allDeletedFunds = GetDeletedFunds().Result;
            fund.DeletedAt = DateTime.Now;
            allDeletedFunds.Add(fund);
            string json = JsonConvert.SerializeObject(allDeletedFunds, _jsonSettings);
            lock (lockObject)
            {
                File.WriteAllText(_filePathDeletedFunds, json);
            }
        }

        public Task<List<Fund>> GetDeletedFunds()
        {
            if (!File.Exists(_filePathDeletedFunds))
                return Task.FromResult(new List<Fund>());

            lock (lockObject)
            {
                string json = File.ReadAllText(_filePathDeletedFunds);
                List<Fund>? oldFunds = JsonConvert.DeserializeObject<List<Fund>>(json);
                return Task.FromResult(oldFunds ?? new List<Fund>());
            }
        }

        public async Task<bool> RemoveTransactionFromFund(string isin, Guid transactionId, bool calledFromTest)
        {
            if (string.IsNullOrEmpty(isin))
            {
                throw new ArgumentException("ISIN must not be null or empty.");
            }
            if (isin.Length != 12)
            {
                throw new ArgumentException("ISIN must be exactly 12 characters long.");
            }
            if (!Tools.IsRequestSafe(isin))
            {
                throw new InvalidOperationException($"Forbidden: \"{isin}\"!");
            }

            Portfolio portfolio = await GetPortfolio(calledFromTest);

            Fund? fund = portfolio.Funds.FirstOrDefault(f => f.ISIN == isin);
            if (fund is null)
                throw new ArgumentException($"No fund with ISIN \"{isin}\" found.");

            FundTransaction? transaction = fund.Transactions.FirstOrDefault(t => t.Id == transactionId);
            if (transaction is null)
                throw new ArgumentException($"No transaction with transactionId \"{transactionId}\" found.");

            bool success = fund.Transactions.Remove(transaction);
            if (success)
            {
                SavePortfolio(portfolio, calledFromTest);
            }

            return success;
        }

        public async Task<bool> UpdateFundName(string isin, string newName, bool calledFromTest)
        {
            if (string.IsNullOrEmpty(isin))
            {
                throw new ArgumentException("ISIN must not be null or empty.");
            }
            if (isin.Length != 12)
            {
                throw new ArgumentException("ISIN must be exactly 12 characters long.");
            }
            if (string.IsNullOrEmpty(newName))
            {
                throw new ArgumentException($"{nameof(newName)} must not be null or empty.");
            }
            if (!Tools.IsRequestSafe(isin))
            {
                throw new InvalidOperationException($"Forbidden: \"{isin}\"!");
            }
            if (!Tools.IsRequestSafe(newName))
            {
                throw new InvalidOperationException($"Forbidden: \"{newName}\"!");
            }

            Portfolio portfolio = await GetPortfolio(calledFromTest);

            Fund? fund = portfolio.Funds.FirstOrDefault(f => f.ISIN == isin);
            if (fund is null)
                throw new ArgumentException($"No fund with ISIN \"{isin}\" found.");

            fund.Name = newName.TrimEnd(trimChars);
            if (fund.Name.Length > 100)
            {
                fund.Name = fund.Name.Substring(0, 100);
            }

            if (!calledFromTest)
            {
                SavePortfolio(portfolio, calledFromTest);
            }

            return true;
        }

        internal async Task<decimal> GetTotalCosts(bool calledFromTest)
        {
            Portfolio portfolio = await GetPortfolio(calledFromTest);

            if (portfolio.Funds.Count == 0)
            {
                throw new ArgumentException("You do not have any funds in your portfolio yet.");
            }

            decimal total = portfolio.Funds.SelectMany(f => f.Transactions).Sum(t => t.TotalCost);
            return decimal.Round(total, 2, MidpointRounding.AwayFromZero);
        }

        internal async Task<PortfolioStatistics> GetPortfolioStatistics(bool calledFromTest)
        {
            Portfolio? portfolio = await GetPortfolio(calledFromTest);

            decimal totalCosts = portfolio.Funds.SelectMany(f => f.Transactions).Sum(t => t.TotalCost);

            decimal totalUnits = portfolio.Funds.SelectMany(f => f.Transactions).Sum(t => t.Units);

            List<FundShareStatistic>? fundShares = portfolio.Funds.Select(f =>
            {
                decimal fundUnits = f.Transactions.Sum(t => t.Units);
                decimal percentage = totalUnits > 0M ? (fundUnits / totalUnits) * 100M : 0M;
                return new FundShareStatistic
                {
                    ISIN = f.ISIN,
                    Name = f.Name,
                    Units = fundUnits,
                    PercentageOfPortfolio = Math.Round(percentage, 2)
                };
            }).ToList();

            Dictionary<string, decimal> byCountry = portfolio.Funds
                .GroupBy(f => f.MainCountry?.ToString() ?? f.MainCountry2 ?? "Unknown")
                .ToDictionary(
                    g => g.Key,
                    g => Math.Round(g.Sum(f => f.Transactions.Sum(t => t.Units)) / totalUnits * 100M, 2)
                );

            Dictionary<string, decimal> byType = portfolio.Funds
                .GroupBy(f => f.Type.ToString())
                .ToDictionary(
                    g => g.Key,
                    g => Math.Round(g.Sum(f => f.Transactions.Sum(t => t.Units)) / totalUnits * 100M, 2)
                );

            Dictionary<string, decimal> bySector = portfolio.Funds
                .SelectMany(f =>
                    f.Sectors.Select(s => new {
                        Sector = s,
                        Units = f.Transactions.Sum(t => t.Units) / f.Sectors.Count
                    })
                )
                .GroupBy(x => x.Sector.ToString())
                .ToDictionary(
                    g => g.Key,
                    g => Math.Round(g.Sum(x => x.Units) / totalUnits * 100, 2)
                )
                .OrderByDescending(kvp => kvp.Value)
                .ToDictionary(kvp => kvp.Key, kvp => kvp.Value);

            return new PortfolioStatistics(totalCosts, fundShares, byCountry, byType, bySector);
        }
    }
}

Tools.cs

using System.Security.Claims;

namespace FundAndEtfApi
{
    internal sealed class Tools
    {
        internal static Guid GetCurrentUserId(IHttpContextAccessor httpContextAccessor)
        {
            Claim? claim = httpContextAccessor.HttpContext?.User?.FindFirst(ClaimTypes.NameIdentifier);
            if (claim is null)
                return Guid.Empty;

            return Guid.Parse(claim.Value);
        }

        internal static Guid GenerateUniqueGuid(IEnumerable<Guid> existingUserIds)
        {
            Guid newId;
            do
            {
                newId = Guid.NewGuid();
            } while (existingUserIds.Contains(newId));

            return newId;
        }

        internal static bool IsRequestSafe(string inputFromUser)
        {
            if (string.IsNullOrEmpty(inputFromUser))
                return true;

            List<string> evilCommands = new List<string>();
            evilCommands.Add("drop table");
            evilCommands.Add("delete from");
            evilCommands.Add("select");
            evilCommands.Add("shutdown");
            evilCommands.Add("truncate table");

            foreach (string cmd in evilCommands)
            {
                if (inputFromUser.Contains(cmd, StringComparison.OrdinalIgnoreCase))
                    return false;
            }

            return true;
        }
    }
}

Statistics

FundShareStatistics.cs

namespace FundAndEtfApi.Models.Statistics
{
    public sealed class FundShareStatistic
    {
        public string ISIN { get; set; } = string.Empty;
        public string Name { get; set; } = string.Empty;
        public decimal Units { get; set; }
        public decimal PercentageOfPortfolio { get; set; }
    }
}

PortfolioStatistics.cs

namespace FundAndEtfApi.Models.Statistics
{
    public class PortfolioStatistics
    {
        public decimal TotalCosts { get; private set; }
        public List<FundShareStatistic> FundShares { get; private set; }
        public Dictionary<string, decimal> DiversificationByCountry { get; private set; }
        public Dictionary<string, decimal> DiversificationByType { get; private set; }
        public Dictionary<string, decimal> DiversificationBySector { get; private set; }

        public PortfolioStatistics(decimal totalCosts,
                                   List<FundShareStatistic> fundShares,
                                   Dictionary<string, decimal> diversificationByCountry,
                                   Dictionary<string, decimal> diversificationByType,
                                   Dictionary<string, decimal> diversificationBySector)
        {
            this.TotalCosts = totalCosts;
            this.FundShares = fundShares;
            this.DiversificationByCountry = diversificationByCountry;
            this.DiversificationByType = diversificationByType;
            this.DiversificationBySector = diversificationBySector;
        }
    }
}

PortfolioTests

PortfolioTests.cs

using FundAndEtfApi.Models;
using FundAndEtfApi.Services;
using Microsoft.AspNetCore.Http;
using Moq;
using Newtonsoft.Json;
using System.Security.Claims;

namespace FundAndEtfApi.Tests
{
    public class PortfolioTests
    {
        [Fact]
        public async Task GetPortfolioAsync_ShouldReturnCorrectPortfolioForUser()
        {
            // Arrange
            Guid userId = Guid.NewGuid();

            // Prepare test data and write to file
            var expectedPortfolio = new Portfolio(new List<Fund>(), userId);
            var allPortfolios = new List<Portfolio> { expectedPortfolio };

            var service = CreateServiceWithPortfolio(expectedPortfolio);

            string json = JsonConvert.SerializeObject(allPortfolios, Formatting.Indented);
            File.WriteAllText("testportfolio.json", json);

            // Act
            var result = await service.GetPortfolio(true);

            // Assert
            Assert.NotNull(result);
            Assert.Equal(userId, result.UserId);
        }

        private PortfolioService CreateServiceWithPortfolio(Portfolio existingPortfolio)
        {
            var httpContextAccessorMock = new Mock<IHttpContextAccessor>();
            httpContextAccessorMock.Setup(a => a.HttpContext!.User.FindFirst(It.IsAny<string>()))
                .Returns(new System.Security.Claims.Claim("sub", existingPortfolio.UserId.ToString()));

            var service = new PortfolioService(httpContextAccessorMock.Object);

            typeof(PortfolioService)
                .GetField("_allPortfolios", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance)!
                .SetValue(service, new List<Portfolio> { existingPortfolio });

            return service;
        }

        [Fact]
        public async Task AddFund_ShouldThrowArgumentException_WhenIsinIsEmptyOrTooShort()
        {
            // Arrange
            // Set up mock claims for the current user
            Guid g = Guid.NewGuid();

            var newFund = new Fund("DE00000001", "Test Fund", new List<FundTransaction>(), null, Fund.Types.ActiveFund, new List<Fund.Sector> { Fund.Sector.Energy }, Fund.Country.United_States, null);

            var existingPortfolio = new Portfolio(new List<Fund>(), g);
            var service = CreateServiceWithPortfolio(existingPortfolio);

            List<Portfolio> portfolios = new List<Portfolio>();
            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(portfolios, Formatting.Indented));

            // Act & Assert
            ArgumentException ex = await Assert.ThrowsAsync<ArgumentException>(() => service.AddFund(newFund, calledFromTest: true));

            Assert.Equal("ISIN must be exactly 12 characters long.", ex.Message);
        }

        [Fact]
        public async Task AddFund_ShouldAddFund_WhenIsinIsUnique()
        {
            // Arrange
            Guid g = Guid.NewGuid();

            FundTransaction transaction = new FundTransaction(DateTime.Now.AddHours(1), 1M, 1M, 1M, Guid.Empty);
            var newFund = new Fund("DE0000000001", "Test Fund", new List<FundTransaction>() { transaction }, null, Fund.Types.ActiveFund, new List<Fund.Sector> { Fund.Sector.Energy }, Fund.Country.United_States, null);

            var existingPortfolio = new Portfolio(new List<Fund>(), g);
            var service = CreateServiceWithPortfolio(existingPortfolio);

            List<Portfolio> portfolios = new List<Portfolio>();
            portfolios.Add(existingPortfolio);

            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(portfolios, Formatting.Indented));

            // Act
            bool result = await service.AddFund(newFund, calledFromTest: true);

            // Assert
            Assert.True(result);
        }

        [Fact]
        public async Task AddFund_ShouldMakeNewGuid_WhenTransactionGuidIsEmpty()
        {
            // Arrange
            Guid g = Guid.NewGuid();

            var fundThatAlreadyExists = new Fund("XX0000000001", "Test Fund", new List<FundTransaction>(), null, Fund.Types.ActiveFund, new List<Fund.Sector> { Fund.Sector.Energy }, Fund.Country.United_States, null);
            FundTransaction transaction = new FundTransaction(DateTime.Now.AddHours(1), 1M, 1M, 1M, Guid.Empty);
            fundThatAlreadyExists.SetTransactions(new List<FundTransaction> { transaction });

            var existingPortfolio = new Portfolio(new List<Fund>() { fundThatAlreadyExists }, g);
            var service = CreateServiceWithPortfolio(existingPortfolio);

            List<Portfolio> portfolios = new List<Portfolio>();
            portfolios.Add(existingPortfolio);

            /* I need a second fund because otherwise the ISIN already exists. In the json, it will be Fund1 again. */
            Fund Fund2 = new Fund("XX0000000002", "Test Fund 2", fundThatAlreadyExists.Transactions, null, Fund.Types.ActiveFund, new List<Fund.Sector> { Fund.Sector.Energy }, Fund.Country.United_States, null);

            // Act
            bool result = await service.AddFund(Fund2, calledFromTest: true);

            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(portfolios, Formatting.Indented));

            // Assert
            Assert.True(result);
            Portfolio p = await service.GetPortfolio(true);
            Assert.True(p.Funds[0].Transactions[0].Id != Guid.Empty);
        }

        [Fact]
        public async Task AddFund_ShouldThrowArgumentException_WhenIsinExists()
        {
            // Arrange
            // Set up mock claims for the current user
            Guid g = Guid.NewGuid();

            var newFund = new Fund("DE0000000001", "Test Fund", new List<FundTransaction>(), null, Fund.Types.ActiveFund, new List<Fund.Sector> { Fund.Sector.Energy }, Fund.Country.United_States, null);

            var existingPortfolio = new Portfolio(new List<Fund> { newFund }, g);
            var service = CreateServiceWithPortfolio(existingPortfolio);

            var duplicateFund = new Fund("DE0000000001", "Another Fund", new List<FundTransaction>(), null, Fund.Types.ActiveFund, new List<Fund.Sector> { Fund.Sector.Energy }, Fund.Country.United_States, null);

            List<Portfolio> portfolios = new List<Portfolio>();
            portfolios.Add(existingPortfolio);
            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(portfolios, Formatting.Indented));

            // Act & Assert
            await Assert.ThrowsAsync<ArgumentException>(() => service.AddFund(duplicateFund, calledFromTest: true));
        }

        [Fact]
        public async Task AddTransactionToFund_ShouldThrowException_WhenIsinIsEmpty()
        {
            // Arrange
            var userId = Guid.NewGuid();
            // HttpContext mit passendem User
            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));

            var service = new PortfolioService(accessorMock.Object);

            // Act & Assert
            var ex = await Assert.ThrowsAsync<ArgumentException>(() => service.AddTransactionToFund("",
                                                                                                    new FundTransactionDto(DateTime.Today, 1M, 100M, 5M),
                                                                                                    true));
            Assert.Equal("The given ISIN is null or empty.", ex.Message);
        }

        [Fact]
        public async Task AddTransactionToFund_ShouldThrowException_WhenIsinIsTooShort()
        {
            // Arrange
            var userId = Guid.NewGuid();
            // HttpContext mit passendem User
            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));

            var service = new PortfolioService(accessorMock.Object);

            // Act & Assert
            var ex = await Assert.ThrowsAsync<ArgumentException>(() => service.AddTransactionToFund("DE00",
                                                                                                    new FundTransactionDto(DateTime.Today, 1M, 100M, 5M),
                                                                                                    true));
            Assert.Equal("ISIN must be exactly 12 characters long.", ex.Message);
        }

        [Fact]
        public async Task AddTransactionToFund_ShouldThrowException_WhenValuesAreNegative()
        {
            // Arrange
            var userId = Guid.NewGuid();
            // HttpContext mit passendem User
            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));
            var service = new PortfolioService(accessorMock.Object);

            // Act & Assert
            var ex1 = await Assert.ThrowsAsync<ArgumentException>(() => service.AddTransactionToFund("DE0000000001",
                                                                                                     new FundTransactionDto(DateTime.Today, -1M, 100M, 5M),
                                                                                                     true));
            Assert.Equal("Units cannot be less than or equal to zero.", ex1.Message);

            var ex2 = await Assert.ThrowsAsync<ArgumentException>(() => service.AddTransactionToFund("DE0000000001",
                                                                                                     new FundTransactionDto(DateTime.Today, 1M, -1M, 5M),
                                                                                                     true));
            Assert.Equal("PricePerUnit cannot be less than zero.", ex2.Message);

            var ex3 = await Assert.ThrowsAsync<ArgumentException>(() => service.AddTransactionToFund("DE0000000001",
                                                                                                     new FundTransactionDto(DateTime.Today, 1M, 1M, -1M),
                                                                                                     true));
            Assert.Equal("Fee cannot be less than zero.", ex3.Message);
        }

        [Fact]
        public async Task AddTransactionToFund_ShouldThrowException_WhenUnitsAreZero()
        {
            // Arrange
            var userId = Guid.NewGuid();
            // HttpContext mit passendem User
            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));
            var service = new PortfolioService(accessorMock.Object);

            // Act & Assert
            var ex1 = await Assert.ThrowsAsync<ArgumentException>(() => service.AddTransactionToFund("DE0000000001",
                                                                                                     new FundTransactionDto(DateTime.Today, 0.0M, 100M, 5M),
                                                                                                     true));
            Assert.Equal($"Units cannot be less than or equal to zero.", ex1.Message);
        }

        [Fact]
        public async Task AddTransactionToFund_ShouldAddTransaction_WhenFundDoesExist()
        {
            // Arrange
            var userId = Guid.NewGuid();

            string existing = "DE0000000001";
            var fund = new Fund(existing,
                                "Name",
                                new List<FundTransaction>(),
                                null,
                                Fund.Types.ActiveFund,
                                new List<Fund.Sector> { Fund.Sector.Energy },
                                Fund.Country.United_States,
                                null);
            var portfolio = new Portfolio(new List<Fund> { fund }, userId);
            var portfolios = new List<Portfolio> { portfolio };

            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(portfolios, Formatting.Indented));

            // HttpContext mit passendem User
            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));

            var service = new PortfolioService(accessorMock.Object);

            // Act
            Guid result = await service.AddTransactionToFund(existing,
                                                             new FundTransactionDto(DateTime.Today, 10M, 100M, 5M),
                                                             true);

            // Assert
            Assert.True(result != Guid.Empty);
        }

        [Fact]
        public async Task AddTransactionToFund_ShouldThrow_WhenFundDoesNotExist()
        {
            // Arrange
            var userId = Guid.NewGuid();

            var fund = new Fund("DE0000000001",
                                "Name",
                                new List<FundTransaction>(),
                                null,
                                Fund.Types.ActiveFund,
                                new List<Fund.Sector> { Fund.Sector.Energy },
                                Fund.Country.United_States,
                                null);
            var portfolio = new Portfolio(new List<Fund> { fund }, userId);
            var portfolios = new List<Portfolio> { portfolio };

            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(portfolios, Formatting.Indented));

            // HttpContext mit passendem User
            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));

            var service = new PortfolioService(accessorMock.Object);

            string missingIsin = "DE0000000002";

            // Act & Assert
            var ex = await Assert.ThrowsAsync<ArgumentException>(() => service.AddTransactionToFund(missingIsin,
                                                                                                    new FundTransactionDto(DateTime.Today, 10M, 100M, 5M),
                                                                                                    true));

            Assert.Equal($"No fund with ISIN \"{missingIsin}\" exists.", ex.Message);
        }

        [Fact]
        public async Task RemoveFundByIsinAndArchiveIt_ShouldRemoveAndArchiveFund_WhenIsinExists()
        {
            // Arrange
            var userId = Guid.NewGuid();

            var fund = new Fund("DE1234567890",
                                "Test Fund",
                                new List<FundTransaction>(),
                                null,
                                Fund.Types.ActiveFund,
                                new List<Fund.Sector> { Fund.Sector.Energy },
                                Fund.Country.United_States,
                                null);
            var portfolio = new Portfolio(new List<Fund> { fund }, userId);
            var portfolios = new List<Portfolio> { portfolio };
            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(portfolios, Formatting.Indented));

            // Delete archive if exists
            if (File.Exists("deletedFunds.json"))
            {
                File.Delete("deletedFunds.json");
            }

            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));
            var service = new PortfolioService(accessorMock.Object);

            // Act
            bool result = await service.RemoveFundByIsinAndArchiveIt("DE1234567890", calledFromTest: true);

            // Assert
            Assert.True(result);

            // Verify that the fund is no longer in the portfolio
            Portfolio updatedPortfolio = await service.GetPortfolio(true);
            Assert.DoesNotContain(updatedPortfolio.Funds, f => f.ISIN == "DE1234567890");

            // Verify that the fund was archived
            Assert.True(File.Exists("deletedFunds.json"));
            List<Fund> archivedFunds = await service.GetDeletedFunds();
            Assert.Single(archivedFunds);
            Assert.Equal("DE1234567890", archivedFunds[0].ISIN);
        }

        [Theory]
        [InlineData(null)]
        [InlineData("")]
        public async Task RemoveFundByIsinAndArchiveIt_ShouldThrowArgumentException_WhenIsinIsNullOrEmpty(string invalidIsin)
        {
            // Arrange
            var userId = Guid.NewGuid();
            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));

            var service = new PortfolioService(accessorMock.Object);

            // Act & Assert
            var ex = await Assert.ThrowsAsync<ArgumentException>(() =>
                service.RemoveFundByIsinAndArchiveIt(invalidIsin, calledFromTest: true));

            Assert.Equal("Given ISIN must not be null or empty.", ex.Message);
        }

        [Fact]
        public async Task RemoveTransactionFromFund_ShouldRemoveTransaction_WhenExists()
        {
            // Arrange
            var userId = Guid.NewGuid();
            var transactionId = Guid.NewGuid();

            var transaction = new FundTransaction(
                date: DateTime.Today,
                units: 10,
                pricePerUnit: 100,
                fee: 1,
                id: transactionId);

            Fund fund = new Fund("TESTISIN1234",
                                 "Test Fund",
                                 new List<FundTransaction> { transaction },
                                 null,
                                 Fund.Types.ActiveFund,
                                 new List<Fund.Sector> { Fund.Sector.Energy },
                                 Fund.Country.United_States,
                                 null);
            Portfolio portfolio = new Portfolio(new List<Fund> { fund }, userId);

            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(new List<Portfolio> { portfolio }, Formatting.Indented));

            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));
            var service = new PortfolioService(accessorMock.Object);

            // Act
            bool result = await service.RemoveTransactionFromFund("TESTISIN1234", transactionId, calledFromTest: true);

            // Assert
            Assert.True(result);
            var updatedPortfolio = await service.GetPortfolio(true);
            var updatedFund = updatedPortfolio.Funds.First();
            Assert.Empty(updatedFund.Transactions);
        }

        [Fact]
        public async Task RemoveTransactionFromFund_ShouldThrow_WhenTransactionNotFound()
        {
            // Arrange
            var userId = Guid.NewGuid();
            var otherTransactionId = Guid.NewGuid();

            List<FundTransaction> transactions = new List<FundTransaction>();
            transactions.Add(new FundTransaction(DateTime.Today, 1M, 1M, 1M, Guid.NewGuid()));
            Fund fund = new Fund("TESTISIN4567",
                                 "Another Fund",
                                 transactions,
                                 null,
                                 Fund.Types.ActiveFund,
                                 new List<Fund.Sector> { Fund.Sector.Energy },
                                 Fund.Country.United_States,
                                 null);
            Portfolio portfolio = new Portfolio(new List<Fund> { fund }, userId);

            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(new List<Portfolio> { portfolio }));

            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));
            var service = new PortfolioService(accessorMock.Object);

            // Act & Assert
            var ex = await Assert.ThrowsAsync<ArgumentException>(() =>
                service.RemoveTransactionFromFund("TESTISIN4567", otherTransactionId, calledFromTest: true));

            Assert.Contains("transactionId", ex.Message);
        }

        [Fact]
        public async Task RemoveTransactionFromFund_ShouldThrow_WhenIsinIsEmtpyOrTooShort()
        {
            // Arrange
            var userId = Guid.NewGuid();
            var otherTransactionId = Guid.NewGuid();

            List<FundTransaction> transactions = new List<FundTransaction>();
            transactions.Add(new FundTransaction(DateTime.Today, 1M, 1M, 1M, Guid.NewGuid()));
            Fund fund = new Fund("TESTISIN4567",
                                 "Another Fund",
                                 transactions,
                                 null,
                                 Fund.Types.ActiveFund,
                                 new List<Fund.Sector> { Fund.Sector.Energy },
                                 Fund.Country.United_States,
                                 null);
            Portfolio portfolio = new Portfolio(new List<Fund> { fund }, userId);

            File.WriteAllText("testportfolio.json", JsonConvert.SerializeObject(new List<Portfolio> { portfolio }));

            var accessorMock = new Mock<IHttpContextAccessor>();
            accessorMock.Setup(x => x.HttpContext!.User.FindFirst(ClaimTypes.NameIdentifier))
                        .Returns(new Claim(ClaimTypes.NameIdentifier, userId.ToString()));
            var service = new PortfolioService(accessorMock.Object);

            // Act & Assert
            var ex = await Assert.ThrowsAsync<ArgumentException>(() =>
                service.RemoveTransactionFromFund("TESTISIN456", otherTransactionId, calledFromTest: true));

            Assert.Equal("ISIN must be exactly 12 characters long.", ex.Message);
        }
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Use SQLite as a database. You can install NuGet packages that include all the necessary code into your application. No installation of any database engine is required on your system (however, it makes development easier if you install SQLiteStudio on your dev PC). If you use an O/R-mapper (Dapper, EF Core, ...) for the databse access, it will be easy to switch to a "big" database like SQL-Server, Oracle, MySQL, etc. later. \$\endgroup\$ Commented Oct 12 at 13:44

1 Answer 1

1
\$\begingroup\$

Here are some of highlights :

PortfolioController.cs

  • catch will only catch the specified excetpions such as ArgumentException and InvalidOperationException, and it will ignore the rest. You need to either to also catch Exception in order to catch all other exceptions.
  • catching specific exceptions are useful to apply some specific logic when thrown, however, you only return the exception message on all of them, which means it's not needed, and better to not specify a specific type of exception, just use Exception instead.
  • DO NOT EVER return the Exception message nor the details to the consumer. log the Exception internally, and return a generic message to the consumer. This will conceal your code information along with any sensitive data that might the exception bring with it.
  • you are returning a 200 OK status code without checking the service results and act upon it.

Fund.cs

  • Move enum out side the class, and rename them after the class (e.g. FundTypes, FundSectors ..etc.) This would add more clarity.
  • use record instead of class if you want immutable class.

FundTransaction.cs and FundTransactionDto.cs are the same except that you excluded Id. If you have similar Dtos, you can replace them with a simple json converter that you will use whenever you need to return the serialized object without the Id property. This would lower the number of dtos that you need, and keep only the dtos that needs some custom logic to flatten the actual models.

PortfolioService.cs

  • instead of injecting IHttpContextAccessor, use method arguments to get the required arguments for each method. This would remove the IHttpContextAccessor dependency from your class.
  • bool calledFromTests to use test/debug, you should use preprocessor directives this is a better option to handle debugging code based on the environment.
  • too many exceptions are used.
  • Exceptions are expensive and used to catch unhandled errors/bugs that the code might have. They are used as last resolution to stop the code when unexpected behaviors happens. In your code, you are treating exceptions as normal logic validation. What you need is to change it to have a result type that handles the user validation, and return the result, in which the consumer can handle it. Something like Results Class
\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.