Skip to content
Permalink
Browse files

Merge pull request #1971 from telstra/improve-add-vlan-details-1640

Add error messages for conflicts on create flow
  • Loading branch information...
sergii-iakovenko committed Mar 14, 2019
2 parents 56488cc + f3c011d commit 8f185de28fed67f687479435210704ae800a15be
@@ -278,7 +278,7 @@ class FlowCrudSpec extends BaseSpecification {
then: "Error is returned, stating a readable reason of conflict"
def error = thrown(HttpClientErrorException)
error.statusCode == HttpStatus.CONFLICT
error.responseBodyAsString.to(MessageError).errorMessage == data.getError(flow)
error.responseBodyAsString.to(MessageError).errorMessage == data.getError(flow, conflictingFlow)

and: "Cleanup: delete the dominant flow"
flowHelper.deleteFlow(flow.id)
@@ -289,8 +289,8 @@ class FlowCrudSpec extends BaseSpecification {
makeFlowsConflicting: { FlowPayload dominantFlow, FlowPayload flowToConflict ->
flowToConflict.id = dominantFlow.id
},
getError : { FlowPayload flowToError ->
"Could not create flow: Flow $flowToError.id already exists"
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict ->
"Could not create flow: Flow $dominantFlow.id already exists"
}
]
}
@@ -316,7 +316,7 @@ class FlowCrudSpec extends BaseSpecification {
then: "Error is returned, stating a readable reason of conflict"
def error = thrown(HttpClientErrorException)
error.statusCode == HttpStatus.CONFLICT
error.responseBodyAsString.to(MessageError).errorMessage == data.getError(flow1, "update")
error.responseBodyAsString.to(MessageError).errorMessage == data.getError(flow1, conflictingFlow, "update")

and: "Cleanup: delete flows"
[flow1, flow2].each { flowHelper.deleteFlow(it.id) }
@@ -438,9 +438,17 @@ class FlowCrudSpec extends BaseSpecification {
}

@Shared
def portError = { String operation, int port, SwitchId switchId, String flowId ->
"Could not $operation flow: The port $port on the switch '$switchId' has already occupied " +
"by the flow '$flowId'."
def errorMessage = { String operation, FlowPayload flow, String endpoint, FlowPayload conflictingFlow,
String conflictingEndpoint ->
"Could not $operation flow: Requested flow '$conflictingFlow.id' conflicts with existing flow '$flow.id'. " +
"Details: requested flow '$conflictingFlow.id' $conflictingEndpoint: " +
"switch=${conflictingFlow."$conflictingEndpoint".datapath} " +
"port=${conflictingFlow."$conflictingEndpoint".portNumber} " +
"vlan=${conflictingFlow."$conflictingEndpoint".vlanId}, " +
"existing flow '$flow.id' $endpoint: " +
"switch=${flow."$endpoint".datapath} " +
"port=${flow."$endpoint".portNumber} " +
"vlan=${flow."$endpoint".vlanId}"
}

/**
@@ -577,9 +585,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.source.portNumber = dominantFlow.source.portNumber
flowToConflict.source.vlanId = dominantFlow.source.vlanId
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.source.portNumber, flowToError.source.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "source", flowToConflict, "source")
}
],
[
@@ -588,9 +596,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.destination.portNumber = dominantFlow.destination.portNumber
flowToConflict.destination.vlanId = dominantFlow.destination.vlanId
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.destination.portNumber, flowToError.destination.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "destination", flowToConflict, "destination")
}
],
[
@@ -599,9 +607,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.source.portNumber = dominantFlow.source.portNumber
flowToConflict.source.vlanId = 0
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.source.portNumber, flowToError.source.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "source", flowToConflict, "source")
}
],
[
@@ -610,9 +618,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.destination.portNumber = dominantFlow.destination.portNumber
flowToConflict.destination.vlanId = 0
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.destination.portNumber, flowToError.destination.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "destination", flowToConflict, "destination")
}
],
[
@@ -621,9 +629,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.source.portNumber = dominantFlow.source.portNumber
dominantFlow.source.vlanId = 0
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.source.portNumber, flowToError.source.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "source", flowToConflict, "source")
}
],
[
@@ -632,9 +640,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.destination.portNumber = dominantFlow.destination.portNumber
dominantFlow.destination.vlanId = 0
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.destination.portNumber, flowToError.destination.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "destination", flowToConflict, "destination")
}
],
[
@@ -644,9 +652,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.source.vlanId = 0
dominantFlow.source.vlanId = 0
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.source.portNumber, flowToError.source.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "source", flowToConflict, "source")
}
],
[
@@ -656,9 +664,9 @@ class FlowCrudSpec extends BaseSpecification {
flowToConflict.destination.vlanId = 0
dominantFlow.destination.vlanId = 0
},
getError : { FlowPayload flowToError, String operation = "create" ->
portError(operation, flowToError.destination.portNumber, flowToError.destination.datapath,
flowToError.id)
getError : { FlowPayload dominantFlow, FlowPayload flowToConflict,
String operation = "create" ->
errorMessage(operation, dominantFlow, "destination", flowToConflict, "destination")
}
]
]
@@ -79,56 +79,102 @@ void checkFlowForEndpointConflicts(Flow requestedFlow) throws FlowValidationExce
// Check the source
Collection<Flow> conflictsOnSource;
conflictsOnSource = flowRepository.findFlowIdsByEndpoint(requestedFlow.getSrcSwitch().getSwitchId(),
requestedFlow.getSrcPort());
requestedFlow.getSrcPort());


Optional<String> conflictedFlow = conflictsOnSource.stream()
Optional<Flow> conflictSrcSrc = conflictsOnSource.stream()
.filter(flow -> !requestedFlow.getFlowId().equals(flow.getFlowId()))
.filter(flow -> (flow.getSrcSwitch().getSwitchId().equals(requestedFlow.getSrcSwitch().getSwitchId())
&& flow.getSrcPort() == requestedFlow.getSrcPort()
&& (flow.getSrcVlan() == requestedFlow.getSrcVlan() || flow.getSrcVlan() == 0
|| requestedFlow.getSrcVlan() == 0))
|| (flow.getDestSwitch().getSwitchId().equals(requestedFlow.getSrcSwitch().getSwitchId())
&& flow.getDestPort() == requestedFlow.getSrcPort()
&& (flow.getDestVlan() == requestedFlow.getSrcVlan() || flow.getDestVlan() == 0
|| requestedFlow.getSrcVlan() == 0)))
.map(Flow::getFlowId)
.filter(flow -> flow.getSrcSwitch().getSwitchId().equals(requestedFlow.getSrcSwitch().getSwitchId())
&& flow.getSrcPort() == requestedFlow.getSrcPort()
&& (flow.getSrcVlan() == requestedFlow.getSrcVlan()
|| flow.getSrcVlan() == 0 || requestedFlow.getSrcVlan() == 0))
.findAny();
if (conflictedFlow.isPresent()) {
throw new FlowValidationException(
format("The port %d on the switch '%s' has already occupied by the flow '%s'.",
requestedFlow.getSrcPort(),
requestedFlow.getSrcSwitch().getSwitchId(),
conflictedFlow.get()),
ErrorType.ALREADY_EXISTS);

if (conflictSrcSrc.isPresent()) {
String errorMessage = format("Requested flow '%s' conflicts with existing flow '%s'. "
+ "Details: "
+ "requested flow '%s' source: switch=%s port=%d vlan=%d, "
+ "existing flow '%s' %s: switch=%s port=%d vlan=%d",
requestedFlow.getFlowId(), conflictSrcSrc.get().getFlowId(),
requestedFlow.getFlowId(), requestedFlow.getSrcSwitch().getSwitchId().toString(),
requestedFlow.getSrcPort(), requestedFlow.getSrcVlan(),
conflictSrcSrc.get().getFlowId(), conflictSrcSrc.get().isForward() ? "source" : "destination",
conflictSrcSrc.get().getSrcSwitch().getSwitchId().toString(),
conflictSrcSrc.get().getSrcPort(), conflictSrcSrc.get().getSrcVlan());
throw new FlowValidationException(errorMessage, ErrorType.ALREADY_EXISTS);
}

Optional<Flow> conflictDstSrc = conflictsOnSource.stream()
.filter(flow -> !requestedFlow.getFlowId().equals(flow.getFlowId()))
.filter(flow -> flow.getDestSwitch().getSwitchId().equals(requestedFlow.getSrcSwitch().getSwitchId())
&& flow.getDestPort() == requestedFlow.getSrcPort()
&& (flow.getDestVlan() == requestedFlow.getSrcVlan()
|| flow.getDestVlan() == 0 || requestedFlow.getSrcVlan() == 0))
.findAny();

if (conflictDstSrc.isPresent()) {
String errorMessage = format("Requested flow '%s' conflicts with existing flow '%s'. "
+ "Details: "
+ "requested flow '%s' source: switch=%s port=%d vlan=%d, "
+ "existing flow '%s' %s: switch=%s port=%d vlan=%d",
requestedFlow.getFlowId(), conflictDstSrc.get().getFlowId(),
requestedFlow.getFlowId(), requestedFlow.getSrcSwitch().getSwitchId().toString(),
requestedFlow.getSrcPort(), requestedFlow.getSrcVlan(),
conflictDstSrc.get().getFlowId(), conflictDstSrc.get().isForward() ? "destination" : "source",
conflictDstSrc.get().getDestSwitch().getSwitchId().toString(),
conflictDstSrc.get().getDestPort(), conflictDstSrc.get().getDestVlan());
throw new FlowValidationException(errorMessage, ErrorType.ALREADY_EXISTS);
}

// Check the destination
Collection<Flow> conflictsOnDest;
conflictsOnDest = flowRepository.findFlowIdsByEndpoint(
requestedFlow.getDestSwitch().getSwitchId(),
requestedFlow.getDestPort());
requestedFlow.getDestSwitch().getSwitchId(),
requestedFlow.getDestPort());


conflictedFlow = conflictsOnDest.stream()
Optional<Flow> conflictSrcDst = conflictsOnDest.stream()
.filter(flow -> !requestedFlow.getFlowId().equals(flow.getFlowId()))
.filter(flow -> (flow.getSrcSwitch().getSwitchId().equals(requestedFlow.getDestSwitch().getSwitchId())
&& flow.getSrcPort() == requestedFlow.getDestPort()
&& (flow.getSrcVlan() == requestedFlow.getDestVlan() || flow.getSrcVlan() == 0
|| requestedFlow.getDestVlan() == 0))
|| (flow.getDestSwitch().getSwitchId().equals(requestedFlow.getDestSwitch().getSwitchId())
&& flow.getDestPort() == requestedFlow.getDestPort()
&& (flow.getDestVlan() == requestedFlow.getDestVlan() || flow.getDestVlan() == 0
|| requestedFlow.getDestVlan() == 0)))
.map(Flow::getFlowId)
.filter(flow -> flow.getSrcSwitch().getSwitchId().equals(requestedFlow.getDestSwitch().getSwitchId())
&& flow.getSrcPort() == requestedFlow.getDestPort()
&& (flow.getSrcVlan() == requestedFlow.getDestVlan()
|| flow.getSrcVlan() == 0 || requestedFlow.getDestVlan() == 0))
.findAny();
if (conflictedFlow.isPresent()) {
throw new FlowValidationException(
format("The port %d on the switch '%s' has already occupied by the flow '%s'.",
requestedFlow.getDestPort(),
requestedFlow.getDestSwitch().getSwitchId(),
conflictedFlow.get()),
ErrorType.ALREADY_EXISTS);

if (conflictSrcDst.isPresent()) {
String errorMessage = format("Requested flow '%s' conflicts with existing flow '%s'. "
+ "Details: "
+ "requested flow '%s' destination: switch=%s port=%d vlan=%d, "
+ "existing flow '%s' %s: switch=%s port=%d vlan=%d",
requestedFlow.getFlowId(), conflictSrcDst.get().getFlowId(),
requestedFlow.getFlowId(), requestedFlow.getDestSwitch().getSwitchId().toString(),
requestedFlow.getDestPort(), requestedFlow.getDestVlan(),
conflictSrcDst.get().getFlowId(), conflictSrcDst.get().isForward() ? "source" : "destination",
conflictSrcDst.get().getSrcSwitch().getSwitchId().toString(),
conflictSrcDst.get().getSrcPort(), conflictSrcDst.get().getSrcVlan());
throw new FlowValidationException(errorMessage, ErrorType.ALREADY_EXISTS);
}

Optional<Flow> conflictDstDst = conflictsOnDest.stream()
.filter(flow -> !requestedFlow.getFlowId().equals(flow.getFlowId()))
.filter(flow -> flow.getDestSwitch().getSwitchId().equals(requestedFlow.getDestSwitch().getSwitchId())
&& flow.getDestPort() == requestedFlow.getDestPort()
&& (flow.getDestVlan() == requestedFlow.getDestVlan()
|| flow.getDestVlan() == 0 || requestedFlow.getDestVlan() == 0))
.findAny();

if (conflictDstDst.isPresent()) {
String errorMessage = format("Requested flow '%s' conflicts with existing flow '%s'. "
+ "Details: "
+ "requested flow '%s' destination: switch=%s port=%d vlan=%d, "
+ "existing flow '%s' %s: switch=%s port=%d vlan=%d",
requestedFlow.getFlowId(), conflictDstDst.get().getFlowId(),
requestedFlow.getFlowId(), requestedFlow.getDestSwitch().getSwitchId().toString(),
requestedFlow.getDestPort(), requestedFlow.getDestVlan(),
conflictDstDst.get().getFlowId(), conflictDstDst.get().isForward() ? "destination" : "source",
conflictDstDst.get().getDestSwitch().getSwitchId().toString(),
conflictDstDst.get().getDestPort(), conflictDstDst.get().getDestVlan());
throw new FlowValidationException(errorMessage, ErrorType.ALREADY_EXISTS);
}
}

0 comments on commit 8f185de

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.