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

Whitelist and object-based columns #31

Conversation

Talljoe
Copy link

@Talljoe Talljoe commented Mar 31, 2011

One more attempt to try to contribute back my changes.

This adds the ability to specify a whitelist of columns to insert or update using an object, Type, or Dictionary. It also allows you to specify the columns to be returned in the same manner. The existing string-based option still exists.

This is the bare functionality. No attempts to clean up improper uses of casts or unused parameters in format strings. No removing redundant type specifiers or object initializers. It is a sea of Resharper yellow. No refactoring or reorganizing. Enumerables are converted to arrays so that they can immediately be used as enumerables again. And since I know what a a sore point it was for you last time, I guarantee this does not compile*.

However it does work in my test suite (a web application I'm building). It contains useful functionality, and attempts to retain backwards compatibility. If it's not useful for you, don't take it.

*Just kidding, it compiles.

@Talljoe
Copy link
Author

Talljoe commented Apr 20, 2011

Started writing some comprehensive tests and found a bug in the where code. Patch at https://github.com/Talljoe/massive/commit/d93e6216090c6a7bdd19bc835ced374ef20cbead.patch

@robconery
Copy link
Contributor

Hi Joe - it's not clear to me what is different from your approach (whitelist) vs. sending in an anonymous object or name/value collection. Put another way: is it Massive's responsibility to apply a whitelist, or is it the calling code's? I tend to think of whitelisting as orthogonal to data access.

Also - as I mention before I love that you're into committing. Being pissy when your commits don't pass muster doesn't serve you very well.

@robconery robconery closed this Apr 20, 2011
@Talljoe
Copy link
Author

Talljoe commented Apr 20, 2011

I'm sorry you mistook my humor for being pissy. It's fine if you don't take my requests, I don't take it personally and I found your previous reasons for rejecting the pull rather humorous. I have my own fork that I'm using to great benefit and I'm sharing some of the more useful parts.

Whitelisting is something everyone has to do, this removes the extra step of taking a form collection or other object and creating a new object to pass to Massive. Yes it can be done externally but then you have additional code cluttering up the usage as opposed to just adding an additional parameter. I could have created a completely separate project that just had helper methods to strip unwanted columns, but that seems like overkill. Whitelisting is so crucial to good, safe data access that it makes sense to me to add it to the library.

Since it takes time to port functionality from my fork to Massive and your standards for acceptance are so vague :) this will probably be my last unsolicited pull request to you. I'm no less committed to sharing but I need to be pragmatic about my time. If you see something you like from Talljoe/Passive let me know and I'll see about porting it.

I missed you at MIX otherwise I would have stopped and introduced myself.

Cheers,
Joe

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

Successfully merging this pull request may close these issues.

None yet

2 participants