Skip to content

Commit

Permalink
Sitemap and Item config parsing adjustments (#1843)
Browse files Browse the repository at this point in the history 
- Adapt sitemap and item lexers to changes in icon name syntax
- Restrict the elements that can be added to a sitemap
- Added extra sitemap validations (in line with xtext validation in
core)
- Added test for sitemap parser and validation

This solves the issue in main UI created by openhab/openhab-core#3539 and openhab/openhab-core#3378.

Signed-off-by: Mark Herwege <[email protected]>
  • Loading branch information
@mherwege
mherwege authored May 5, 2023
1 parent 095bb2f commit cb253f6
Show file tree
Hide file tree
Showing 6 changed files with 301 additions and 25 deletions.
26 changes: 17 additions & 9 deletions bundles/org.openhab.ui/web/src/assets/items-lexer.nearley
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
itemtype: ['Group ', 'Number ', 'Switch ', 'Rollershutter ', 'String ', 'Dimmer ', 'Contact ', 'DateTime ', 'Color ', 'Player ', 'Location ', 'Call ', 'Image '],
membertype: [':Number', ':Switch', ':Rollershutter', ':String', ':Dimmer', ':Contact', ':DateTime', ':Color', ':Player', ':Location', ':Call', ':Image'],
aggfunc: ['AVG', 'SUM', 'MIN', 'MAX', 'OR', 'AND', 'COUNT', 'LATEST', 'EARLIEST', 'EQUALITY'],
identifier: /[A-Za-z0-9_-]+/,
identifier: /(?:[A-Za-z_][A-Za-z0-9_]*)|(?:[0-9]+[A-Za-z_][A-Za-z0-9_]*)/,
lparen: '(',
rparen: ')',
colon: ':',
Expand All @@ -21,6 +21,8 @@
gt: '>',
comma: ',',
equals: '=',
colon: ':',
hyphen: '-',
NL: { match: /\n/, lineBreaks: true },
})
%}
Expand Down Expand Up @@ -54,7 +56,7 @@ Type -> %itemtype {% (d) => [d[0].text.trim()]
| "Group" %membertype {% (d) => ['Group', d[1].text.substring(1)] %}
| "Group" %membertype ":" %aggfunc AggArgs {% (d) => ['Group', d[1].text.substring(1), {name: d[3].text, params: d[4]}] %}
| "Group" %membertype ":" %identifier {% (d) => ['Group', 'Number:' + d[3].text] %}
| "Group" %membertype ":" %identifier ":" %aggfunc AggArgs {% (d) => ['Group', 'Number:' + d[3].text, {name: d[5].text, params: d[6]}] %}
| "Group" %membertype ":" %identifier ":" %aggfunc AggArgs {% (d) => ['Group', 'Number:' + d[3].text, {name: d[5].text, params: d[6]}] %}
# group function aggregation arguments
AggArgs -> null {% (d) => undefined %}
| "(" %identifier ")" {% (d) => [d[1].text] %}
Expand All @@ -69,7 +71,13 @@ Label -> null {% (d) => undefined %}

# Icon (category)
Icon -> null {% (d) => undefined %}
| __ "<" _ %identifier _ ">" {% (d) => d[3].text %}
| __ "<" _ IconValue _ ">" {% (d) => d[3].join("") %}
IconValue -> %string
| IconName
| %identifier %colon IconName
| %identifier %colon %identifier %colon IconName
IconName -> %identifier
| %identifier %hyphen IconName {% (d) => d[0].value + "-" + d[2] %}

# Groups
Groups -> null {% (d) => undefined %}
Expand Down Expand Up @@ -107,14 +115,14 @@ MetadataConfigValue -> %string {% (d) => d[0].value %}
| %number {% (d) => parseInt(d[0].value) %}


_ -> null {% () => null %}
| _ %WS {% () => null %}
| _ %NL {% () => null %}
_ -> null {% () => null %}
| _ %WS {% () => null %}
| _ %NL {% () => null %}
| _ %comment {% () => null %}

__ -> %WS {% () => null %}
| %NL {% () => null %}
__ -> %WS {% () => null %}
| %NL {% () => null %}
| %comment {% () => null %}
| __ %WS {% () => null %}
| __ %NL {% () => null %}
| __ %comment {% () => null %}
| __ %comment {% () => null %}
11 changes: 10 additions & 1 deletion bundles/org.openhab.ui/web/src/assets/sitemap-lexer.nearley
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
gt: '>',
equals: '=',
comma: ',',
colon: ':',
hyphen: '-',
NL: { match: /\n/, lineBreaks: true },
boolean: /(?:true)|(?:false)/,
identifier: /(?:[A-Za-z_][A-Za-z0-9_]*)|(?:[0-9]+[A-Za-z_][A-Za-z0-9_]*)/,
Expand Down Expand Up @@ -99,12 +101,19 @@ WidgetAttr -> %widgetswitchattr
| %widgetfrcitmattr WidgetBooleanAttrValue {% (d) => ['forceAsItem', d[1]] %}
| %widgetboolattr WidgetBooleanAttrValue {% (d) => [d[0].value, d[1]] %}
| %widgetfreqattr WidgetAttrValue {% (d) => ['frequency', d[1]] %}
| %icon WidgetIconAttrValue {% (d) => [d[0].value, d[1].join("")] %}
| WidgetAttrName WidgetAttrValue {% (d) => [d[0][0].value, d[1]] %}
| WidgetVisibilityAttrName WidgetVisibilityAttrValue {% (d) => [d[0][0].value, d[1]] %}
| WidgetColorAttrName WidgetColorAttrValue {% (d) => [d[0][0].value, d[1]] %}
WidgetAttrName -> %item | %label | %icon | %widgetattr
WidgetAttrName -> %item | %label | %widgetattr
WidgetBooleanAttrValue -> %boolean {% (d) => (d[0].value === 'true') %}
| %string {% (d) => (d[0].value === 'true') %}
WidgetIconAttrValue -> %string
| WidgetIconName
| %identifier %colon WidgetIconName
| %identifier %colon %identifier %colon WidgetIconName
WidgetIconName -> %identifier
| %identifier %hyphen WidgetIconName {% (d) => d[0].value + "-" + d[2] %}
WidgetAttrValue -> %number {% (d) => { return parseFloat(d[0].value) } %}
| %identifier {% (d) => d[0].value %}
| %string {% (d) => d[0].value %}
Expand Down
49 changes: 48 additions & 1 deletion ...rg.openhab.ui/web/src/components/pagedesigner/sitemap/__tests__/sitemap-code_jest.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,57 @@ describe('SitemapCode', () => {
})
})

