ONOS-6332: Flows: bad parsing of EXTENSION:of:0000nnnn identifier
- added special handling of ExtensionInstructionWrappers
- added unit tests for JavaScript regexp parsing
- added unit tests for rendering instructions
- added TEST_DEPS for default_drivers
Change-Id: I668ad6ad77ea6a0ce4659497bc5813ed48b1de0a
diff --git a/web/gui/BUCK b/web/gui/BUCK
index 7c0747e..2354dfe 100644
--- a/web/gui/BUCK
+++ b/web/gui/BUCK
@@ -23,6 +23,7 @@
TEST_DEPS = [
'//lib:TEST',
'//core/api:onos-api-tests',
+ '//drivers/default:onos-drivers-default',
]
RESOURCES = {
diff --git a/web/gui/src/main/java/org/onosproject/ui/impl/FlowViewMessageHandler.java b/web/gui/src/main/java/org/onosproject/ui/impl/FlowViewMessageHandler.java
index 41fb033..6e49dba 100644
--- a/web/gui/src/main/java/org/onosproject/ui/impl/FlowViewMessageHandler.java
+++ b/web/gui/src/main/java/org/onosproject/ui/impl/FlowViewMessageHandler.java
@@ -95,6 +95,14 @@
private static final String ONOS_PREFIX = "org.onosproject.";
private static final String ONOS_MARKER = "*";
+ // json structure keys
+ private static final String IMMED = "immed";
+ private static final String DEFER = "defer";
+ private static final String METER = "meter";
+ private static final String TABLE = "table";
+ private static final String META = "meta";
+ private static final String CLEARDEF = "clearDef";
+
// TODO: replace the use of the following constants with localized text
private static final String MSG_NO_SELECTOR =
"(No traffic selector criteria for this flow)";
@@ -344,7 +352,7 @@
if (!instructs.isEmpty()) {
sb.append(type).append(SQUARE_O);
for (Instruction i : instructs) {
- sb.append(i).append(COMMA);
+ sb.append(renderInstructionForDisplay(i)).append(COMMA);
}
removeTrailingComma(sb);
sb.append(SQUARE_C).append(COMMA);
@@ -456,17 +464,25 @@
private ArrayNode jsonInstrList(List<Instruction> instructions) {
ArrayNode array = arrayNode();
for (Instruction i : instructions) {
- array.add(i.toString());
+ array.add(renderInstructionForDisplay(i));
}
return array;
}
}
- // json structure keys
- private static final String IMMED = "immed";
- private static final String DEFER = "defer";
- private static final String METER = "meter";
- private static final String TABLE = "table";
- private static final String META = "meta";
- private static final String CLEARDEF = "clearDef";
+ // package private to allow unit test access...
+ String renderInstructionForDisplay(Instruction instr) {
+
+ // special handling for Extension Instruction Wrappers...
+ if (instr instanceof Instructions.ExtensionInstructionWrapper) {
+ Instructions.ExtensionInstructionWrapper wrap =
+ (Instructions.ExtensionInstructionWrapper) instr;
+ return wrap.type() + COLON + wrap.extensionInstruction();
+ }
+
+ // special handling of other instruction classes could be placed here
+
+ // default to the natural string representation otherwise
+ return instr.toString();
+ }
}
diff --git a/web/gui/src/main/webapp/app/view/flow/flow.js b/web/gui/src/main/webapp/app/view/flow/flow.js
index f8c6491..2000997 100644
--- a/web/gui/src/main/webapp/app/view/flow/flow.js
+++ b/web/gui/src/main/webapp/app/view/flow/flow.js
@@ -273,7 +273,10 @@
function selCb($event, row) {
if ($scope.selId) {
- wss.sendEvent(detailsReq, { flowId: row.id, appId: row.appId });
+ wss.sendEvent(detailsReq, {
+ flowId: row.id,
+ appId: row.appId
+ });
} else {
$scope.hidePanel();
}
@@ -301,65 +304,67 @@
.directive('flowDetailsPanel',
['$rootScope', '$window', '$timeout', 'KeyService',
- function ($rootScope, $window, $timeout, ks) {
- return function (scope) {
- var unbindWatch;
+ function ($rootScope, $window, $timeout, ks) {
+ return function (scope) {
+ var unbindWatch;
- function heightCalc() {
- pStartY = fs.noPxStyle(d3.select('.tabular-header'), 'height')
- + mast.mastHeight() + topPdg;
- wSize = fs.windowSize(pStartY);
- pHeight = wSize.height;
- }
+ function heightCalc() {
+ var tabhead = d3.select('.tabular-header');
- function initPanel() {
- heightCalc();
- createDetailsPane();
- }
+ pStartY = fs.noPxStyle(tabhead, 'height') +
+ mast.mastHeight() + topPdg;
+ wSize = fs.windowSize(pStartY);
+ pHeight = wSize.height;
+ }
- // Safari has a bug where it renders the fixed-layout table wrong
- // if you ask for the window's size too early
- if (scope.onos.browser === 'safari') {
- $timeout(initPanel);
- } else {
- initPanel();
+ function initPanel() {
+ heightCalc();
+ createDetailsPane();
+ }
+
+ // Safari has a bug where it renders the fixed-layout
+ // table wrong if you ask for the window's size too early
+ if (scope.onos.browser === 'safari') {
+ $timeout(initPanel);
+ } else {
+ initPanel();
+ }
+ // create key bindings to handle panel
+ ks.keyBindings({
+ esc: [handleEscape, 'Close the details panel'],
+ _helpFormat: ['esc'],
+ });
+ ks.gestureNotes([
+ ['click', 'Select a row to show cluster node details'],
+ ['scroll down', 'See available cluster nodes'],
+ ]);
+ // if the panelData changes
+ scope.$watch('panelData', function () {
+ if (!fs.isEmptyObject(scope.panelData)) {
+ populateDetails(scope.panelData);
+ detailsPanel.show();
}
- // create key bindings to handle panel
- ks.keyBindings({
- esc: [handleEscape, 'Close the details panel'],
- _helpFormat: ['esc'],
- });
- ks.gestureNotes([
- ['click', 'Select a row to show cluster node details'],
- ['scroll down', 'See available cluster nodes'],
- ]);
- // if the panelData changes
- scope.$watch('panelData', function () {
+ });
+ // if the window size changes
+ unbindWatch = $rootScope.$watchCollection(
+ function () {
+ return {
+ h: $window.innerHeight,
+ w: $window.innerWidth,
+ };
+ }, function () {
if (!fs.isEmptyObject(scope.panelData)) {
+ heightCalc();
populateDetails(scope.panelData);
- detailsPanel.show();
}
- });
- // if the window size changes
- unbindWatch = $rootScope.$watchCollection(
- function () {
- return {
- h: $window.innerHeight,
- w: $window.innerWidth,
- };
- }, function () {
- if (!fs.isEmptyObject(scope.panelData)) {
- heightCalc();
- populateDetails(scope.panelData);
- }
- }
- );
+ }
+ );
- scope.$on('$destroy', function () {
- unbindWatch();
- ps.destroyPanel(pName);
- });
- };
- }]);
+ scope.$on('$destroy', function () {
+ unbindWatch();
+ ps.destroyPanel(pName);
+ });
+ };
+ }]);
}());
diff --git a/web/gui/src/main/webapp/tests/app/view/flow/flow-spec.js b/web/gui/src/main/webapp/tests/app/view/flow/flow-spec.js
index 48ca797..dc0fe47 100644
--- a/web/gui/src/main/webapp/tests/app/view/flow/flow-spec.js
+++ b/web/gui/src/main/webapp/tests/app/view/flow/flow-spec.js
@@ -39,4 +39,41 @@
var controller = createController();
});
+ // Unit test for defect ONOS-6332
+ // See lines 164-167 addLabVal(...) of flow.js which handles display of
+ // properties in the details panel
+ // (this is if we keep the device ID in the extension rendered string)
+ it('should break into two pieces, using first colon only', function () {
+ var lv = 'EXTENSION:of:0000000000000205/Ofdpa3SetMplsType{mplsType=32}',
+ b1 = 'EXTENSION',
+ b2 = 'of:0000000000000205/Ofdpa3SetMplsType{mplsType=32}';
+
+ var bits = lv.match(/^([^:]*):(.*)/);
+
+ // console.log(bits);
+
+ expect(bits.length).toBe(3);
+ expect(bits[0]).toBe(lv);
+ expect(bits[1]).toBe(b1);
+ expect(bits[2]).toBe(b2);
+ });
+
+ // Unit test for defect ONOS-6332
+ // See lines 164-167 addLabVal(...) of flow.js which handles display of
+ // properties in the details panel
+ // (this is if we DON'T keep the device ID in the extension rendered string)
+ it('should break into two pieces, using first colon only (2)', function () {
+ var lv = 'EXTENSION:Ofdpa3SetMplsType{mplsType=32}',
+ b1 = 'EXTENSION',
+ b2 = 'Ofdpa3SetMplsType{mplsType=32}';
+
+ var bits = lv.match(/^([^:]*):(.*)/);
+
+ // console.log(bits);
+
+ expect(bits.length).toBe(3);
+ expect(bits[0]).toBe(lv);
+ expect(bits[1]).toBe(b1);
+ expect(bits[2]).toBe(b2);
+ });
});
diff --git a/web/gui/src/test/java/org/onosproject/ui/impl/FlowViewMessageHandlerTest.java b/web/gui/src/test/java/org/onosproject/ui/impl/FlowViewMessageHandlerTest.java
new file mode 100644
index 0000000..ea2c5e8
--- /dev/null
+++ b/web/gui/src/test/java/org/onosproject/ui/impl/FlowViewMessageHandlerTest.java
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2017-present Open Networking Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.ui.impl;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.onosproject.driver.extensions.Ofdpa3SetMplsType;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.flow.instructions.ExtensionTreatment;
+import org.onosproject.net.flow.instructions.Instruction;
+import org.onosproject.net.flow.instructions.Instructions;
+
+import static org.junit.Assert.assertEquals;
+import static org.onosproject.net.DeviceId.deviceId;
+import static org.onosproject.net.PortNumber.portNumber;
+
+/**
+ * Unit tests for {@link FlowViewMessageHandler}.
+ */
+public class FlowViewMessageHandlerTest extends AbstractUiImplTest {
+
+ private static final String DEV_OF_204 = "of:0000000000000204";
+
+ private static final String EXT_FULL_STR =
+ "EXTENSION:of:0000000000000204/Ofdpa3SetMplsType{mplsType=32}";
+ private static final String EXT_NO_DPID =
+ "EXTENSION:Ofdpa3SetMplsType{mplsType=32}";
+
+ private FlowViewMessageHandler handler;
+ private Instruction instr;
+ private String string;
+ private String render;
+
+
+ @Before
+ public void setUp() {
+ handler = new FlowViewMessageHandler();
+ }
+
+ @Test
+ public void renderOutputInstruction() {
+ title("renderOutputInstruction");
+ instr = Instructions.createOutput(portNumber(4));
+ string = instr.toString();
+ render = handler.renderInstructionForDisplay(instr);
+
+ print(string);
+ assertEquals("not same output", string, render);
+ assertEquals("not output to port 4", "OUTPUT:4", render);
+ }
+
+
+ @Test
+ public void renderExtensionInstruction() {
+ title("renderExtensionInstruction");
+
+ ExtensionTreatment extn = new Ofdpa3SetMplsType((short) 32);
+ DeviceId devid = deviceId(DEV_OF_204);
+
+ instr = Instructions.extension(extn, devid);
+ string = instr.toString();
+ render = handler.renderInstructionForDisplay(instr);
+
+ print(string);
+ print(render);
+
+ assertEquals("unexpected toString", EXT_FULL_STR, string);
+ assertEquals("unexpected short string", EXT_NO_DPID, render);
+ }
+}