Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SendResponse Overloads for Byte Array and MemoryStream #164

Closed
archeg opened this issue Jan 27, 2017 · 3 comments
Closed

Add SendResponse Overloads for Byte Array and MemoryStream #164

archeg opened this issue Jan 27, 2017 · 3 comments
Assignees
Milestone

Comments

@archeg
Copy link

archeg commented Jan 27, 2017

Right now Grapevine only supports text responses or filestream responses. It would be nice if you could also respond with an array of bytes or memory stream (I think internally it is available, so just need to make it public)

Also I'm not sure what headers are necessary to set if you want to sent let's say image as byte array - so it would be nice if API could handle that (but this is optional as API already provides a way to set headers manually)

@scottoffen scottoffen changed the title Send a byte array as the response Add SendResponse Overloads for Byte Array and MemoryStream Jan 27, 2017
@scottoffen scottoffen self-assigned this Jan 27, 2017
@scottoffen scottoffen added this to the 4.0.0.x milestone Jan 27, 2017
@scottoffen
Copy link
Owner

scottoffen commented Jan 27, 2017

I like this; it's caused me to rethink the options I've provided for sending responses. Here's what I think should be done to provide the biggest bang for the buck, and allow for maximum reusability and extensibility with the least impact to the existing user base.

  1. Refactor the interface - hear me out! - refactor the interface to only have one SendResponse method that takes, as it's input, a byte array.

  2. Change all other SendResponse methods to be extensions methods on the interface that, ultimately, call the SendResponse(ByteArray) method. After all, they are just convenience methods anyway.

This would mean that the next time a new SendResponse method is needed that doesn't already exist, they can write their own extension without needing me to do anything.

Thoughts? Objections? Praise?

@archeg
Copy link
Author

archeg commented Jan 30, 2017

Looks good. This should clear up the interface, but still providing reach functionality as before.

@scottoffen
Copy link
Owner

I have the initial work for this one up in it's own branch. I just need to double-check code coverage and then I can merge this into master.

scottoffen pushed a commit that referenced this issue Feb 8, 2017
This moves functionality for sending responses primarily to extension
methods, while HttpResponse only exposes a single SendResponse method (one
that takes a byte array). All extension methods simply set properties and
then create a byte array from the response body. Extension methods will
now opperate on a Stream object instead of specifically on a FileStream
object, allowing the data to be sent to come from any stream, while also
allowing *Stream specific overloads to be written for specific cases.

Resolves Issue #164
scottoffen pushed a commit that referenced this issue Feb 9, 2017
This moves functionality for sending responses primarily to extension
methods, while HttpResponse only exposes a single SendResponse method (one
that takes a byte array). All extension methods simply set properties and
then create a byte array from the response body. Extension methods will
now opperate on a Stream object instead of specifically on a FileStream
object, allowing the data to be sent to come from any stream, while also
allowing *Stream specific overloads to be written for specific cases.

Resolves Issue #164
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants