refactor changelly & improve tests (#366)

BTCPayServer.Tests/ChangellyTests.cs

@@ -4,6 +4,7 @@ using System.Linq;
 using System.Net.Http;
 using System.Threading.Tasks;
 using BTCPayServer.Controllers;
+using BTCPayServer.Data;
 using BTCPayServer.Models;
 using BTCPayServer.Models.StoreViewModels;
 using BTCPayServer.Payments.Changelly;
@@ -112,7 +113,8 @@ namespace BTCPayServer.Tests
                 user.GrantAccess();
                 var changellyController =
                     tester.PayTester.GetController<ChangellyController>(user.UserId, user.StoreId);
-
+                changellyController.IsTest = true;
+                
                 //test non existing payment method
                 Assert.IsType<BitpayErrorModel>(Assert
                     .IsType<BadRequestObjectResult>(await changellyController.GetCurrencyList(user.StoreId))
@@ -161,42 +163,24 @@ namespace BTCPayServer.Tests
                 //save changelly settings
                 var updateModel = new UpdateChangellySettingsViewModel()
                 {
-                    ApiSecret = "secret",
-                    ApiKey = "key",
-                    ApiUrl = "http://gozo.com",
-                    ChangellyMerchantId = "aaa"
+                    Enabled = true
                 };
                 var storesController = tester.PayTester.GetController<StoresController>(user.UserId, user.StoreId);
 
                 //confirm saved
                 Assert.Equal("UpdateStore", Assert.IsType<RedirectToActionResult>(
                     await storesController.UpdateChangellySettings(user.StoreId, updateModel, "save")).ActionName);
-
-
-                var mockChangelly = new MockChangelly(new MockHttpClientFactory(), updateModel.ApiKey, updateModel.ApiSecret, updateModel.ApiUrl);
-                var mock = new MockChangellyClientProvider(mockChangelly, tester.PayTester.StoreRepository);
                 
                 var factory = UnitTest1.CreateBTCPayRateFactory();
                 var fetcher = new RateFetcher(factory);
-
-                var changellyController = new ChangellyController(mock, tester.NetworkProvider, fetcher);
-
-                
-                mockChangelly.GetCurrenciesFullResult = new List<CurrencyFull>()
-                {
-                    new CurrencyFull()
-                    {
-                        Name = "a",
-                        Enable = true,
-                        PayInConfirmations = 10,
-                        FullName = "aa",
-                        ImageLink = ""
-                    }
-                };
+                var httpClientFactory = new MockHttpClientFactory();
+                var changellyController = new ChangellyController(
+                    new ChangellyClientProvider(tester.PayTester.StoreRepository,httpClientFactory), tester.NetworkProvider, fetcher);
+                changellyController.IsTest = true;
                 var result = Assert
                     .IsType<OkObjectResult>(await changellyController.GetCurrencyList(user.StoreId))
                     .Value as IEnumerable<CurrencyFull>;
-                Assert.Equal(1, mockChangelly.GetCurrenciesFullCallCount);
+                Assert.True(result.Any());
 
             }
         }
@@ -213,41 +197,22 @@ namespace BTCPayServer.Tests
 
                 var updateModel = new UpdateChangellySettingsViewModel()
                 {
-                    ApiSecret = "secret",
-                    ApiKey = "key",
-                    ApiUrl = "http://gozo.com",
-                    ChangellyMerchantId = "aaa"
+                    Enabled = true
                 };
                 var storesController = tester.PayTester.GetController<StoresController>(user.UserId, user.StoreId);
 
                 Assert.Equal("UpdateStore", Assert.IsType<RedirectToActionResult>(
                     await storesController.UpdateChangellySettings(user.StoreId, updateModel, "save")).ActionName);
 
-                var mockChangelly = new MockChangelly(new MockHttpClientFactory(),  updateModel.ApiKey, updateModel.ApiSecret, updateModel.ApiUrl);
-                var mock = new MockChangellyClientProvider(mockChangelly, tester.PayTester.StoreRepository);
-                
                 var factory = UnitTest1.CreateBTCPayRateFactory();
                 var fetcher = new RateFetcher(factory);
-                
-                var changellyController = new ChangellyController(mock,tester.NetworkProvider,fetcher);
-
-                mockChangelly.GetExchangeAmountResult = (from, to, amount) =>
-                {
-                    Assert.Equal("A", from);
-                    Assert.Equal("B", to);
-
-                    switch (mockChangelly.GetExchangeAmountCallCount)
-                    {
-                        case 1:
-                            return 0.5m;
-                        default:
-                            return 1.01m;
-                    }
-                };
-
+                var httpClientFactory = new MockHttpClientFactory();
+                var changellyController = new ChangellyController(
+                    new ChangellyClientProvider(tester.PayTester.StoreRepository,httpClientFactory), tester.NetworkProvider, fetcher);
+                changellyController.IsTest = true;
                 Assert.IsType<decimal>(Assert
-                    .IsType<OkObjectResult>(await changellyController.CalculateAmount(user.StoreId, "A", "B", 1.0m)).Value);
-                Assert.True(mockChangelly.GetExchangeAmountCallCount > 1);
+                    .IsType<OkObjectResult>(await changellyController.CalculateAmount(user.StoreId, "ltc", "btc", 1.0m)).Value);
+
             }
         }
     }
