Web Api to upload excel file

Naming Convention

current code is not using camelCase naming convention on local variables, and also is not given a good naming to some variables like DisplayFileName, Month, DirectoryCreate ..etc.it should be cameled-case.

Some variables have wrong naming like DisplayFileName it should be file because it's used for Request.Form.Files[0] and not to Request.Form.Files[0].FileName. dbPath is not best name for explaining a file that is related to the stored one. It could be changed to fileToComparePath since it's going to be compared with the input file and the exportPath can be fileExportPath.

it's important to always differs between Directory and File paths by either suffix the file or dir to the name, otherwise, it won't be a clear enough to other developers.

When dealing with paths you should always use the built-in methods for like System.IO.File and System.IO.Directory. This would ensure the compatibility with the current system requirements.

Short names are fast to type, easy to miss, and hard to follow. So, try to avoid short names like CExcel ex = new CExcel(); it would be better to name it excel rather than ex, and also var m in mods in the foreach loop.

Always give your variables a good explainable naming.

using

with using clause, there is no need to Flush or Close or Dispose, as it would automatically do that at the end of the block.

So this :

using (var stream = new FileStream(dbPath, FileMode.Create))
{
    Request.Form.Files[0].CopyTo(stream);
    stream.Flush();
    stream.Close();
}

should be like this :

using (var stream = new FileStream(dbPath, FileMode.Create))
{
   // you should use Write(stream) instead, or reset the stream position.
    Request.Form.Files[0].CopyTo(stream); 
}

Unused variables FinalPath, filedata, inputTemplatePath, con why they're still there ? you should remove unused variables.

Other Notes

• When working with files and directories, it would be wise to use the built-in methods Directory, File and Path to avoid any unexcepted exceptions that have been already covered by them.
• with if conditions, don't put the exception or the error at the else, keep it at the top as it would be clearer than to be at the end of the code.
• when doing ToList() it'll return a new List.
• with Stream always use Write instead of CopyTo as Write is specific use and optimized for Stream, while CopyTo is built for all compatible collections types.

Code Comments

here is some comments on the code (code has been shorten for brevity).

if (DisplayFileName.Length > 0) // should be inverted.
{
//  var filedata = ContentDispositionHeaderValue.Parse(Request.Form.Files[0].ContentDisposition).FileName.Trim('"'); //unused 
 
    var dbPath = Path.Combine(DirectoryCreate, fileName); // this would return ../path/month/fileName-SomeGUID.xlsx/fileName-SomeGUID.xlsx

    using (var stream = new FileStream(dbPath, FileMode.Create))
    {
        Request.Form.Files[0].CopyTo(stream);
        // the Flush() and Close are handled by the `using` blocks
//      stream.Flush();
//      stream.Close();
    }
    
//  GC.Collect(); // unnecessary
    
    string error = ""; // use null;
    
    int rowCount = 0;
    
//  string inputTemplatePath = ""; //unused
  
    var InputfilePath = System.IO.Path.Combine(GetFilesDownload, "DeliveryGeneration_Input.xlsx");
    
    bool areIdentical = ex.CompareExcel(dbPath, InputfilePath, out rowCount, out error); // rowCount and error have never been checked. what happens if they have values ? 


    if (areIdentical == true)
    {
//      List<InputExcel> inputexcellist = new List<InputExcel>();// unnecessary
        
        List<InputExcel> inputexcellist = ex.Import(dbPath);
       
//      List<string> mods = new List<string>(); // unnecessary
        
        // it could be used in the foreach directlry  
        List<string> mods = inputexcellist.Select(x => x.ModuleName).Distinct().ToList();
        
        var OutputfilePath = System.IO.Path.Combine(GetFilesDownload, "DeliveryGeneration_Output.xlsx");
        
         // This is unnecessary as the `File.Copy` ovewrite flag is true.
         // Although Directory.Exists should be File.Exists 
        // if (System.IO.Directory.Exists(Path.Combine(exportDirectory, fileName)))
        // {
             // throw new Exception("Ok so the error message IS right.");
        // }

        // Path.Combine(exportDirectory, fileName) has the same value of exportPath 
        System.IO.File.Copy(OutputfilePath, Path.Combine(exportDirectory, fileName), true);


//      SqlConnection con;  // unused

        foreach (var m in mods) // m should be moduleName or name
        {
//          List<InputExcel> inputmodulelist = new List<InputExcel>(); // unnecessary

            List<InputExcel> inputmodulelist = inputexcellist.Where(x => x.ModuleName == m).ToList();

            DataTable dtimport = DatatableConversion.ToDataTable(inputmodulelist); 
            
//          DataTable dtexport = new DataTable(); // unnecessary

            DataTable dtexport = _deliveryService.LoadExcelToDataTable(_connectionString, dtimport);

            ex.Export(dtexport, m, exportPath);

        }
    }
    
    
    var memory2 = new MemoryStream();
    
    using (var stream = new FileStream(exportPath, FileMode.Open))
    {
        stream.CopyTo(memory2);
    }
    
    memory2.Position = 0;
    
    
    // Path.GetFileName(exportPath) is equal to fileName
    // "text/plain" ? why ? you should use the appropriate mime type for Excel 
    // for .xls  = application/vnd.ms-excel
    // for .xlsx = application/vnd.openxmlformats-officedocument.spreadsheetml.sheet
    // or just use the ClosedXML Web API Extension
    return File(memory2, "text/plain", Path.GetFileName(exportPath)); 

}
else
{
    // invert the if, and move this at the top of blocks 
    return BadRequest();
}

Suggested Revision

if(Request.Form.Files.Length == 0)
{
    throw new InvalidOperationException(" No Files");   
}

var file = Request.Form.Files[0];

var fileName = $"{Path.GetFileNameWithoutExtension(file.FileName)}-{Guid.NewGuid()}.xlsx";

Func<string, string> generateFilePath = (path) => 
    !string.IsNullOrWhiteSpace(path) && !string.IsNullOrWhiteSpace(fileName) 
    ? Path.Combine(Directory.CreateDirectory(path).FullName, DateTime.Today.Month.ToString() , fileName) 
    : null;

var fileToComparePath = generateFilePath(myValue1);

var fileToExportPath = generateFilePath(myValue2);

if(fileToComparePath == null)
{
    throw new ArgumentNullException(nameof(fileToComparePath));
}

if(fileToExportPath == null)
{
    throw new ArgumentNullException(nameof(fileToExportPath));
}

using(var fileStream = File.Create(dbPath))
{
    fileStream.Write(file);
}

if(!ex.CompareExcel(fileToComparePath, Path.Combine(GetFilesDownload, "DeliveryGeneration_Input.xlsx"), out int rowCount, out string error))
{
    return BadRequest();
}

File.Copy(Path.Combine(GetFilesDownload, "DeliveryGeneration_Output.xlsx"), fileToExportPath, true);

CExcel excel = new CExcel();

foreach (var moduleName in excel.Import(dbPath).Select(x => x.ModuleName).Distinct())
{
    var inputModuleList = inputexcellist.Where(x => x.ModuleName == moduleName).ToList();

    var tableImport = DatatableConversion.ToDataTable(inputModuleList);
    
    var tableExport = _deliveryService.LoadExcelToDataTable(_connectionString, tableImport);

    excel.Export(tableExport, moduleName, fileToExportPath);
}

MemoryStream excelFileStream = new MemoryStream();

using(var fileStream = File.Open(fileToExportPath, FileMode.Open))
{
    fileStream.Write(excelFileStream);
}

return File(excelFileStream, "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", fileName);