Thank your to all your suggestions. So far, I've arrived to the following degree of refactorization. Are there further suggestions?
First, I encapsulated and reorganized all the parameters, so the new call was simplified:
public class PharmacyInfo {
public string FirstName { get; set; }
public string LastName { get; set; }
public string Email { get; set; }
public string Cif { get; set; }
public string NumberColegiado { get; set; }
public string OrgId { get; set; }
public bool EsDelegado { get; set; }
public string Sanibrick { get; set; }
public string Territory { get; set; }
}
public class AddressInfo {
public string Address { get; set; }
public string Phone { get; set; }
public string ZipCode { get; set; }
public string OrganizationName { get; set; }
public string City { get; set; }
public string Community { get; set; }
public string Province { get; set; }
}
public Result RegisterPharmacy(PharmacyInfo pharmacyInfo, AddressInfo addressInfo) {
//Check first if a contact exists for the given orgid
ICustomerContact pharmacy = repository.GetPharmacyInterface(pharmacyInfo.OrgId);
//If it does not exists, build it
if (pharmacy == null)
pharmacy = customerFactory.CreateInstance(pharmacyInfo, addressInfo);
else pharmacy = customerFactory.UpdateInstance(pharmacy, pharmacyInfo, addressInfo);
//Update/Create the pharmacy
repository.SavePharmacy(pharmacy);
return new Result(true);
}
Let's get deep with the RegisterPharmacy: I refactored the sealed class "CustomerContact" using the adapter pattern. I identified that in order to register a customer in my system I needed 2 steps: a MembershipUser and a CustomerContact, so I combined them together into one interface, ICustomerContact, and created an implementation for that interface:
public interface ICustomerContact
{
PrimaryKeyId? PrimaryKeyId { get; set; }
IEnumerable<CustomerAddress> ContactAddresses { get; }
CustomerAddress PreferredBillingAddress { get; set; }
CustomerAddress PreferredShippingAddress { get; set; }
string Email { get; set; }
string FirstName { get; set; }
string FullName { get; set; }
string LastName { get; set; }
DateTime? LastOrder { get; set; }
PrimaryKeyId? PreferredBillingAddressId { get; set; }
PrimaryKeyId? PreferredShippingAddressId { get; set; }
string RegistrationSource { get; set; }
object ProviderUserKey { get; }
string Cif { get; set; }
string Orgid { get; set; }
bool EsDelegado { get; set; }
string NumberColegiado { get; set; }
Dictionary<IWarehouse, string> AssociatedWholesalers { get; set; }
void DeleteContactAddress(CustomerAddress address);
void UpdateContactAddress(CustomerAddress address);
void AddContactAddress(CustomerAddress address);
void SaveChanges();
}
public class Pharmacy : ICustomerContact
{
public CustomerContact Contact {
get;
private set;
}
public PrimaryKeyId? PrimaryKeyId {
get {
return Contact.PrimaryKeyId;
}
set {
Contact.PrimaryKeyId = value;
}
}
public MembershipUser User {
get;
private set;
}
public Pharmacy(CustomerContact contact, MembershipUser membershipUser) {
Contact = contact;
User = membershipUser;
var profile = EPiServerProfile.Get(User.UserName);
if (profile != null) {
var wholesalerList = profile[Constants.Profile.WholesalersData] as List<string>;
AssociatedWholesalers = wholesalerList == null ? new Dictionary<IWarehouse, string>() :
UserWholesaler.ConvertToWholesalerList(wholesalerList).ToDictionary(uw => WarehouseHelper.GetWarehouse(uw.WholesalerId), uw => uw.CollaborationCode);
Cif = profile[Constants.Profile.CifCode] as string;
NumberColegiado = profile[Constants.Profile.NumberColegiado] as string;
}
else AssociatedWholesalers = new Dictionary<IWarehouse, string>();
}
public void SaveChanges() {
if (User != null) {
Membership.UpdateUser(User);
MapUserKey _mapUserKey = new MapUserKey();
Contact.UserId = _mapUserKey.ToTypedString(User.ProviderUserKey);
var wholesalersString = AssociatedWholesalers.Select(aw => new UserWholesaler {
CollaborationCode = aw.Value,
Region = aw.Key.ContactInformation.RegionCode,
RegionName = aw.Key.ContactInformation.RegionName,
WholesalerId = aw.Key.WarehouseId.Value,
WholesalerName = aw.Key.Name,
WholesalerCode = aw.Key.Code
}.ToString()).ToList();
var profile = EPiServerProfile.Get(User.UserName);
profile[Constants.Profile.WholesalersData] = wholesalersString;
profile[Constants.Profile.CifCode] = Cif;
profile[Constants.Profile.NumberColegiado] = NumberColegiado;
profile.Save();
}
Contact.SaveChanges();
var defaultAddress = ContactAddresses.FirstOrDefault();
this.PreferredBillingAddressId = defaultAddress != null ? defaultAddress.PrimaryKeyId : null;
this.PreferredShippingAddressId = defaultAddress != null ? defaultAddress.PrimaryKeyId : null;
Contact.SaveChanges();
}
public IEnumerable<CustomerAddress> ContactAddresses {
get { return Contact.ContactAddresses; }
}
public CustomerAddress PreferredBillingAddress {
get {
return Contact.PreferredBillingAddress ?? lastAddedAddress;
}
set {
Contact.PreferredBillingAddress = value;
}
}
public CustomerAddress PreferredShippingAddress {
get {
return Contact.PreferredShippingAddress ?? lastAddedAddress;
}
set {
Contact.PreferredShippingAddress = value;
}
}
public string Email {
get {
return Contact.Email;
}
set {
Contact.Email = value;
}
}
public string FirstName {
get {
return Contact.FirstName;
}
set {
Contact.FirstName = value;
}
}
{...}
}
The method "SaveChanges" from CustomerContact had direct contact with the Database (unavoidable) but I wanted to manage all my connections to database through Repositories (repository pattern). So I ended up adding a new operation into the repository to which calls the SaveChanges function. probably not the best solution, but it works out quite well:
public interface IPharmacyRepository
{
void SavePharmacy(ICustomerContact pharmacy);
}
{...}
public void SavePharmacy(ICustomerContact pharmacy) {
pharmacy.SaveChanges();
}
Then I extracted all the logic to create an instance of ICustomerContact to a factory:
public interface ICustomerFactory
{
ICustomerContact CreateInstance(PharmacyInfo pharmacyInfo, AddressInfo addressInfo);
ICustomerContact UpdateInstance(ICustomerContact contact, PharmacyInfo pharmacyInfo, AddressInfo addressInfo);
}
public class PharmacyFactory : ICustomerFactory
{
ICustomerContextWrapper customerContext;
IMembershipProvider membershipProvider;
public PharmacyFactory(ICustomerContextWrapper customerContext, IMembershipProvider membershipProvider) {
this.customerContext = customerContext;
this.membershipProvider = membershipProvider;
}
public ICustomerContact UpdateInstance(ICustomerContact contact, PharmacyInfo pharmacyInfo, AddressInfo addressInfo) {
if (!pharmacyInfo.EsDelegado) {
var address = contact.ContactAddresses.FirstOrDefault();
if (address == null) contact.AddContactAddress(CreateAddress(contact, addressInfo));
else contact.UpdateContactAddress(UpdateAddress(address, contact, addressInfo));
}
var membershipUser = UpdateMembership(contact, pharmacyInfo.Email, pharmacyInfo.Email);
try {
this.AssignDefaultRoles(membershipUser, pharmacyInfo.EsDelegado);
}
catch { /*Swallow the exception due to possible duplication of Role assignment
Consider refactor this later*/
}
contact = new Pharmacy(customerContext.GetContactById(contact.PrimaryKeyId.Value), membershipUser);
contact = SetProperties(contact, pharmacyInfo);
return contact;
}
public ICustomerContact CreateInstance(PharmacyInfo pharmacyInfo, AddressInfo addressInfo) {
var customerContact = CustomerContact.CreateInstance();
var membershipUser = CreateMembership(pharmacyInfo.Email, pharmacyInfo.Email);
try {
this.AssignDefaultRoles(membershipUser, pharmacyInfo.EsDelegado);
}
catch { /*Swallow the exception due to possible duplication of Role assignment.
Consider refactor this later*/ }
ICustomerContact pharmacy = new Pharmacy(customerContact, membershipUser);
pharmacy = this.SetProperties(pharmacy, pharmacyInfo);
if (!pharmacyInfo.EsDelegado)
pharmacy.AddContactAddress(CreateAddress(pharmacy, addressInfo));
return pharmacy;
}
private ICustomerContact SetProperties(ICustomerContact contact, PharmacyInfo pharmacyInfo) {
contact.FirstName = pharmacyInfo.FirstName;
contact.FullName = pharmacyInfo.FirstName;
contact.LastName = pharmacyInfo.LastName;
contact.RegistrationSource = String.Format("{0}, {1}", "Job Run", SiteContext.Current);
contact.Email = pharmacyInfo.Email;
contact.Orgid = pharmacyInfo.OrgId;
contact.EsDelegado = pharmacyInfo.EsDelegado;
contact.Cif = pharmacyInfo.Cif;
return contact;
}
private MembershipUser UpdateMembership(ICustomerContact contact, string email, string username) {
MembershipUser membershipUser = null;
if (contact.ProviderUserKey != null)
membershipUser = membershipProvider.GetUser(contact.ProviderUserKey);
else membershipUser = CreateMembership(email, username);
return membershipUser;
}
private MembershipUser CreateMembership(string email, string username) {
//Check first if the membershipUser exists for the given emailAddress. If it does not, create it.
MembershipUser membershipUser = membershipProvider.GetUser(username);
if (membershipUser == null) {
var generatedPassword = membershipProvider.GeneratePassword(6, 3);
membershipUser = membershipProvider.CreateUser(username, generatedPassword, email);
if (membershipUser == null)
throw new Exception("Could not create the delegate account. The contact doesn't have an associated account and " +
"the email could not be found nor created");
}
membershipUser.IsApproved = true;
return membershipUser;
}
private void AssignDefaultRoles(MembershipUser user, bool esDelegado) {
string[] DefaultUserRoles = { AppRoles.RegisteredRole, AppRoles.EveryoneRole, AppRoles.ManagerUserRole };
var roles = from roleName in DefaultUserRoles
where !SecurityContext.Current.CheckUserInGlobalRole(user, roleName)
select roleName;
//Assign default roles to new user
foreach (var roleName in roles) {
var principal = SecurityContext.Current.GetPrincipalByUser(user);
var globalRoleAssign = new GlobalRoleAssignment(principal, roleName);
SecurityContext.Current.CreateUserRoleAssignments(globalRoleAssign);
}
if (AppSettings.Roles_Guest != null && Roles.RoleExists(AppSettings.Roles_Guest))
Roles.AddUserToRole(user.UserName, AppSettings.Roles_Guest);
if (esDelegado && Roles.RoleExists("Delegados"))
Roles.AddUserToRole(user.UserName, "Delegados");
}
private CustomerAddress CreateAddress(ICustomerContact pharmacy, AddressInfo addressInfo) {
var address = CustomerAddress.CreateInstance();
return UpdateAddress(address, pharmacy, addressInfo);
}
private CustomerAddress UpdateAddress(CustomerAddress address, ICustomerContact pharmacy, AddressInfo addressInfo) {
address.Name = "Primero";
address.Line1 = addressInfo.Address;
address.City = addressInfo.City;
address.CountryCode = "es";
address.CountryName = "Spain";
address.State = addressInfo.Community;
address.RegionName = addressInfo.Province;
address.PostalCode = addressInfo.ZipCode;
address.OrganizationName = addressInfo.OrganizationName;
address.DaytimePhoneNumber = addressInfo.Phone;
address.Email = pharmacy.Email;
address.FirstName = pharmacy.FirstName;
address.LastName = pharmacy.LastName;
return address;
}
}
Unfortunately Membership holds heavy dependencies for the Web server, so I created also a wrapper for it:
public interface IMembershipProvider
{
MembershipUser GetUser();
{...}
}
All the dependencies are managed by an IoC container.
After all these changes, now I can finally mock dependencies and Unit test it quite well. Here are some of the tests:
[Test]
public void UpdateExistingUser() {
//Arrange
var customerfactoryMock = new Mock<ICustomerFactory>();
var pharmacyrepoMock = new Mock<IPharmacyRepository>();
var customermock = new Mock<ICustomerContact>();
pharmacyrepoMock.Setup(m => m.GetPharmacyInterface(It.IsAny<string>()))
.Returns(customermock.Object);
customerfactoryMock.Setup(m => m.UpdateInstance(It.IsAny<ICustomerContact>(), It.IsAny<PharmacyInfo>(), It.IsAny<AddressInfo>()))
.Returns(customermock.Object);
var assignmentstestData = TestDataFactory.GetAssignments();
var regHelper = new RegistrationHelper(Mock.Of<IMailSender>(), null, new Utils(), null, pharmacyrepoMock.Object, customerfactoryMock.Object);
//Act
var result = regHelper.RegisterPharmacy(new PharmacyInfo() { OrgId = "1234567890" }, null);
//Assert
customerfactoryMock.Verify(m => m.UpdateInstance(It.IsAny<ICustomerContact>(), It.IsAny<PharmacyInfo>(), It.IsAny<AddressInfo>()), Times.Once());
Assert.AreEqual(result.Success, true);
}
[Test]
public void RegisterNewUser() {
//Arrange
var customerfactoryMock = new Mock<ICustomerFactory>();
var pharmacyrepoMock = new Mock<IPharmacyRepository>();
var customermock = new Mock<ICustomerContact>();
customerfactoryMock.Setup(m => m.CreateInstance(It.IsAny<PharmacyInfo>(), It.IsAny<AddressInfo>()))
.Returns(customermock.Object);
var assignmentstestData = TestDataFactory.GetAssignments();
var regHelper = new RegistrationHelper(Mock.Of<IMailSender>(), null, new Utils(), null, pharmacyrepoMock.Object, customerfactoryMock.Object);
//Act
var result = regHelper.RegisterPharmacy(new PharmacyInfo() { OrgId = "1234567890" }, null);
//Assert
customerfactoryMock.Verify<ICustomerContact>(m => m.CreateInstance(It.IsAny<PharmacyInfo>(), It.IsAny<AddressInfo>()), Times.Once());
Assert.AreEqual(result.Success, true);
}
[Test]
public void CreateCustomerContact() {
//Arrange
var customerContextMock = new Mock<ICustomerContextWrapper>();
var membershipProviderMock = new Mock<IMembershipProvider>();
var membershipUserMock = new Mock<MembershipUser>();
membershipUserMock.SetupGet(m => m.ProviderUserKey).Returns("Hola");
membershipProviderMock.Setup(m => m.GetUser(It.IsAny<string>()))
.Returns(membershipUserMock.Object);
var factory = new PharmacyFactory(customerContextMock.Object, membershipProviderMock.Object);
var pharmacyInfo = new PharmacyInfo() {
Cif = "12345678A",
Email = "a@a.com",
EsDelegado = false,
FirstName = "Pablo",
LastName = "Neruda",
OrgId = "1234567890"
};
var addressInfo = new AddressInfo() {
City = "Barcelona",
Community = "Catalunya",
Phone = "123456788",
Province = "Barcelona",
Address = "Calle absurda"
};
//Act
var result = factory.CreateInstance(pharmacyInfo, addressInfo);
//Assert
Assert.AreEqual(pharmacyInfo.OrgId, result.Orgid);
Assert.AreEqual(pharmacyInfo.Cif, result.Cif);
Assert.AreEqual(addressInfo.Address, result.PreferredBillingAddress.Line1);
}