@@ -259,60 +224,4 @@ namespace BTCPayServer.Tests
             return  new HttpClient();
         }
     }
-    
-    public class MockChangelly : Changelly
-    {
-        public IEnumerable<CurrencyFull> GetCurrenciesFullResult { get; set; }
-
-        public delegate decimal ParamsFunc<T1, T2, T3, TResult>(T1 arg1, T2 arg2, T3 arg3);
-
-        public ParamsFunc<string, string, decimal, (decimal amount, bool Success, string Error)> GetExchangeAmountResult
-        {
-            get;
-            set;
-        }
-
-        public int GetCurrenciesFullCallCount { get; set; } = 0;
-        public int GetExchangeAmountCallCount { get; set; } = 0;
-
-        public MockChangelly(IHttpClientFactory httpClientFactory,  string apiKey, string apiSecret, string apiUrl) : base(httpClientFactory, apiKey, apiSecret, apiUrl)
-        {
-        }
-
-#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
-        public override async Task<IEnumerable<CurrencyFull>> GetCurrenciesFull()
-#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously
-        {
-            GetCurrenciesFullCallCount++;
-            return GetCurrenciesFullResult;
-        }
-
-#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
-        public override async Task<decimal> GetExchangeAmount(string fromCurrency,
-#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously
-            string toCurrency, decimal amount)
-        {
-            GetExchangeAmountCallCount++;
-            return GetExchangeAmountResult.Invoke(fromCurrency, toCurrency, amount);
-        }
-    }
-
-    public class MockChangellyClientProvider : ChangellyClientProvider
-    {
-        public MockChangelly MockChangelly;
-
-        public MockChangellyClientProvider(
-            MockChangelly mockChangelly,
-            StoreRepository storeRepository) : base(storeRepository, new MockHttpClientFactory())
-        {
-            MockChangelly = mockChangelly;
-        }
-
-        public override bool TryGetChangellyClient(string storeId, out string error, out Changelly changelly)
-        {
-            error = null;
-            changelly = MockChangelly;
-            return true;
-        }
-    }
 }

BTCPayServer/Controllers/ChangellyController.cs

@@ -29,13 +29,11 @@ namespace BTCPayServer.Controllers
         [Route("currencies")]
         public async Task<IActionResult> GetCurrencyList(string storeId)
         {
-            if (!TryGetChangellyClient(storeId, out var actionResult, out var client))
-            {
-                return actionResult;
-            }
-
             try
             {
+                
+                var client = await TryGetChangellyClient(storeId);
+                
                 return Ok(await client.GetCurrenciesFull());
             }
             catch (Exception e)
@@ -52,26 +50,16 @@ namespace BTCPayServer.Controllers
         public async Task<IActionResult> CalculateAmount(string storeId, string fromCurrency, string toCurrency,
             decimal toCurrencyAmount)
         {
-            if (!TryGetChangellyClient(storeId, out var actionResult, out var client))
-            {
-                return actionResult;
-            }
-
-
-            if (fromCurrency.Equals("usd", StringComparison.InvariantCultureIgnoreCase)
-                || fromCurrency.Equals("eur", StringComparison.InvariantCultureIgnoreCase))
-            {
-                var store = HttpContext.GetStoreData();
-                var rules = store.GetStoreBlob().GetRateRules(_btcPayNetworkProvider);
-                var rate = await _RateProviderFactory.FetchRate(new CurrencyPair(toCurrency, fromCurrency), rules);
-                if (rate.BidAsk == null) return BadRequest();
-                var flatRate = rate.BidAsk.Center;
-                return Ok(flatRate * toCurrencyAmount);
-            }
-
-
             try
             {
+                var client = await TryGetChangellyClient(storeId);
+
+                if (fromCurrency.Equals("usd", StringComparison.InvariantCultureIgnoreCase)
+                    || fromCurrency.Equals("eur", StringComparison.InvariantCultureIgnoreCase))
+                {
+                    return await HandleCalculateFiatAmount(fromCurrency, toCurrency, toCurrencyAmount);
+                }
+                
                 var callCounter = 0;
                 var response1 = await client.GetExchangeAmount(fromCurrency, toCurrency, 1);
                 var currentAmount = response1;
@@ -105,21 +93,25 @@ namespace BTCPayServer.Controllers
             }
         }
 
-        private bool TryGetChangellyClient(string storeId, out IActionResult actionResult,
-            out Changelly changelly)
+        private async Task<Changelly> TryGetChangellyClient(string storeId)
         {
-            changelly = null;
-            actionResult = null;
-            storeId = storeId ?? HttpContext.GetStoreData()?.Id;
-
-            if (_changellyClientProvider.TryGetChangellyClient(storeId, out var error, out changelly)) 
-                return true;
-            actionResult = BadRequest(new BitpayErrorModel()
-            {
-                Error = error
-            });
-            return false;
+            var store = IsTest? null: HttpContext.GetStoreData();
+            storeId = storeId ?? store?.Id;
 
+            return await _changellyClientProvider.TryGetChangellyClient(storeId, store);
         }
+
+        private async Task<IActionResult> HandleCalculateFiatAmount(string fromCurrency, string toCurrency,
+            decimal toCurrencyAmount)
+        {
+            var store = HttpContext.GetStoreData();
+            var rules = store.GetStoreBlob().GetRateRules(_btcPayNetworkProvider);
+            var rate = await _RateProviderFactory.FetchRate(new CurrencyPair(toCurrency, fromCurrency), rules);
+            if (rate.BidAsk == null) return BadRequest();
+            var flatRate = rate.BidAsk.Center;
+            return Ok(flatRate * toCurrencyAmount);
+        }
+
+        public bool IsTest { get; set; } = false;
     }
 }

