10
\$\begingroup\$

I have made a program that given a directory, will get the permissions for that directory and all sub-directories. The output is stored in a string that can be easily written to .csv. Each permissions entry is given its own line. The path is displayed first, with each directory level split into separate cells. Obviously, if the directory nesting has varying depth, the rows have a different number of cells. The permissions data follows, aligned after the deepest level, so the output would look like this:

enter image description here

Right now, the only way I see to do this is a multiple iteration of the results. I must go through the file system to fetch all the directories and save them, then find the longest path, and then format them all into a string. Needless to say, this takes a very long time on large file systems, such as one of my company's network shares with over 200,000 directories. It takes over a day for the program to run on this share, but if I make the path print in a single cell, using only one iteration, it takes minutes.

For more narrow queries, the program runs acceptably well, usually within a few seconds. However, I'm sure there is some way to reduce the multiple iterations so that the time required is reduced also.

PermissionsChecker

using System;
using System.Collections.Generic;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Extensions.CollectionExtensions;

namespace DirectoryPermissionsChecker
{
    internal class PermissionsChecker
    {
        public static readonly string OutputPath =
            Path.Combine(
                Environment.GetFolderPath(
                    Environment.SpecialFolder.MyDocuments),
                "DirectoryPermissionsChecker");

        private const string LogFileName = "Log";
        private const string OutputFileName = "DirectoryPermissions";

        private const string CsvExtension = ".csv";
        private const string DateTimeFormat = "yyyyMMddTHHmmss";

        private readonly CancellationTokenSource _cancellationTokenSource;
        private readonly DirectoryGetter _directoryGetter;
        private readonly List<string> _log;

        internal PermissionsChecker(
            string rootPath,
            SearchDepth searchDepth)
        {
            _cancellationTokenSource = new CancellationTokenSource();
            _log = new List<string>();
            _directoryGetter = new DirectoryGetter(
                rootPath, searchDepth, _cancellationTokenSource.Token, _log);
            Directory.CreateDirectory(OutputPath);
        }

        internal List<DirectoryInfo> Directories { get; private set; }

        internal void Cancel()
        {
            _cancellationTokenSource.Cancel();
        }

        internal async Task Execute()
        {
            var task = Task.Run(
                () =>
                {
                    Directories = _directoryGetter.GetDirectories();
                    var resultsFileName = Path.Combine(
                OutputPath,
                $"{OutputFileName} - " +
                $"{DateTime.Now.ToString(DateTimeFormat)}{CsvExtension}");
                    File.WriteAllText(
                        resultsFileName,
                        new DirectoryFormatter(
                            Directories, _cancellationTokenSource.Token, _log)
                        .FormatDirectories());
                },
                _cancellationTokenSource.Token);
            await task;
            if (_log.IsEmpty()) return;
            var fileName = Path.Combine(
                OutputPath,
                $"{LogFileName} - " +
                $"{DateTime.Now.ToString(DateTimeFormat)}{CsvExtension}");
            File.WriteAllLines(fileName, _log);
        }
    }
}

DirectoryGetter

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace DirectoryPermissionsChecker
{
    internal enum SearchDepth
    {
        All,
        Children,
        Current,
        None
    }

    internal class DirectoryGetter
    {
        private const int RootLevel = 0;
        private readonly DirectoryInfo _rootDirectory;
        private readonly SearchDepth _searchDepth;
        private readonly CancellationToken _cancellationToken;
        private readonly List<string> _log;

        internal DirectoryGetter(
            string rootPath,
            SearchDepth searchDepth,
            CancellationToken cancellationToken,
            List<string> log)
        {
            if (!Directory.Exists(rootPath))
            {
                throw new DirectoryNotFoundException(
                    $"Could not find directory '{rootPath}'");
            }
            _rootDirectory = new DirectoryInfo(rootPath);
            if (searchDepth == SearchDepth.None)
            {
                throw new ArgumentException("Search Depth must not be null.");
            }
            _searchDepth = searchDepth;
            _cancellationToken = cancellationToken;
            _log = log;
        }

        internal List<DirectoryInfo> GetDirectories()
        {
            var directories = new List<DirectoryInfo>();
            GetDirectories(_rootDirectory, directories, RootLevel);
            return directories;
        }

        private void GetDirectories(
            DirectoryInfo directory,
            ICollection<DirectoryInfo> directories,
            int currentLevel)
        {
            directories.Add(directory);
            if (_searchDepth == SearchDepth.Current) return;
            if (_searchDepth == SearchDepth.Children &&
                currentLevel > 0)
                return;
            try
            {
                foreach (var subDirectory in directory.GetDirectories())
                {
                    _cancellationToken.ThrowIfCancellationRequested();
                    GetDirectories(
                        subDirectory, directories, currentLevel + 1);
                }
            }
            catch (PathTooLongException e)
            {
                _log.Add(e.Message);
            }
            catch (UnauthorizedAccessException e)
            {
                _log.Add(e.Message);
            }
        }
    }
}