it('parses a segmented icon name with hyphens', async () => {
expect(wrapper.vm.sitemapDsl).toBeDefined()
// simulate updating the sitemap in code
const sitemap = [
'sitemap test label="Test" {',
' Default item=Item_Icon icon=iconify:wi:day-sunny-overcast',
' Default item=Item_Icon_String icon="iconify:wi:day-sunny-overcast"',
'}',
''
].join('\n')
wrapper.vm.updateSitemap(sitemap)
expect(wrapper.vm.sitemapDsl).toMatch(/^sitemap test label="Test"/)
expect(wrapper.vm.parsedSitemap.error).toBeFalsy()

await wrapper.vm.$nextTick()

// check whether an 'updated' event was emitted and its payload
// (should contain the parsing result for the new sitemap definition)
const events = wrapper.emitted().updated
expect(events).toBeTruthy()
expect(events.length).toBe(1)
const payload = events[0][0]
expect(payload.slots).toBeDefined()
expect(payload.slots.widgets).toBeDefined()
expect(payload.slots.widgets.length).toBe(2)
expect(payload.slots.widgets[0]).toEqual({
component: 'Default',
config: {
item: 'Item_Icon',
icon: 'iconify:wi:day-sunny-overcast'
}
})
expect(payload.slots.widgets[1]).toEqual({
component: 'Default',
config: {
item: 'Item_Icon_String',
icon: 'iconify:wi:day-sunny-overcast'
}
})
})

it('parses a mapping code back to a mapping on a component', async () => {
expect(wrapper.vm.sitemapDsl).toBeDefined()
// simulate updating the sitemap in code
wrapper.vm.updateSitemap('sitemap test label="Test" {\n Selection item=Scene_General mappings=[1=Morning,2="Evening",10="Cinéma",11=TV,3="Bed time",4=Night]\n}\n')
const sitemap = [
'sitemap test label="Test" {',
' Selection item=Scene_General mappings=[1=Morning,2="Evening",10="Cinéma",11=TV,3="Bed time",4=Night]',
'}',
''
].join('\n')
wrapper.vm.updateSitemap(sitemap)
expect(wrapper.vm.sitemapDsl).toMatch(/^sitemap test label="Test"/)
expect(wrapper.vm.parsedSitemap.error).toBeFalsy()

Expand Down
3 changes: 3 additions & 0 deletions bundles/org.openhab.ui/web/src/css/app.styl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,6 @@ html
--f7-toast-text-color white
--f7-button-text-color white
--f7-button-border-color white

.sitemap-validation-dialog
--f7-dialog-width 80%
168 changes: 166 additions & 2 deletions ...s/org.openhab.ui/web/src/pages/settings/pages/sitemap/__tests__/sitemap-edit_jest.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,83 @@ describe('SitemapEdit', () => {
expect(wrapper.vm.sitemap.component).toEqual('Sitemap')
})

it('validates frame does not contain frames', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Frame')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Frame')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0].slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'label', 'Frame Test')