BTCPayServer/Payments/Changelly/ChangellyClientProvider.cs

@@ -1,6 +1,8 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Net.Http;
+using System.Threading.Tasks;
+using BTCPayServer.Data;
 using BTCPayServer.Services.Stores;
 using NBitcoin;
 
@@ -29,47 +31,40 @@ namespace BTCPayServer.Payments.Changelly
         }
 
 
-        public virtual bool TryGetChangellyClient(string storeId, out string error,
-            out Changelly changelly)
+        public virtual async Task<Changelly> TryGetChangellyClient(string storeId, StoreData storeData = null)
         {
             if (_clientCache.ContainsKey(storeId))
             {
-                changelly = _clientCache[storeId];
-                error = null;
-                return true;
+                return _clientCache[storeId];
             }
 
-            changelly = null;
-
-
-            var store = _storeRepository.FindStore(storeId).Result;
-            if (store == null)
+            if (storeData == null)
             {
-                error = "Store not found";
-                return false;
+                storeData = await _storeRepository.FindStore(storeId);
+                if (storeData == null)
+                {
+                    throw new ChangellyException("Store not found");
+                }
             }
 
-            var blob = store.GetStoreBlob();
+            var blob = storeData.GetStoreBlob();
             var changellySettings = blob.ChangellySettings;
 
 
             if (changellySettings == null || !changellySettings.IsConfigured())
             {
-                error = "Changelly not configured for this store";
-                return false;
+                throw new ChangellyException("Changelly not configured for this store");
             }
 
             if (!changellySettings.Enabled)
             {
-                error = "Changelly not enabled for this store";
-                return false;
+                throw new ChangellyException("Changelly not enabled for this store");
             }
 
-            changelly = new Changelly(_httpClientFactory, changellySettings.ApiKey, changellySettings.ApiSecret,
+            var changelly = new Changelly(_httpClientFactory, changellySettings.ApiKey, changellySettings.ApiSecret,
                 changellySettings.ApiUrl, changellySettings.ShowFiat);
             _clientCache.AddOrReplace(storeId, changelly);
-            error = null;
-            return true;
+            return changelly;
         }
     }
 }

BTCPayServer/Views/Stores/UpdateChangellySettings.cshtml

@@ -11,6 +11,12 @@
 <div class="row">
     <div class="col-md-10">
         <form method="post">
+            <p>
+                You can obtain API keys at
+                <a href="https://changelly.com/?ref_id=804298eb5753" target="_blank">
+                    Changelly.com
+                </a>
+            </p>
             <div class="form-group">
                 <label asp-for="ApiUrl"></label>
                 <input asp-for="ApiUrl" class="form-control"/>
@@ -25,17 +31,17 @@
                 <label asp-for="ApiSecret"></label>
                 <input asp-for="ApiSecret" class="form-control"/>
                 <span asp-validation-for="ApiSecret" class="text-danger"></span>
-            </div>            
+            </div>
             <div class="form-group">
                 <label asp-for="ChangellyMerchantId"></label>
                 <input asp-for="ChangellyMerchantId" class="form-control"/>
                 <span asp-validation-for="ChangellyMerchantId" class="text-danger"></span>
-            </div>            
+            </div>
             <div class="form-group">
                 <label asp-for="AmountMarkupPercentage"></label>
                 <input asp-for="AmountMarkupPercentage" class="form-control"/>
                 <span asp-validation-for="AmountMarkupPercentage" class="text-danger"></span>
-            </div>            
+            </div>
             <div class="form-group">
                 <label asp-for="ShowFiat"></label>
                 <input asp-for="ShowFiat" class="form-check"/>