DirectoryFormatter

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Security.AccessControl;
using System.Security.Principal;
using System.Threading;
using Extensions.PrimitiveExtensions;

namespace DirectoryPermissionsChecker
{
    internal class DirectoryFormatter
    {
        private const char Comma = ',';
        private const char Quote = '"';
        private const char Newline = '\n';

        private readonly IEnumerable<DirectoryInfo> _directories;
        private readonly CancellationToken _cancellationToken;
        private readonly List<string> _log;

        internal DirectoryFormatter(
            IEnumerable<DirectoryInfo> directories, 
            CancellationToken cancellationToken, 
            List<string> log)
        {
            _directories = directories;
            _cancellationToken = cancellationToken;
            _log = log;
        }

        internal string FormatDirectories()
        {
            var result = string.Empty;
            var directoriesParts = new List<DirectoryParts>();
            foreach (var directory in _directories)
            {
                _cancellationToken.ThrowIfCancellationRequested();
                try
                {
                    var fullName = directory.FullName;
                    var fullNameSplit = fullName.Split(
                        Path.DirectorySeparatorChar).Where(
                        s => !s.IsNullOrWhiteSpace()).ToArray();
                    directoriesParts.Add(
                        new DirectoryParts
                        {
                            SplitPath = fullNameSplit,
                            AccessRules =
                                directory.GetAccessControl().GetAccessRules(
                                    true, true, typeof(NTAccount))
                        });
                }
                catch (PathTooLongException e)
                {
                    _log.Add(e.Message);
                }
                catch (UnauthorizedAccessException e)
                {
                    _log.Add(e.Message);
                }
            }
            var longestPath = 0;
            foreach (var directoryParts in directoriesParts)
            {
                _cancellationToken.ThrowIfCancellationRequested();
                if (directoryParts.SplitPath.Length > longestPath)
                    longestPath = directoryParts.SplitPath.Length;
            }
            for (var i = 0; i < longestPath; i++)
            {
                result += $"Level {i}".Wrap(Quote) + Comma;
            }
            result += string.Join(
                Comma.ToString(),
                "Identity".Wrap(Quote),
                "File System Rights".Wrap(Quote),
                "Access Control Type".Wrap(Quote),
                "Is Inherited?".Wrap(Quote),
                Newline);
            foreach (var directoryParts in directoriesParts)
            {
                _cancellationToken.ThrowIfCancellationRequested();
                var pathString = string.Empty;
                for (var i = 0; i < longestPath; i++)
                {
                    _cancellationToken.ThrowIfCancellationRequested();
                    if (i < directoryParts.SplitPath.Length)
                    {
                        pathString += directoryParts.SplitPath[i].Wrap(Quote) +
                                  Comma;
                    }
                    else
                    {
                        pathString += string.Empty.Wrap(Quote) + Comma;
                    }
                }
                foreach (FileSystemAccessRule accessRule in
                    directoryParts.AccessRules)
                {
                    _cancellationToken.ThrowIfCancellationRequested();
                    result += pathString + string.Join(
                        Comma.ToString(),
                        accessRule.IdentityReference.Value.Wrap(Quote),
                        accessRule.FileSystemRights.ToString().Wrap(Quote),
                        accessRule.AccessControlType.ToString().Wrap(Quote),
                        accessRule.IsInherited.ToString().Wrap(Quote),
                        Newline);
                }
            }
            return result;
        }

