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

SetJSONBody() shouldn't call SetCompressed(true) #30

Open
tleyden opened this issue Mar 8, 2018 · 0 comments
Open

SetJSONBody() shouldn't call SetCompressed(true) #30

tleyden opened this issue Mar 8, 2018 · 0 comments
Assignees
Labels
Milestone

Comments

@tleyden
Copy link
Contributor

tleyden commented Mar 8, 2018

This code doesn't work as expected:

	changesRequest := blip.NewRequest()
	changesRequest.SetCompressed(false)
	changesRequest.SetProfile("changes")
	changesRequest.SetJSONBody(changes)

because SetJSONBody() ends up reverting the compression setting:

func (m *Message) SetJSONBody(value interface{}) error {
	body, err := json.Marshal(value)
	if err == nil {
		m.SetBody(body)
		m.Properties["Content-Type"] = "application/json"
		m.SetCompressed(true)  // <--- reverts earlier setting
	}
	return err
}

This was very confusing and I had to spend time debugging this.

I would like to change this, but before this is changed there would need to do be review of the callers of SetJSONBody() to make sure SetCompressed(true) would need to be enabled where appropriate.

At the very least, the API should be documented to warn users about this.

@djpongh djpongh added this to the 2.0.0 milestone Mar 8, 2018
@djpongh djpongh added P1: high and removed P1: high labels Mar 8, 2018
@djpongh djpongh modified the milestones: 2.0.0, 2.1.0 Mar 9, 2018
@djpongh djpongh added the icebox label Mar 9, 2018
@adamcfraser adamcfraser modified the milestones: 2.1.0, 2.2.0 Apr 2, 2018
@djpongh djpongh modified the milestones: 2.5.0, Iridium Aug 30, 2018
@djpongh djpongh added backlog and removed icebox labels Sep 6, 2018
@djpongh djpongh modified the milestones: Iridium, Cobalt Nov 19, 2018
@djpongh djpongh removed this from the Cobalt milestone Dec 13, 2018
@djpongh djpongh added this to the Mercury milestone Dec 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants