3
\$\begingroup\$

This is an update to my previous post about my Active Directory Query Application. I have made multiple modifications, and I request some feedback.

Summary: This program queries an active directory and writes either a list of the users or a list of the users with a list of the groups each user belongs to. The search is done in an Organizational Unit selected via a dropdown. Files are saved to .csv.

"Problems" I'd Appreciate Commentary On:

  1. The front end (ActiveDirectoryTool) does a bit of what seems to be back end work, but if I move the work to the back end, I don't see how I can have the front end report progress.

  2. On the same note, the front end's backgroundWorker_DoWork method is very complex, but aside from chopping it into separate methods for each of the cases, I don't know what to do.

  3. It feels like I have a lot of duplicate code, but I don't see how to trim things down without losing functionality.

ActiveDirectoryTool

using ActiveDirectoryTool.ActiveDirectoryToolBackend;
using System;
using System.ComponentModel;
using System.DirectoryServices.AccountManagement;
using System.Windows.Forms;

using static ActiveDirectoryTool.ActiveDirectoryToolBackend
    .ActiveDirectoryToolConstants;

namespace ActiveDirectoryTool
{
    public partial class ActiveDirectoryTool : Form
    {
        private ActiveDirectoryToolBackEnd backEnd;

        private Task currentTask;

        public ActiveDirectoryTool()
        {
            InitializeComponent();
            backEnd = new ActiveDirectoryToolBackEnd();
            UpdateDisplay();
        }

        private enum Task
        {
            PrintAllUsers,
            PrintAllUserGroups,
            PrintAllGroups,
            PrintAllComputers
        }
        private void backgroundWorker_ProgressChanged(object sender,
            ProgressChangedEventArgs e)
        {
            progressBar.Value = e.ProgressPercentage;
            var counts = (Tuple<int, int>)e.UserState;
            progressLabel.Text = counts.Item1 + ProgressLabelDivider
                + counts.Item2;
        }

        private void backgroundWorker_DoWork(object sender, DoWorkEventArgs e)
        {
            try
            {
                int percentComplete = 0;
                int currentCount = 0;
                switch (currentTask)
                {
                    case Task.PrintAllUsers:
                        var users = backEnd.GetUsers();
                        var filename = backEnd.AllUsersFilename;
                        backEnd.WriteHeaderToFile(filename, Header.User);
                        for (int i = 0; i < users.Count; i++)
                        {
                            currentCount = i + 1;
                            backEnd.WriteUserToFile(filename, users[i]);
                            percentComplete = currentCount * PercentMultiplier
                                / users.Count;
                            backgroundWorker.ReportProgress(percentComplete,
                                new Tuple<int, int>(currentCount,
                                users.Count));
                        }
                        MessageBox.Show("Wrote all users to " + filename);
                        break;

                    case Task.PrintAllUserGroups:
                        users = backEnd.GetUsers();
                        filename = backEnd.AllUsersGroupsFilename;
                        backEnd.WriteHeaderToFile(filename, Header.UserGroup);
                        for (int i = 0; i < users.Count; i++)
                        {
                            currentCount = i + 1;
                            try
                            {
                                foreach (GroupPrincipal group in users[i]
                                    .GetGroups())
                                {
                                    backEnd.WriteUserGroupsToFile(filename,
                                        users[i], group);
                                }
                            }
                            catch (Exception exc)
                            {
                                Console.WriteLine(exc.StackTrace);
                                continue;
                            }
                            percentComplete = currentCount * PercentMultiplier
                                / users.Count;
                            backgroundWorker.ReportProgress(percentComplete,
                                new Tuple<int, int>(currentCount,
                                users.Count));
                        }
                        MessageBox.Show("Wrote all users' groups to "
                            + filename);
                        break;

                    // TODO
                    case Task.PrintAllGroups:
                        break;

                    // TODO
                    case Task.PrintAllComputers:
                        break;
                }
            }
            catch (Exception exc)
            {
                MessageBox.Show("Exception: " + exc.Message + "\n"
                    + exc.StackTrace);
            }
        }