        private struct DirectoryParts
        {
            internal string[] SplitPath { get; set; }
            internal AuthorizationRuleCollection AccessRules { get; set; }
        }
    }
}

The code is executed by creating a new PermissionsChecker and calling Execute().

Like I said before, if I change the program so that the string building is handled inside the directory getting, thereby only iterating over each directory once, the program runs many times faster, but at the expense of being able to split the paths and then "right-align" the permissions result. I thought about displaying the permissions first, and then the path, but that goes against the spec I was given to work by.

\$\endgroup\$
  • 4
    \$\begingroup\$ This is not efficient pathString +=. Use a string builder. \$\endgroup\$ – paparazzo Jul 26 '16 at 16:11
  • \$\begingroup\$ @Paparazzi That entirely fixed the performance issue. Now Excel is just struggling to open a file with over a million records (over 200K directories with several users/groups per entry). Correction: almost 1.5M records, which is too much for Excel to open anyway. :P \$\endgroup\$ – Michael Brandon Morris Jul 26 '16 at 16:55
10
\$\begingroup\$

This is not efficient

pathString +=

String is immutable so it builds a new string every time

Use StringBuilder

\$\endgroup\$
7
\$\begingroup\$

Well, its really hard to read the code or grasp at first glance what is it about. This is due to the lack of vertical space and mixing of styles (sometimes using braces {} and sometims not using them).

E.g the PermissionChecker.Execute() method

internal async Task Execute()
{
    var task = Task.Run(
        () =>
        {
            Directories = _directoryGetter.GetDirectories();
            var resultsFileName = Path.Combine(
        OutputPath,
        $"{OutputFileName} - " +
        $"{DateTime.Now.ToString(DateTimeFormat)}{CsvExtension}");
            File.WriteAllText(
                resultsFileName,
                new DirectoryFormatter(
                    Directories, _cancellationTokenSource.Token, _log)
                .FormatDirectories());
        },
        _cancellationTokenSource.Token);
    await task;
    if (_log.IsEmpty()) return;
    var fileName = Path.Combine(
        OutputPath,
        $"{LogFileName} - " +
        $"{DateTime.Now.ToString(DateTimeFormat)}{CsvExtension}");
    File.WriteAllLines(fileName, _log);
}

would be easier to read by having some vertical space and some small easy to maintain methods like so

    internal async Task Execute()
    {
        var task = Task.Run(
            () =>
            {
                Directories = _directoryGetter.GetDirectories();

                ExportResult();

            },
            _cancellationTokenSource.Token);

        await task;

        ExportLog();
    }

    private void ExportResult()
    {
        var fileName = ComposeFileName(OutputFileName);
        var resultsFileName = Path.Combine(OutputPath, fileName);

        var formatter = new DirectoryFormatter(Directories, _cancellationTokenSource.Token, _log);

        var content = formatter.FormatDirectories()

        File.WriteAllText(resultsFileName, content);
    }

    private void ExportLog()
    {
        if(_log.IsEmpty()) { return; }

        fileName = ComposeFileName(LogFileName);
        var logFileName = Path.Combine(OutputPath, fileName);
        File.WriteAllLines(fileName, _log);
    }

    private string ComposeFileName(string fileName)
    {
        return $"{fileName} - {DateTime.Now.ToString(DateTimeFormat)}{CsvExtension}");

    }  

Now, although there are more LOC for doing the same, it is easier to read and to maintain.


Like @Paparazzi mentioned in the comments

This is not efficient pathString +=. Use a string builder.

whenever you are concatenating strings inside a loop you should use a stringbuilder instead, because using += will each time create a new string because strings are immutable.

And if you use a StringBuilder and you know in advance that it will grow very big, you should initialize it with a size near the expected end size. This is because the stringbuilder will double its size if the internal buffer is full. So initializing it with a bigger number will reduce the doubling.

\$\endgroup\$
  • \$\begingroup\$ This is only scratching the surface. \$\endgroup\$ – Heslacher Jul 26 '16 at 17:08

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.