Skip to content

Commit

Permalink
feat: allow publishing of empty message (#724)
Browse files Browse the repository at this point in the history
* feat: allow publishing of empty message

* feat: improve publishing error message for incorrect headers + bindings
  • Loading branch information
timonback authored May 3, 2024
1 parent 9e7a4be commit 67d859e
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 64 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ public ResponseEntity<String> publish(@RequestParam String topic, @RequestBody M
}

PublishingPayloadCreator.Result result = publishingPayloadCreator.createPayloadObject(message);
if (result.payload() != null) {
if (result.errorMessage().isEmpty()) {
publishMessage(topic, message, result.payload());
return ResponseEntity.ok().build();
}
return ResponseEntity.badRequest().body(result.errorMessage());
return ResponseEntity.badRequest().body(result.errorMessage().get());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import io.github.springwolf.core.asyncapi.components.ComponentsService;
import io.github.springwolf.core.controller.dtos.MessageDto;
import jakarta.annotation.Nullable;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.lang3.StringUtils;

import java.text.MessageFormat;
import java.util.Optional;
import java.util.Set;

/**
Expand All @@ -26,19 +28,23 @@ public class PublishingPayloadCreator {
public Result createPayloadObject(MessageDto message) {
String messagePayloadType = message.getPayloadType();

if (MessageDto.EMPTY.equals(message.getPayload())) {
return new Result(null, Optional.empty());
}

Set<String> knownSchemaNames = componentsService.getSchemas().keySet();
for (String schemaPayloadType : knownSchemaNames) {
// security: match against user input, but always use our controlled data from the DefaultSchemaService
if (schemaPayloadType != null && schemaPayloadType.equals(messagePayloadType)) {
try {
Class<?> payloadClass = Class.forName(schemaPayloadType);
Object payload = objectMapper.readValue(message.getPayload(), payloadClass);
return new Result(payload, null);
return new Result(payload, Optional.empty());
} catch (ClassNotFoundException | JsonProcessingException ex) {
String errorMessage = MessageFormat.format(
"Unable to create payload {0} from data: {1}", schemaPayloadType, message.getPayload());
log.info(errorMessage, ex);
return new Result(null, errorMessage);
return new Result(null, Optional.of(errorMessage));
}
}
}
Expand All @@ -47,9 +53,9 @@ public Result createPayloadObject(MessageDto message) {
"Specified payloadType {0} is not a registered springwolf schema.", messagePayloadType);
String knownPayloadsMessage =
MessageFormat.format(" Known payloadTypes: [{0}]", StringUtils.join(knownSchemaNames, ", "));
log.info(errorMessage + knownPayloadsMessage);
return new Result(null, errorMessage);
log.info("{}{}", errorMessage, knownPayloadsMessage);
return new Result(null, Optional.of(errorMessage));
}

public record Result(Object payload, String errorMessage) {}
public record Result(@Nullable Object payload, Optional<String> errorMessage) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@
@Builder
@Jacksonized
public class MessageDto {
public static final String EMPTY = "";

private final Map<String, String> bindings;

private final Map<String, String> headers;

@Builder.Default
private final String payload = "";
private final String payload = EMPTY;

@Builder.Default
private final String payloadType = String.class.getCanonicalName();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
Expand Down Expand Up @@ -97,40 +96,36 @@ void setup() {
}

@Test
void testControllerShouldReturnBadRequestIfPayloadIsEmpty() {
try {
String content =
"""
void testControllerShouldReturnBadRequestIfPayloadIsEmpty() throws Exception {
String content =
"""
{
"bindings": null,
"headers": null,
"payload": ""
}""";
mvc.perform(post("/springwolf/jms/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().isBadRequest());
} catch (Exception e) {
verifyNoInteractions(springwolfJmsProducer);
}
mvc.perform(post("/springwolf/jms/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().is2xxSuccessful());

verify(springwolfJmsProducer).send(eq("test-topic"), isNull(), eq(null));
}

@Test
void testControllerShouldReturnBadRequestIfPayloadIsNotSet() {
try {
String content =
"""
void testControllerShouldAcceptIfPayloadIsNotSet() throws Exception {
String content =
"""
{
"bindings": null,
"headers": null
}""";
mvc.perform(post("/springwolf/jms/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().isBadRequest());
} catch (Exception e) {
verifyNoInteractions(springwolfJmsProducer);
}
mvc.perform(post("/springwolf/jms/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().is2xxSuccessful());

verify(springwolfJmsProducer).send(eq("test-topic"), isNull(), eq(null));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
Expand Down Expand Up @@ -97,40 +96,36 @@ void setup() {
}

@Test
void testControllerShouldReturnBadRequestIfPayloadIsEmpty() {
try {
String content =
"""
void testControllerShouldReturnBadRequestIfPayloadIsEmpty() throws Exception {
String content =
"""
{
"bindings": null,
"headers": null,
"payload": ""
}""";
mvc.perform(post("/springwolf/kafka/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().isBadRequest());
} catch (Exception e) {
verifyNoInteractions(springwolfKafkaProducer);
}
mvc.perform(post("/springwolf/kafka/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().is2xxSuccessful());

verify(springwolfKafkaProducer).send(eq("test-topic"), isNull(), isNull(), eq(null));
}

@Test
void testControllerShouldReturnBadRequestIfPayloadIsNotSet() {
try {
String content =
"""
void testControllerShouldAcceptIfPayloadIsNotSet() throws Exception {
String content =
"""
{
"bindings": null,
"headers": null
}""";
mvc.perform(post("/springwolf/kafka/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().isBadRequest());
} catch (Exception e) {
verifyNoInteractions(springwolfKafkaProducer);
}
mvc.perform(post("/springwolf/kafka/publish?topic=test-topic")
.contentType(MediaType.APPLICATION_JSON)
.content(content))
.andExpect(status().is2xxSuccessful());

verify(springwolfKafkaProducer).send(eq("test-topic"), isNull(), isNull(), eq(null));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Operation } from "../../../models/operation.model";
import { Schema } from "../../../models/schema.model";
import { AsyncApiService } from "../../../service/asyncapi/asyncapi.service";
import { PublisherService } from "../../../service/publisher.service";
import { wrapException } from "../../../util/error-boundary";

@Component({
selector: "app-channel-main",
Expand Down Expand Up @@ -119,8 +120,20 @@ export class ChannelMainComponent implements OnInit {
bindings?: string
): void {
try {
const headersJson = JSON.parse(headers);
const bindingsJson = JSON.parse(bindings);
const headersJson =
headers === ""
? {}
: wrapException(
"Unable to convert headers to JSON object (nor is empty)",
() => JSON.parse(headers)
);
const bindingsJson =
bindings === ""
? {}
: wrapException(
"Unable to convert bindings to JSON object (nor is empty)",
() => JSON.parse(bindings)
);

this.publisherService
.publish(
Expand All @@ -136,9 +149,13 @@ export class ChannelMainComponent implements OnInit {
(err) => this.handlePublishError(err)
);
} catch (error) {
this.snackBar.open("Example payload is not valid", "ERROR", {
duration: 3000,
});
this.snackBar.open(
"Unable to create publishing payload: " + error.message,
"ERROR",
{
duration: 3000,
}
);
}
}

Expand Down
2 changes: 2 additions & 0 deletions springwolf-ui/src/app/models/example.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export class Example {

if (typeof exampleObject === "string") {
this.value = exampleObject;
} else if (Object.keys(exampleObject).length === 0) {
this.value = "";
} else {
this.value = JSON.stringify(exampleObject, null, 2);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { ServerComponents } from "./models/components.model";
import { Binding, Bindings } from "../../models/bindings.model";
import { Message } from "../../models/message.model";
import { Operation } from "../../models/operation.model";
import { catchException } from "../../util/error-boundary";

@Injectable()
export class AsyncApiMapperService {
Expand Down Expand Up @@ -353,13 +354,10 @@ export class AsyncApiMapperService {
}

private parsingErrorBoundary<T>(path: string, f: () => T): T | undefined {
try {
return f();
} catch (e) {
return catchException(f, (e) => {
this.notificationService.showError(
"Error parsing AsyncAPI " + path + ": " + e.message
);
return undefined;
}
});
}
}
28 changes: 28 additions & 0 deletions springwolf-ui/src/app/util/error-boundary.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* SPDX-License-Identifier: Apache-2.0 */
class ApplicationError extends Error {}

export function wrapException<T>(message: string, f: () => T): T {
try {
return f();
} catch (e) {
if (e instanceof Error) {
throw new ApplicationError(message + " (" + e.message + ")");
}
throw new ApplicationError(message + " (" + e + ")");
}
}

export function catchException<T>(
f: () => T,
errorHandler: (e: any) => void = () => {}
): T | undefined {
try {
return f();
} catch (e) {
if (errorHandler !== undefined) {
errorHandler(e);
}

return undefined;
}
}

0 comments on commit 67d859e

Please sign in to comment.