        private void backgroundWorker_RunWorkerCompleted(object sender,
            RunWorkerCompletedEventArgs e)
        {
            progressBar.Value = 0;
            progressLabel.Text = "Awaiting task...";
            this.Enabled = true;
        }

        private void getAllUsers_Click(object sender, EventArgs e)
        {
            PerformTask(Task.PrintAllUsers);
        }

        private void organizationalUnits_SelectedIndexChanged(object sender,
            EventArgs e)
        {
            backEnd.Scope = backEnd.OrganizationalUnits[organizationalUnits
                .SelectedIndex];
        }

        private void PerformTask(Task task)
        {
            if (!backgroundWorker.IsBusy
                && organizationalUnits.SelectedIndex > -1)
            {
                this.Enabled = false;
                currentTask = task;
                backgroundWorker.RunWorkerAsync();
            }
            else if (organizationalUnits.SelectedIndex < 0)
            {
                MessageBox.Show("Please select an Organizational Unit!");
            }
            else
            {
                MessageBox.Show("Currently performing an operation!");
            }
        }

        private void printAllComputers_Click(object sender, EventArgs e)
        {
            PerformTask(Task.PrintAllComputers);
        }

        private void printAllGroups_Click(object sender, EventArgs e)
        {
            PerformTask(Task.PrintAllGroups);
        }

        private void printAllUserGroups_Click(object sender, EventArgs e)
        {
            PerformTask(Task.PrintAllUserGroups);
        }
        private void UpdateDisplay()
        {
            foreach (var organizationalUnit in backEnd.OrganizationalUnits)
            {
                string organizationalUnitDisplay = organizationalUnit
                    .Replace(",OU=Accounts,OU=Domtar,OU=DPP,DC=dnet,DC=domtar",
                    "")
                    .Replace("OU=", "");
                organizationalUnits.Items.Add(organizationalUnitDisplay);
            }
        }
    }
}

ActiveDirectoryToolBackend

using System;
using System.Collections.Generic;
using System.DirectoryServices;
using System.DirectoryServices.AccountManagement;
using System.IO;

using static ActiveDirectoryTool.ActiveDirectoryToolBackend
    .ActiveDirectoryToolConstants;

namespace ActiveDirectoryTool.ActiveDirectoryToolBackend
{
    internal enum Header
    {
        User,
        Group,
        UserGroup,
        Computer
    }

    internal class ActiveDirectoryToolBackEnd
    {
        private string path = Path.Combine(Environment
            .GetFolderPath(Environment.SpecialFolder.MyDocuments),
            FolderName);

        public ActiveDirectoryToolBackEnd()
        {
            Directory.CreateDirectory(path);
        }

        public string AllUsersFilename
        {
            get
            {
                return Path.Combine(path, GenerateFilename(FilenameUsers));
            }
        }

        public string AllUsersGroupsFilename
        {
            get
            {
                return Path.Combine(path,
                    GenerateFilename(FilenameUserGroups));
            }
        }

        public List<string> OrganizationalUnits
        {
            get
            {
                var organizationalUnits = new List<string>();
                DirectoryEntry userRoot = new DirectoryEntry(
                    DefaultUserOrganizationalUnitsRoot);
                var searcher = new DirectorySearcher(userRoot);
                searcher.Filter = DirectorySearcherFilterOrganizationalUnits;
                searcher.SearchScope = SearchScope.OneLevel;
                foreach (SearchResult result in searcher.FindAll())
                {
                    string resultString = result.Path.Replace(LdapPrefix,
                        String.Empty);
                    organizationalUnits.Add(resultString);
                }
                return organizationalUnits;
            }
        }

        public String Scope { get; set; }

