by ssmith via Blog on 12/10/2008 12:34:01 AM
Still working on cleaning up some legacy ASP.NET code. Here's where we are:
Now it's time to take the big step of pulling the main ugly method guts out into its own object. Since the main purpose of the method, GetImageOrFlashData(), is to store a file that has been uploaded, I'm thinking at the moment that the name for the class who will take on this responsibility is going to be CreativeFileStore (a creative is a noun in this context; a creative file is a file representing a creative, not a file that uses its left brain). What should the CreativeFileStore's interface look like?
Since we know our current implementation relies on the System.IO, it's simple enough to have the class use the WindowsFileSystem by default, resulting in a basic structure like so:
1: public class CreativeFileStore
2: {
3: private readonly IFileSystem _fileSystem;
4:
5: public CreativeFileStore() : this(new WindowsFileSystem()) {}
6:
7: public CreativeFileStore(IFileSystem fileSystem)
8: {
9: this._fileSystem = fileSystem;
10: }
11: }
We also need to update IFileSystem (and its implementation) to support writing files based on the code we came up with during our spike solution (in part 2). Here's the new IFileSystem interface:
1: public interface IFileSystem
3: bool FileExists(string path);
4: bool DirectoryExists(string path);
5: void CreateDirectory(string path);
6: void MoveFile(string oldPath, string newPath);
7: string ReadAllText(string path);
8: void WriteBinaryFile(byte[] bytes, string savingPath);
9: }
And here's the WindowsFileSystem implementation of WriteBinaryFile:
1: public string StorageFolderPath { get; set; }
2:
3: public WindowsFileSystem()
4: {
5: StorageFolderPath = String.Empty;
6: }
7:
8: public void WriteBinaryFile(byte[] bytes, string savingPath)
9: {
10: using (var writingStream =
11: new FileStream(
12: Path.Combine(StorageFolderPath, savingPath),
13: FileMode.OpenOrCreate)
14: )
15: {
16: writingStream.Write(bytes, 0, bytes.Length);
17: writingStream.Flush();
18: writingStream.Close();
19: }
20: }
21:
We determined in the spike solution that the windows file system would need to know an upload path, so we've added a property StorageFolderPath to represent this. It will be up to whatever part of the system creates my implementation of IFileSystem to ensure that any additional properties like this one are properly set. In any event, if a full path is given to WriteBinaryFile, the lack of a StorageFolderPath shouldn't cause it to fail. That sounds like an assumption - let's verify with a test, shall we?
Pull out the Path.Combine into a separate method (extract method) which in our case we'll make protected since nothing outside this class should need it.
WindowsFileSystem (refactored):
1: public void WriteBinaryFile(byte[] bytes, string savingPath)
3: using (var writingStream =
4: new FileStream(
5: GetFilePath(savingPath),
6: FileMode.OpenOrCreate)
7: )
9: writingStream.Write(bytes, 0, bytes.Length);
10: writingStream.Flush();
11: writingStream.Close();
12: }
13: }
14:
15: protected string GetFilePath(string savingPath)
16: {
17: return Path.Combine(StorageFolderPath, savingPath);
18: }
Then we can test it easily enough by simply subclassing, like so:
TestWindowsFileSystem
1: public class TestWindowsFileSystem : WindowsFileSystem
3: public string TestGetFilePath(string filePath)
5: return GetFilePath(filePath);
7: }
Tests
1: [TestMethod]
2: public void GetFilePath_Returns_Combined_Folder_And_File_Path()
3: {
4: string folderPath = @"C:\Foo";
5: string filePath = "bar.txt";
6: string expectedFilePath = @"C:\Foo\bar.txt";
8: TestWindowsFileSystem myFileSystem = new TestWindowsFileSystem();
9: myFileSystem.StorageFolderPath = folderPath;
10: Assert.AreEqual(expectedFilePath, myFileSystem.TestGetFilePath(filePath));
11: myFileSystem.StorageFolderPath = folderPath + @"\";
12: Assert.AreEqual(expectedFilePath, myFileSystem.TestGetFilePath(filePath));
15: [TestMethod]
16: public void GetFilePath_Returns_File_Path_When_No_Folder_Set()
17: {
18: string filePath = @"C:\Foo\bar.txt";
19: string expectedFilePath = @"C:\Foo\bar.txt";
20:
21: TestWindowsFileSystem myFileSystem = new TestWindowsFileSystem();
22: Assert.AreEqual(expectedFilePath, myFileSystem.TestGetFilePath(filePath));
23: }
These pass, so it sounds like we got this right. Now back to CreativeFileStore, which needs a method for writing files. This method replaces the goo that was in GetImageOrFlashData(), which when we're done should look pretty much like this:
1: private void GetImageOrFlashData()
3: var myCreativeFile = new CreativeFile
5: FileName = ImageOrFlashUpload.FileName,
6: Bytes = ImageOrFlashUpload.FileBytes
7: };
8:
9: var myCreativeFileStore = new CreativeFileStore();
10: try
11: {
12: myCreativeFileStore.WriteFile(myCreativeFile,
13: MyCreativeFormat, DateTime.Now);
14: }
15: catch (Exception ex)
17: FileErrorLabel.Text = ex.Message;
18: return;
In the original implementation, we called into an upload control's PostedFile property to do SaveAs(filePath). We can now replace this with a call to our file system's WriteBinaryFile() method. The resulting method looks something like this (doesn't quite compile):
CreativeFileStore.WriteFile
1: public void WriteFile(CreativeFile creativeFile, CreativeFormat expectedFormat, DateTime time)
3: string fileName = creativeFile.GenerateStandardFileName(time);
4: string directoryPath = LQPromotionServerConfig.Settings.TemporaryCreativeBaseFilePath;
5: string filePath = directoryPath + fileName;
7: if (_fileSystem.FileExists(filePath))
9: throw new IOException(@"An error occurred which did not allow the uploading of your file. " +
10: @"Please try again or contact your account manager.");
12: if (!_fileSystem.DirectoryExists(directoryPath))
13: {
14: _fileSystem.CreateDirectory(directoryPath);
15: }
16:
17: if (creativeFile.GetFileExtension() == ".swf")
18: {
19: // TODO: Put some type of check to make sure this is really a flash file.
20: _fileSystem.WriteBinaryFile(creativeFile.Bytes, filePath);
21: }
22: else // Is not Flash Type (Image)
23: {
24: try
25: {
26: // THIS CALL FAILS SINCE ImageOrFlashUpload and Server are both undefined here
27: Ardalis.Framework.Drawing.Image.FormatAndSaveUploadedImage(
28: ImageOrFlashUpload.PostedFile,
29: expectedFormat.Height,
30: expectedFormat.Width, false,
31: filePath,
32: Server.MapPath("/images/admin/blank_logo.gif"));
33: }
34: catch (Exception ex)
35: {
36: throw new ApplicationException(
37: string.Format(
38: @"There was a problem uploading your image. Please verify that the width
39: height of the image you're uploading are exactly {0} x {1} pixels.",
40: expectedFormat.Width, expectedFormat.Height), ex);
41: }
42: }
43: }
Now we need to refactor this library call so that it doesn't depend on an HttpPostedFile, then add some tests, and we're done (finally). What this code does is determine the dimensions of the uploaded image and verify they're what's expected, throwing an exception if not. To get this to compile for now, we can replace the call with the same thing that we used for .swf files:
1: _fileSystem.WriteBinaryFile(creativeFile.Bytes, filePath);
Summary
In this part, we extracted an ugly method into its own class and along the way wrote some tests and added some properties and methods to our IFileSystem and WindowsFileSystem components. The end result is a testable CreativeFileStore (we haven't written enough tests yet - I decided this was getting long enough so they'll come in the next part) that (currently) is missing a little functionality it had before (verifying image dimensions) but which we'll work on restoring in the next step.
Original Post: IFileSystem Dependency Inversion Part 4
The content of the postings is owned by the respective author. CSharpFeeds is not responsible for the contents of the postings. This site is automatically generated and cannot be reviewed for abusive content. If you find abusive content on CSharpFeeds, please contact us. Designated trademarks and brands are the property of their respective owners. All rights reserved.
