7
\$\begingroup\$

This class represents a single query on an Active Directory. It is given the type of query to run and the types of data to return, and performs the query and returns the data when it is commanded to do so.

The goals I had with this class were to ensure that the query is run async so it doesn't freeze (and crash) the UI and to be able to create the query without executing it immediately. The queries also support cancellation (it is handled farther "down" the calling tree in the ActiveDirectorySearcher and DataPreparer classes).

Questions I have:

  • Are the methods separated logically?

  • Does my usage of Lazy make sense, or would I be better served by doing something different?

  • Does my usage of dictionaries make sense, or does it seem "hacky?"

  • This contents of this class used to belong to another class, and I split the classes to try to follow the single responsibility principle. It seems to me that this class has a single responsibility, but I could be mistaken. Does this class, in your opinion, really only do one thing? If not, where/how would you generally suggest I split it again?

Query.cs

public class Query
{
    private readonly Scope _activeDirectoryScope;
    private readonly DataPreparer _dataPreparer;
    private readonly IEnumerable<string> _distinguishedNames;
    private readonly PrincipalContext _principalContext;

    private CancellationTokenSource _cancellationTokenSource;

    public Query(
        QueryType queryType,
        Scope activeDirectoryScope = null,
        IEnumerable<string> distinguishedNames = null)
    {
        QueryType = queryType;
        _principalContext = new PrincipalContext(ContextType.Domain);
        _activeDirectoryScope = activeDirectoryScope;
        _distinguishedNames = distinguishedNames;
        if (QueryTypeIsOu())
        {
            _dataPreparer = SetUpOuDataPreparer();
        }
        else if (QueryTypeIsComputer())
        {
            _dataPreparer = SetUpComputerDataPreparer();
        }
        else if (QueryTypeIsDirectReportOrUser())
        {
            _dataPreparer = SetUpDirectReportOrUserDataPreparer();
        }
        else if (QueryTypeIsGroup())
        {
            _dataPreparer = SetUpGroupDataPreparer();
        }
    }

    private CancellationToken CancellationToken
        => _cancellationTokenSource.Token;

    public IEnumerable<ExpandoObject> Data { get; private set; }

    public string Name => Scope + " - " + QueryType;

    public QueryType QueryType { get; }

    public string Scope { get; private set; }

    public void Cancel()
    {
        _cancellationTokenSource?.Cancel();
    }

    public void DisposeData()
    {
        Data = null;
    }

    public async Task Execute()
    {
        _cancellationTokenSource = new CancellationTokenSource();

        var task = Task.Run(
            () => { Data = GetData(_dataPreparer); },
            _cancellationTokenSource.Token);
        await task;
    }

    private ComputerPrincipal GetComputerPrincipal(
        string distinguishedName)
    {
        return ComputerPrincipal.FindByIdentity(
            _principalContext, distinguishedName);
    }

    private IEnumerable<ComputerPrincipal> GetComputerPrincipals()
    {
        return _distinguishedNames.Select(GetComputerPrincipal);
    }

    private static IEnumerable<ExpandoObject> GetData(
        DataPreparer dataPreparer)
    {
        return new List<ExpandoObject>(dataPreparer.GetResults());
    }

    private GroupPrincipal GetGroupPrincipal(
        string distinguishedName)
    {
        return GroupPrincipal.FindByIdentity(
            _principalContext, distinguishedName);
    }

    private IEnumerable<GroupPrincipal> GetGroupPrincipals()
    {
        return _distinguishedNames.Select(GetGroupPrincipal);
    }

    private UserPrincipal GetUserPrincipal(string distinguishedName)
    {
        return UserPrincipal.FindByIdentity(
            _principalContext, distinguishedName);
    }

    private IEnumerable<UserPrincipal> GetUserPrincipals()
    {
        return _distinguishedNames.Select(GetUserPrincipal);
    }

    private bool QueryTypeIsComputer()
    {
        return QueryType == ComputersGroups ||
               QueryType == ComputersSummaries;
    }

