BSON BinData is not handled correctly #28

Closed
mweibel opened this Issue Sep 19, 2012 · 18 comments

Comments

Projects
None yet
3 participants
@mweibel

mweibel commented Sep 19, 2012

See:

http://cl.ly/image/1f1Z313D012V

Thanks for the app :)

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 19, 2012

Owner

Ooh. That's an interesting one. You're using BinData for an ID? What would you expect to have happen in this case?

Owner

bobthecow commented Sep 19, 2012

Ooh. That's an interesting one. You're using BinData for an ID? What would you expect to have happen in this case?

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 19, 2012

Owner

Proper GridFS and BinData support is on my shortlist for v2.1 features, but in this case I'm not sure how best to represent it in the document heading (and incidentally, in the URL). What is your BinData ID composed of?

Owner

bobthecow commented Sep 19, 2012

Proper GridFS and BinData support is on my shortlist for v2.1 features, but in this case I'm not sure how best to represent it in the document heading (and incidentally, in the URL). What is your BinData ID composed of?

@mweibel

This comment has been minimized.

Show comment
Hide comment
@mweibel

mweibel Sep 20, 2012

Well I guess it's a little bit hard to cover all cases what a BinData can consist of.
We use UUID BinData (Subtype 3) so I think the best way for us would be if you could render it to an UUID.

The question is: What happens when someone else uses something different for it. Maybe you could define different representations for different subtypes of BinData or just use the same format as e.g. the mongo CLI displays (and for the URL you could urlencode it or something like that..)

Thanks for this tool btw. it's nice to finally have a good looking and working tool :)

mweibel commented Sep 20, 2012

Well I guess it's a little bit hard to cover all cases what a BinData can consist of.
We use UUID BinData (Subtype 3) so I think the best way for us would be if you could render it to an UUID.

The question is: What happens when someone else uses something different for it. Maybe you could define different representations for different subtypes of BinData or just use the same format as e.g. the mongo CLI displays (and for the URL you could urlencode it or something like that..)

Thanks for this tool btw. it's nice to finally have a good looking and working tool :)

@pstadler

This comment has been minimized.

Show comment
Hide comment
@pstadler

pstadler Sep 20, 2012

+1

Oh, by the way: before saving (updating) a record containing a BinData object, it might be necessary to prepend the keyword new to it. Example:

{ "_id" : BinData(3,"PXvPG7WxT029i0N17Corog=="), ... }     // bar = db.foo.findOne();
{ "_id" : new BinData(3,"PXvPG7WxT029i0N17Corog=="), ...}  // db.foo.save(bar);

+1

Oh, by the way: before saving (updating) a record containing a BinData object, it might be necessary to prepend the keyword new to it. Example:

{ "_id" : BinData(3,"PXvPG7WxT029i0N17Corog=="), ... }     // bar = db.foo.findOne();
{ "_id" : new BinData(3,"PXvPG7WxT029i0N17Corog=="), ...}  // db.foo.save(bar);
@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 20, 2012

Owner

Something like this?

I'm thinking we use a special case for the document title when it's a UUID BinData, otherwise just the base64 encoded binary string.

Owner

bobthecow commented Sep 20, 2012

Something like this?

I'm thinking we use a special case for the document title when it's a UUID BinData, otherwise just the base64 encoded binary string.

@mweibel

This comment has been minimized.

Show comment
Hide comment
@mweibel

mweibel Sep 20, 2012

I think that's perfect! Very nice :)

mweibel commented Sep 20, 2012

I think that's perfect! Very nice :)

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 20, 2012

Owner

Awesome. It's 95% implemented, and will be in v2.1. Along with the ID fix, I fixed BinData rendering and editing in the whole document (as you can see from the screenshot).

Owner

bobthecow commented Sep 20, 2012

Awesome. It's 95% implemented, and will be in v2.1. Along with the ID fix, I fixed BinData rendering and editing in the whole document (as you can see from the screenshot).