        public List<UserPrincipal> GetUsers()
        {
            var users = new List<UserPrincipal>();
            using (var searcher = new PrincipalSearcher(new UserPrincipal(
                GetPrincipalContext())))
            {
                foreach (UserPrincipal user in searcher.FindAll())
                {
                    users.Add(user);
                }
            }
            return users;
        }

        public void WriteHeaderToFile(string filename, Header header)
        {
            using (var writer = new StreamWriter(filename))
            {
                writer.WriteLine(CommaSeparatedValuesSeparatorSemicolon);
                switch (header)
                {
                    case Header.User:
                        writer.WriteLine(HeaderUser);
                        break;

                    case Header.Group:
                        writer.WriteLine(HeaderGroup);
                        break;

                    case Header.UserGroup:
                        writer.WriteLine(HeaderUserGroup);
                        break;

                    case Header.Computer:
                        writer.WriteLine(HeaderComputer);
                        break;
                }
            }
        }

        public void WriteUserGroupsToFile(string filename, UserPrincipal user,
            GroupPrincipal group)
        {
            using (var writer = new StreamWriter(filename, true))
            {
                writer.WriteLine(UserGroupAsString(user, group));
            }
        }

        public void WriteUserToFile(string filename, UserPrincipal user)
        {
            using (var writer = new StreamWriter(filename, true))
            {
                writer.WriteLine(UserAsString(user));
            }
        }
        private string ComputerAsString(ComputerPrincipal computer)
        {
            return String.Join(
                Char.ToString(Tab),
                computer.Name);
        }

        private string GenerateFilename(string fileType)
        {
            return fileType + Hyphen + DateTime.Now.ToString(DateTimeFormat)
                + DefaultExtension;
        }

        private PrincipalContext GetPrincipalContext()
        {
            return new PrincipalContext(ContextType.Domain, Domain,
                Scope);
        }

        private string GroupAsString(GroupPrincipal group)
        {
            DirectoryEntry entry = group.GetUnderlyingObject() as
                DirectoryEntry;
            string manager = (string)entry.Properties[PropertyManager].Value;
            if (String.IsNullOrWhiteSpace(manager))
            {
                manager = PropertyNone;
            }
            return String.Join(
                Char.ToString(Tab),
                group.SamAccountName,
                manager,
                group.Description,
                group.DistinguishedName);
        }

        private bool IsActive(DirectoryEntry de)
        {
            if (de.NativeGuid == null) return false;

            int flags = (int)de.Properties[PropertyUserAccountControl].Value;

            return !Convert.ToBoolean(flags & 0x0002);
        }

        private string UserAsString(UserPrincipal user)
        {
            DirectoryEntry entry = user.GetUnderlyingObject() as
                DirectoryEntry;
            return string.Join(
                Char.ToString(Semicolon),
                user.Surname,
                user.GivenName,
                user.DisplayName,
                user.SamAccountName,
                IsActive(entry).ToString(),
                user.IsAccountLockedOut().ToString(),
                user.Description,
                user.HomeDrive,
                user.HomeDirectory,
                user.ScriptPath,
                user.EmailAddress,
                (string)entry.Properties[PropertyStreetAddress].Value,
                (string)entry.Properties[PropertyCity].Value,
                (string)entry.Properties[PropertyState].Value,
                user.VoiceTelephoneNumber,
                user.DistinguishedName);
        }

        private string UserGroupAsString(UserPrincipal user,
            GroupPrincipal group)
        {
            return String.Join(
                Char.ToString(Semicolon),
                user.SamAccountName,
                group.SamAccountName,
                user.Name,
                user.DistinguishedName);
        }
    }
}

ActiveDirectoryToolConstants

