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

api: move default response from XML to JSON #2593

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@marcaurele
Contributor

marcaurele commented Apr 23, 2018

Description

Moving the default response type (when not provided) from XML to JSON.

Java community used to love XML a lot, but let's face it, nowadays people are using JSON whenever they can.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

How Has This Been Tested?

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly. (apache/cloudstack-docs#25)
    Testing
  • I have added tests to cover my changes.
  • All relevant new and existing integration tests have passed.
  • A full integration testsuite with all test that can run on my environment has passed.
@rafaelweingartner

This comment has been minimized.

Show comment
Hide comment
@rafaelweingartner

rafaelweingartner Apr 23, 2018

Member

This can break compatibility with tools that assume the default return as XML. Should'nt we discuss it first in the users and dev lists?

Member

rafaelweingartner commented Apr 23, 2018

This can break compatibility with tools that assume the default return as XML. Should'nt we discuss it first in the users and dev lists?

@@ -170,12 +170,6 @@ void processRequestInContext(final HttpServletRequest req, final HttpServletResp
try {
if (HttpUtils.RESPONSE_TYPE_JSON.equalsIgnoreCase(responseType)) {

This comment has been minimized.

@rhtyd

rhtyd May 1, 2018

Member

I think we should keep this, otherwise people won't be able to get XML output if a response type was provided.

@rhtyd

rhtyd May 1, 2018

Member

I think we should keep this, otherwise people won't be able to get XML output if a response type was provided.

This comment has been minimized.

@marcaurele

marcaurele May 2, 2018

Contributor

It's some dummy code. The content type is set when writing the String response to the HttpResponse later on. Doing it so early does not have much sense. See HttpUtils.writeHttpResponse()

@marcaurele

marcaurele May 2, 2018

Contributor

It's some dummy code. The content type is set when writing the String response to the HttpResponse later on. Doing it so early does not have much sense. See HttpUtils.writeHttpResponse()

@rhtyd rhtyd added this to the 4.12.0.0 milestone May 9, 2018

@wido

This comment has been minimized.

Show comment
Hide comment
@wido

wido Jul 26, 2018

Contributor

I agree with @rafaelweingartner

This would be a rather big change and I think we should discuss it properly. I understand the change, but we will break a lot of things.

Do I hear somebody saying CloudStack 5.0?

Contributor

wido commented Jul 26, 2018

I agree with @rafaelweingartner

This would be a rather big change and I think we should discuss it properly. I understand the change, but we will break a lot of things.

Do I hear somebody saying CloudStack 5.0?

@DaanHoogland

This comment has been minimized.

Show comment
Hide comment
@DaanHoogland

DaanHoogland Jul 26, 2018

Contributor

let's do the 5.0 dance

Contributor

DaanHoogland commented Jul 26, 2018

let's do the 5.0 dance

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