    private bool QueryTypeIsDirectReportOrUser()
    {
        return QueryType == UsersDirectReports ||
               QueryType == UsersGroups ||
               QueryType == UsersSummaries ||
               QueryType == DirectReportsDirectReports ||
               QueryType == DirectReportsGroups ||
               QueryType == DirectReportsSummaries;
    }

    private bool QueryTypeIsGroup()
    {
        return QueryType == GroupsComputers ||
               QueryType == GroupsUsers ||
               QueryType == GroupsUsersDirectReports ||
               QueryType == GroupsUsersGroups ||
               QueryType == GroupsSummaries;
    }

    private bool QueryTypeIsOu()
    {
        return QueryType == OuComputers ||
               QueryType == OuGroups ||
               QueryType == OuGroupsUsers ||
               QueryType == OuUsers ||
               QueryType == OuUsersDirectReports ||
               QueryType == OuUsersGroups;
    }

    private DataPreparer SetUpComputerDataPreparer()
    {
        var computerPrincipals = GetComputerPrincipals();
        Scope = "Computers";
        var computerDataPreparers =
            new Dictionary<QueryType, Func<DataPreparer>>
            {
                [ComputersGroups] = () =>
                {
                    return new DataPreparer
                    {
                        Data = new Lazy<IEnumerable<object>>(() =>
                                Searcher.GetComputersGroups(
                                    computerPrincipals,
                                    CancellationToken)),
                        Properties = DefaultComputerGroupsProperties
                    };
                },
                [ComputersSummaries] = () =>
                {
                    return new DataPreparer
                    {
                        Data = new Lazy<IEnumerable<object>>(
                            () =>
                                computerPrincipals),
                        Properties = DefaultComputerProperties
                    };
                }
            };
        return computerDataPreparers[QueryType]();
    }

    private DataPreparer SetUpDirectReportOrUserDataPreparer()
    {
        var userPrincipals = GetUserPrincipals();
        Scope = "Users";
        var simplifiedQueryTypes =
            new Dictionary<QueryType, SimplifiedQueryType>
            {
                [DirectReportsDirectReports] = DirectReports,
                [DirectReportsGroups] = Groups,
                [DirectReportsSummaries] = Summaries,
                [UsersDirectReports] = DirectReports,
                [UsersGroups] = Groups,
                [UsersSummaries] = Summaries
            };
        var directReportOrUserDataPreparers = new Dictionary
            <SimplifiedQueryType, Func<DataPreparer>>
        {
            [DirectReports] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () =>
                            Searcher.GetUsersDirectReports(
                                userPrincipals, CancellationToken)),
                    Properties = DefaultUserDirectReportsProperties
                };
            },
            [Groups] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () =>
                            Searcher.GetUsersGroups(
                                userPrincipals, CancellationToken)),
                    Properties = DefaultUserGroupsProperties
                };
            },
            [Summaries] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () => userPrincipals),
                    Properties = DefaultUserProperties
                };
            }
        };
        return directReportOrUserDataPreparers[
            simplifiedQueryTypes[QueryType]]();
    }

    private DataPreparer SetUpGroupDataPreparer()
    {
        var groupPrincipals = GetGroupPrincipals();
        Scope = "Groups";
        var groupDataPreparers =
            new Dictionary<QueryType, Func<DataPreparer>>
            {
                [GroupsComputers] = () =>
                {
                    return new DataPreparer
                    {
                        Data = new Lazy<IEnumerable<object>>(
                            () =>
                                Searcher
                                    .GetComputerPrincipals(
                                        groupPrincipals, CancellationToken)),
                        Properties = DefaultGroupComputersProperties
                    };
                },
                [GroupsSummaries] = () =>
                {
                    return new DataPreparer
                    {
                        Data = new Lazy<IEnumerable<object>>(
                            () =>
                                groupPrincipals),
                        Properties = DefaultGroupProperties
                    };
                },
                [GroupsUsers] = () =>
                {
                    return new DataPreparer
                    {
                        Data = new Lazy<IEnumerable<object>>(
                            () =>
                                Searcher.GetGroupsUsers(
                                    groupPrincipals, CancellationToken)),
                        Properties = DefaultGroupUsersProperties
                    };
                },
                [GroupsUsersDirectReports] = () =>
                {
                    return new DataPreparer
                    {
                        Data = new Lazy<IEnumerable<object>>(
                            () =>
                                Searcher
                                    .GetGroupsUsersDirectReports(
                                        groupPrincipals, CancellationToken)),
                        Properties =
                            DefaultGroupUsersDirectReportsProperties
                    };
                },
                [GroupsUsersGroups] = () =>
                {
                    return new DataPreparer
                    {
                        Data = new Lazy<IEnumerable<object>>(
                            () =>
                                Searcher
                                    .GetGroupsUsersGroups(
                                        groupPrincipals, CancellationToken)),
                        Properties = DefaultGroupUsersGroupsProperties
                    };
                }
            };
        return groupDataPreparers[QueryType]();
    }

    private DataPreparer SetUpOuDataPreparer()
    {
        Scope = _activeDirectoryScope.Context;
        var activeDirectorySearcher = new Searcher(
            _activeDirectoryScope);
        var ouDataPreparers = new Dictionary<QueryType, Func<DataPreparer>>
        {
            [OuComputers] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () =>
                            activeDirectorySearcher.GetOuComputerPrincipals
                                ()),
                    Properties = DefaultComputerProperties
                };
            },
            [OuGroups] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () =>
                            activeDirectorySearcher.GetOuGroupPrincipals()),
                    Properties = DefaultGroupProperties
                };
            },
            [OuGroupsUsers] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () =>
                            activeDirectorySearcher.GetOuGroupsUsers(
                                CancellationToken)),
                    Properties = DefaultGroupUsersProperties
                };
            },
            [OuUsers] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () =>
                            activeDirectorySearcher.GetOuUserPrincipals()),
                    Properties = DefaultUserProperties
                };
            },
            [OuUsersDirectReports] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () =>
                            activeDirectorySearcher.GetOuUsersDirectReports
                                (
                                    CancellationToken)),
                    Properties = DefaultUserDirectReportsProperties
                };
            },
            [OuUsersGroups] = () =>
            {
                return new DataPreparer
                {
                    Data = new Lazy<IEnumerable<object>>(
                        () => activeDirectorySearcher.GetOuUsersGroups(
                                CancellationToken)),
                    Properties = DefaultUserGroupsProperties
                };
            }
        };
        return ouDataPreparers[QueryType]();
    }
}
\$\endgroup\$
1
+50
\$\begingroup\$

I cannot give you an exact and perfect solution because there are several parts in your code that I do not understand like the DataPreparer and I haven't worked with AD yet but I think I can give you some hints.

The Query class definitely does more then one thing. The most obvious indicator for it is the QueryType parameter and the four different initializations at the end.

You should split it into those four queries so that each query prepares and maintaines data only for its own use.


I'd create a QueryBase to hold the common data and to provide the async execution and the cancellation.

abstract class QueryBase
{
    protected readonly DataPreparer _dataPreparer;
    private CancellationTokenSource _cancellationTokenSource;

    protected QueryBase(/* common parameters */)
    {
        // ...
    }

    public void Cancel()
    {
        _cancellationTokenSource?.Cancel();
    }

    public abstract async Task Execute();

    protected async Task Execute(Action action)
    {
        _cancellationTokenSource = new CancellationTokenSource();

        var task = Task.Run(action, _cancellationTokenSource.Token);
        await task;
    }
}

Next I'd create specialized queries where each of them does only one thing (currently your query is able to do four different things)

class ComputerQuery : QueryBase 
{
    public ComputerQuery(/* common parameters */) : base(...) 
    {
        // do what the SetUpComputerDataPreparer does 
    }

    public override async Task Execute()
    {
        return Execute(() => { Data = GetData(_dataPreparer); });
    }
}

The other queries would then be:

class DirectReportOrUserQuery {}
class GroupQuery {}
class OuQuery {}

One last remark:

public void DisposeData()
{
    Data = null;
}

This method shouldn't be called Dispose because it doesn't dispose any resources - I suggest to call it ClearData

\$\endgroup\$

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.