namespace ActiveDirectoryTool.ActiveDirectoryToolBackend
{
    internal static class ActiveDirectoryToolConstants
    {
        public const char Comma = ',';
        public const char Hyphen = '-';
        public const char Quote = '"';
        public const char Semicolon = ';';
        public const char Tab = '\t';
        public const int PercentMultiplier = 100;
        public const string CommaSeparatedValuesExtension = ".csv";
        public const string CommaSeparatedValuesSeparatorSemicolon = "sep=;";
        public const string DateTimeFormat = "yyyyMMddTHHmmss";
        public const string DefaultExtension = CommaSeparatedValuesExtension;
        public const string DefaultUserOrganizationalUnitsRoot
            = "LDAP://OU=Accounts,OU=Domtar,OU=DPP,DC=dnet,DC=domtar";
        public const string DirectorySearcherFilterOrganizationalUnits
            = "(objectCategory=organizationalUnit)";
        public const string Domain = "dnet.domtar";
        public const string FilenameComputers = "Computers";
        public const string FilenameGroups = "Groups";
        public const string FilenameUserGroups = "UserGroups";
        public const string FilenameUsers = "Users";
        public const string FolderName = "ActiveDirectoryTool";
        public const string HeaderComputer = "Name";
        public const string HeaderGroup =
            "Group Name;Group ID;Managed By;Description;Distinguished Name";
        public const string HeaderUser
            = "Last;First;Display Name;ID;Active;Locked;Description;"
            + "Home Drive;Home Folder;Login Script;Email;Street;City;State;"
            + "Phone;Distinguished Name";
        public const string HeaderUserGroup
            = "User ID;Group;User Full Name;User Distinguished Name";
        public const string LdapPrefix = "LDAP://";
        public const string PlainTextExtension = ".txt";
        public const string ProgressLabelDivider = " of ";
        public const string PropertyCity = "l";
        public const string PropertyManager = "managedBy";
        public const string PropertyNone = "none";
        public const string PropertyState = "st";
        public const string PropertyStreetAddress = "streetAddress";
        public const string PropertyUserAccountControl = "userAccountControl";
        public const string TabSeparatedValuesExtension = ".tsv";
    }
}
\$\endgroup\$
3
\$\begingroup\$

It seems that your backgroundWorker is fiddling with UI elements, although the only thing he does is to show a MessageBox. I do not know if this is safe or not, but I will be consider to be unsafe on my review.

BackgroundWorker died long ago since TPL - Task Parallel Library appeared. With TPL you can do everything you used to do with the old backgroundWorker and even in a neater way.

Instead of having that Task enum, you will have Methods that return exactly a Task object (this is marvellous I know). That task will do exactly what a case statement is doing at this moment. Plus when the operation ends in will also show the message on the UI thread instead of background thread like your backgroundWorker_DoWork is doing at the moment.

But optimally you would change all your tasks so they would return a common type, that you could use to do the things you wanted (show messages, updates labels or other controls...)

Tasks per se do not report progress but what we can do is to implement the interface IProgress on our form and call the Report method whenever needed. Talking about reporting, one thing that annoyed me the most is that you didn't define a class to specify that data that you want to report. If you did that you could realize that Percentage could be a calculated read-only Property.