@pstadler

This comment has been minimized.

Show comment
Hide comment
@pstadler

pstadler Sep 20, 2012

Great, thanks a lot! :-)

Great, thanks a lot! :-)

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 20, 2012

Owner

Committed and pushed to the develop branch. Do me a favor and run genghis.php or genghis.rb from the dev branch locally and make sure it does what you think it should?

Thanks!

Owner

bobthecow commented Sep 20, 2012

Committed and pushed to the develop branch. Do me a favor and run genghis.php or genghis.rb from the dev branch locally and make sure it does what you think it should?

Thanks!

@mweibel

This comment has been minimized.

Show comment
Hide comment
@mweibel

mweibel Sep 21, 2012

http://cl.ly/image/1x1B41051h3m

Thanks a lot, seems to work besides saving which tells me that "t" is undefined.

Also when editing, the textarea is very small initially (you can resize it but it would be nice to have the full size).

When editing documents with BinData in it it might be helpful to add a "new" before the BinData statements because otherwise I think it won't work.

But really nice work :)

mweibel commented Sep 21, 2012

http://cl.ly/image/1x1B41051h3m

Thanks a lot, seems to work besides saving which tells me that "t" is undefined.

Also when editing, the textarea is very small initially (you can resize it but it would be nice to have the full size).

When editing documents with BinData in it it might be helpful to add a "new" before the BinData statements because otherwise I think it won't work.

But really nice work :)

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 21, 2012

Owner

It will work with or without the new in Genghis. It's awesome that way :)

I'm not sure about the "t" and editor... That's definitely not what the editor should be showing. For some reason the rich editor isn't showing up.

Owner

bobthecow commented Sep 21, 2012

It will work with or without the new in Genghis. It's awesome that way :)

I'm not sure about the "t" and editor... That's definitely not what the editor should be showing. For some reason the rich editor isn't showing up.

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 21, 2012

Owner

Also, shouldn't that be BinData(3, ...) instead of BinData("3", ...)?

Owner

bobthecow commented Sep 21, 2012

Also, shouldn't that be BinData(3, ...) instead of BinData("3", ...)?

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 21, 2012

Owner

Hmm. The rich editor doesn't show up when editing a doc with BinData, but does for all other docs. Looks like a bug :)

Owner

bobthecow commented Sep 21, 2012

Hmm. The rich editor doesn't show up when editing a doc with BinData, but does for all other docs. Looks like a bug :)

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 21, 2012

Owner

Okay. Should be fixed now. Take a look?

Owner

bobthecow commented Sep 21, 2012

Okay. Should be fixed now. Take a look?

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 21, 2012

Owner

Just pushed a fix for that quoted subtype argument.

Owner

bobthecow commented Sep 21, 2012

Just pushed a fix for that quoted subtype argument.

@mweibel

This comment has been minimized.

Show comment
Hide comment
@mweibel

mweibel Sep 23, 2012

Hi,

You're so fast!

I'll take a look on tuesday because then I'm back at the office.

Thanks again :)

mweibel commented Sep 23, 2012

Hi,

You're so fast!

I'll take a look on tuesday because then I'm back at the office.

Thanks again :)

@bobthecow

This comment has been minimized.

Show comment
Hide comment
@bobthecow

bobthecow Sep 27, 2012

Owner

This has been merged into develop and will be going out with v2.1.

Owner

bobthecow commented Sep 27, 2012

This has been merged into develop and will be going out with v2.1.

@bobthecow bobthecow closed this Sep 27, 2012

@mweibel

This comment has been minimized.

Show comment
Hide comment
@mweibel

mweibel Sep 28, 2012

Hey, sorry didn't had time on tuesday for it. It works perfectly :) Thanks a lot

mweibel commented Sep 28, 2012

Hey, sorry didn't had time on tuesday for it. It works perfectly :) Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment