search

syndesisio / syndesis-rest

The API for Syndesis - a flexible, customizable, cloud-hosted platform that provides core integration capabilities as a service. It leverages Red Hat's existing product architecture using OpenShift Online/Dedicated and Fuse Integration Services.
https://syndesis-staging.b6ff.rh-idev.openshiftapps.com/api/v1/
Apache License 2.0
6 stars 17 forks source link

Failing to set configuredProperties returns a non-JSON error #300

Closed gashcrumb closed 7 years ago

gashcrumb commented 7 years ago

If I just zip through the integration editor and pick an HTTP start/finish connection and don't even bother to fill in a URI no configuredProperties field is set on those steps. When saving the integration, instead of getting back a JSON response I get back a java exception:

com.fasterxml.jackson.databind.JsonMappingException: configuredProperties value at [Source: io.undertow.servlet.spec.ServletInputStreamImpl@328e215; line: 43, column: 7] (through reference chain: com.redhat.ipaas.model.integration.Integration$Builder["steps"]->java.util.ArrayList[0]->com.redhat.ipaas.model.integration.Step$Builder["configuredProperties"])
KurtStam commented 7 years ago

Yeah looks like configuredProperties is not optional. Will fix! @gashcrumb Should name and stepKind be optional too?

gashcrumb commented 7 years ago

stepKind definitly shouldn't be optional.

I'm not certain configuredProperties should be optional or not, this issue is really more about the fact that the response is not JSON but a raw string instead, I can't do much with this response client-side.

jimmidyson commented 7 years ago

Right - this issue is not about changing configuredProperties to be optional, but to ensure that all error messages that come back to clients are JSON formatted. I thought that had been sorted previously...

KurtStam commented 7 years ago

The issue is that we need to use the spring way of handling rest errors, not the resteasy way; that no longer works when we switched to spring. But.. if the UI lets you save w/o properties then they should be optional?

jimmidyson commented 7 years ago

Please don't conflate two different issues - open a new issue for the configuredProperties if you want.

jimmidyson commented 7 years ago

So in the case https://github.com/redhat-ipaas/ipaas-rest/issues/247#issuecomment-290330949 that's returning a JSON response properly. What am I missing?

gashcrumb commented 7 years ago

Not sure! Was doing some quick testing this morning and came across this behavior. Again, whether or not configuredProperties should be set, I don't get consistent responses back for HTTP calls, sometimes I get back json which is awesome, other times I get back whatevs which is less awesome :-)

KurtStam commented 7 years ago

@gashcrumb can this be closed now that Jimmi fixed the exception-mapper https://github.com/redhat-ipaas/ipaas-rest/issues/247? Or do you still want configuredProperties to be optional?

gashcrumb commented 7 years ago

Oh, sure!