public partial class ActiveDirectoryTool : Form, 
IProgress<ActiveDirectoryTool.ProgressReport>
{
    public class ProgressReport
    {
        public int Percentage { get { return CurrentCount / Total; } }
        public int CurrentCount { get; set; }
        public int Total { get; set; }
    }

    private ActiveDirectoryToolBackEnd backEnd;
    private Task PrintAllUsers ()=> Task.Run(() =>
    {
        var users = backEnd.GetUsers();
        var filename = backEnd.AllUsersFilename;
        backEnd.WriteHeaderToFile(filename, Header.User);
        for (int i = 0; i < users.Count; i++)
        {
            backEnd.WriteUserToFile(filename, users[i]);
            this.Report(new ProgressReport()
            {
                Total = users.Count,
                CurrentCount = i + 1
            });
        };
        return filename;
    }).ContinueWith(t =>
    {
        MessageBox.Show("Wrote all users to " + t.Result);
    }, 
        CancellationToken.None, 
        TaskContinuationOptions.OnlyOnRanToCompletion, 
        TaskScheduler.FromCurrentSynchronizationContext()
    );
    private Task PrintAllUserGroups() => Task.Run(() =>
    {
        var users = backEnd.GetUsers();
        var filename = backEnd.AllUsersGroupsFilename;
        backEnd.WriteHeaderToFile(filename, Header.UserGroup);
        for (int i = 0; i < users.Count; i++)
        {
            try
            {
                foreach (GroupPrincipal group in users[i].GetGroups())
                {
                    backEnd.WriteUserGroupsToFile(filename,
                        users[i], group);
                }
            }
            catch (Exception exc)
            {
                Console.WriteLine(exc.StackTrace);
                continue;
            }
            this.Report(new ProgressReport()
            {
                Total = users.Count,
                CurrentCount = i + 1
            });
        }
        return filename;
    }).ContinueWith(t =>
    {
        MessageBox.Show("Wrote all users' groups to " + t.Result);
    },
        CancellationToken.None,
        TaskContinuationOptions.OnlyOnRanToCompletion,
        TaskScheduler.FromCurrentSynchronizationContext()
    );
    private Task PrintAllGroups() => Task.Run(() =>
    {

    });
    private Task PrintAllComputers() => Task.Run(() =>
    {

    });

    private async void getAllUsers_Click(object sender, EventArgs e)
    {
        await PerformTask(PrintAllUsers());
    }

    private void organizationalUnits_SelectedIndexChanged(object sender,
        EventArgs e)
    {
        backEnd.Scope = backEnd.OrganizationalUnits[organizationalUnits
            .SelectedIndex];
    }

    private async Task PerformTask(Task task)
    {
        if (organizationalUnits.SelectedIndex > -1)
        {
            this.Enabled = false;
            try
            {
                await task;
            }
            catch (Exception exc)
            {
                MessageBox.Show("Exception: " + exc.Message + "\n"
                    + exc.StackTrace);
            }

            //this is what you would do on complete
            progressBar.Value = 0;
            progressLabel.Text = "Awaiting task...";
            this.Enabled = true;
        }
        else if (organizationalUnits.SelectedIndex < 0)
        {
            MessageBox.Show("Please select an Organizational Unit!");
        }
        else
        {
            MessageBox.Show("Currently performing an operation!");
        }
    }

    private async void printAllComputers_Click(object sender, EventArgs e)
    {
        await PerformTask(PrintAllComputers());
    }

    private async void printAllGroups_Click(object sender, EventArgs e)
    {
        await PerformTask(PrintAllGroups());
    }

    private async void printAllUserGroups_Click(object sender, EventArgs e)
    {
        await PerformTask(PrintAllUserGroups());
    }
    private void UpdateDisplay()
    {
        foreach (var organizationalUnit in backEnd.OrganizationalUnits)
        {
            string organizationalUnitDisplay = organizationalUnit
                .Replace(",OU=Accounts,OU=Domtar,OU=DPP,DC=dnet,DC=domtar",
                "")
                .Replace("OU=", "");
            organizationalUnits.Items.Add(organizationalUnitDisplay);
        }
    }

    public void Report(ProgressReport value)
    {
        if (InvokeRequired)
        {
            Invoke((Action) (() => Report(value)));
        }
        else
        {
            progressBar.Value = value.Percentage;
            progressLabel.Text = value.CurrentCount + ProgressLabelDivider
                + value.Total;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ What does the if (InvokeRequired) block in the Report method do? \$\endgroup\$ – Michael Brandon Morris May 26 '16 at 11:44
  • 1
    \$\begingroup\$ @michaelbmorris The Report method may be invoked on a thread of the thread pool, because is being called inside a task. To make sure we update the UI on the UI thread then we need to check InvokeRequired. InvokeRequired will be true if the current thread is not the UI thread. The same strategy could be used to show the MessageBox \$\endgroup\$ – Bruno Costa May 26 '16 at 12:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.