// should not validate as the frame contains a frame
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Frame widget Frame Test, frame not allowed in frame/)
})

it('validates frame in text does not contain frames', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Frame')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Text')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0].slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Frame')
await wrapper.vm.$nextTick()

// should validate, as frame in text in frame is allowed
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeFalsy()

// add a frame inside the frame in the text
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0].slots.widgets[0].slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Frame')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0].slots.widgets[0].slots.widgets[0].slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'label', 'Frame Test')

// should not validate as the frame contains a frame
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Frame widget Frame Test, frame not allowed in frame/)
})

it('validates only frames or no frames at all', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Frame')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Text')
await wrapper.vm.$nextTick()

// should not validate as mix of frame and text
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Widget without label, only frames or no frames at all allowed in linkable widget/)
})

it('validates item name is checked', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Frame')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Switch')
Expand Down Expand Up @@ -94,7 +167,7 @@ describe('SitemapEdit', () => {
expect(lastDialogConfig).toBeFalsy()
})

it('validates period is checked', async () => {
it('validates period is configured and valid', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Chart')
Expand All @@ -108,7 +181,16 @@ describe('SitemapEdit', () => {
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Chart widget Chart Test, no period configured/)
expect(lastDialogConfig.content).toMatch(/Chart widget Chart Test, invalid period configured: undefined/)

// configure an invalid period for the Chart
lastDialogConfig = null
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'period', '5h')
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Chart widget Chart Test, invalid period configured: 5h/)

// configure a period for the Chart and check that there are no validation errors anymore
lastDialogConfig = null
Expand All @@ -119,6 +201,88 @@ describe('SitemapEdit', () => {
expect(lastDialogConfig).toBeFalsy()
})

it('validates step is positive', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Slider')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'item', 'Item1')
localVue.set(wrapper.vm.selectedWidget.config, 'label', 'Slider Test')

// no step, should validate
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeFalsy()

// configure a negative step, should not validate
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'step', -1)
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Slider widget Slider Test, step size cannot be 0 or negative: -1/)

// configure a 0 step, should not validate
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'step', 0)
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Slider widget Slider Test, step size cannot be 0 or negative: 0/)

// configure a positive step, should validate
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'step', 5)
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeFalsy()
})

it('validates minValue less than or equal maxValue', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
wrapper.vm.addWidget('Setpoint')
await wrapper.vm.$nextTick()
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'item', 'Item1')
localVue.set(wrapper.vm.selectedWidget.config, 'label', 'Setpoint Test')

// no minValue or maxValue, should validate
lastDialogConfig = null
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeFalsy()

// configure a minValue more than maxValue, should not validate
lastDialogConfig = null
localVue.set(wrapper.vm.selectedWidget.config, 'minValue', 10)
localVue.set(wrapper.vm.selectedWidget.config, 'maxValue', 5)
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeTruthy()
expect(lastDialogConfig.content).toMatch(/Setpoint widget Setpoint Test, minValue must be less than or equal maxValue: 10 > 5/)

// configure a minValue equal to maxValue, should validate
lastDialogConfig = null
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'minValue', 5)
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeFalsy()

// configure a minValue less to maxValue, should validate
lastDialogConfig = null
wrapper.vm.selectWidget([wrapper.vm.sitemap.slots.widgets[0], wrapper.vm.sitemap])
await wrapper.vm.$nextTick()
localVue.set(wrapper.vm.selectedWidget.config, 'minValue', 1)
wrapper.vm.validateWidgets()
expect(lastDialogConfig).toBeFalsy()
})

it('validates mappings', async () => {
wrapper.vm.selectWidget([wrapper.vm.sitemap, null])
await wrapper.vm.$nextTick()
Expand Down
Loading

0 comments on commit cb253f6

Please sign in to comment.