Improvements of working with binary files

[Maximys]

@dj-nitehawk , hello. I want to make little improvements for working with binary files on MongoDB.Entities. I want make inheritance DataStreamer from Stream. What do you think about this feature? Will you approve my PR with this changes or you think it'll large mistake for architecture?

[dj-nitehawk]

why exactly would you like to do that? what can be gained from doing that?

if the reasoning is good enough we can make inheritance from stream. but keep in mind that we cannot change the current public API of the library as it's already being used in many projects. I personally have about 6 large projects.

so there can be no breaking changes. also all the tests should pass without changing the tests.

let me know your thoughts. I'm open for improvements.

thanks!

[Maximys]

First of all, it'll provide more usual interface for working with binary files. Secondly, end user of your library should not add one more additional variable (Stream). Thirdly, we provide to end user synchronous and asynchronous way for operate objects. We can mark current DownloadAsync and UploadAsync method by ObsoleteAttribute and delete those methods after next major release.

[dj-nitehawk]

i feel like inheriting from Stream would make the public API more verbose than it is right now.

can you tell me which methods/properties of Stream class you're planning to override and what the public api would look like afterwards? can you show examples for uploading/downloading after proposed change?

also i strongly believe we shouldn't provide sync methods for IO bound work like this and should always be async.

PS:

end user of your library should not add one more additional variable (Stream)

95% of the time, the target stream is not created by user (at least in my projects) and comes from asp.net/webapi Response.Body or IFormFile.OpenReadStream(). so i don't think this is a big deal.

but yeah, let's see your example api and i might be convinced.

[dj-nitehawk]

we can however leave the current FileEntity.Data property alone and provide a new FileEntity.Stream property. which will return a new class called "DataStream" which you could add leaving the current "DataStreamer" class alone. so we would be not doing any breaking changes and providing a new stream interface for the api.

let me know your thoughts?

[Maximys]

Just for sample:

    public class MediaFile : FileEntity
    {
        public Guid Id { get; set; }
        public string Name { get; set; }

        public MediaFile()
        {
        }
    }

    [HttpGet]
    [Route("mediaContents/getFile/{fileId}")]
    public HttpResponseMessage GetFile(Guid fileId)
    {  
        MediaFile requiredFile;
        HttpResponseMessage returnValue;

        new DB("mediaContentStorage", "localhost", 27017);
        requiredFile = DB.Queryable<MediaFile>().SingleOrDefault(mf => mf.Id == fileId);
        if (requiredFile != default)
        {
            returnValue = Request.CreateResponse(HttpStatusCode.OK);
            returnValue.Content = new StreamContent(requiredFile.Data);
            returnValue.Content.Headers.ContentDisposition = new ContentDispositionHeaderValue("attachment");
            returnValue.Content.Headers.ContentDisposition.FileName = requiredFile.Name;
            returnValue.Content.Headers.ContentType = new MediaTypeHeaderValue("application/octet-stream");
        }
        else
        {
            returnValue = Request.CreateResponse(HttpStatusCode.NotFound);
        }
        return returnValue;
    }

instead:

    public async Task<HttpResponseMessage> GetFile(Guid fileId)
    {  
        MediaFile requiredFile;
        HttpResponseMessage returnValue;

        new DB("mediaContentStorage", "localhost", 27017);
        requiredFile = DB.Queryable<MediaFile>().SingleOrDefault(mf => mf.Id == fileId);
        if (requiredFile != default)
        {
            //boilerplate code for stream preparation
            var resultStream = new MemoryStream();
            await requiredFile.DownloadAsync(resultStream);
            //===
            returnValue = Request.CreateResponse(HttpStatusCode.OK);
            returnValue.Content = new StreamContent(resultStream);
            returnValue.Content.Headers.ContentDisposition = new ContentDispositionHeaderValue("attachment");
            returnValue.Content.Headers.ContentDisposition.FileName = requiredFile.Name;
            returnValue.Content.Headers.ContentType = new MediaTypeHeaderValue("application/octet-stream");
        }
        else
        {
            returnValue = Request.CreateResponse(HttpStatusCode.NotFound);
        }
        return returnValue;
    }

I did not compile first and second samples, that's why they can contain some lexical errors.

[Maximys]

we can however leave the current FileEntity.Data property alone and provide a new FileEntity.Stream property. which will return a new class called "DataStream" which you could add leaving the current "DataStreamer" class alone. so we would be not doing any breaking changes and providing a new stream interface for the api.

let me know your thoughts?

Why you think, that inheriting DataStreamer from Stream will be breaking change? As I already wrote, I'll just mark DownloadAsync and UploadAsync by ObsoleteAttribute.

[dj-nitehawk]

this is what i usually do without boilerplate:

    public async Task<ActionResult> GetFile(Guid fileId)
    {
        new DB("mediaContentStorage", "localhost", 27017);

        var requiredFile = DB.Queryable<MediaFile>().SingleOrDefault(mf => mf.Id == fileId);

        if (requiredFile != default)
        {
            Response.StatusCode = (int)HttpStatusCode.OK;
            Response.ContentType = "application/octet-stream";
            await requiredFile.Data.DownloadAsync(Response.Body);
            return new EmptyResult();
        }
        else
        {
            Response.StatusCode = (int)HttpStatusCode.NotFound;
            return new EmptyResult();
        }
    }

but yeah all good. let's leave current functionality alone and add a new class called "DataStream" which inherits from "Stream" and new property called "FileEntity.Stream". if you do that as well as write tests for the new functionality i can accept your PR. in a future date, once the "DataStream" class is mature enough, we can deprecate the old functionality. by doing that, no existing projects need any refactoring.

deal?

PS: pls submit PR to the dev branch.

[dj-nitehawk]

also, in my projects i cannot expose MongoDB.Entities classes or methods in my web api layer. my data access code is 2 layers down. so i need to pass down a stream from the web api layer down to the data access layer. here's an example.

[dj-nitehawk]

closing due to inactivity. feel free to re-open.