Skip to content
This repository has been archived by the owner on Jan 4, 2022. It is now read-only.

fix: building indices on arrays using prefixItems #343

Merged
merged 4 commits into from
Aug 31, 2021

Conversation

shuplenkov
Copy link
Member

@shuplenkov shuplenkov commented Aug 31, 2021

Issue being fixed or feature implemented

DPP throws an error when you try to use index on arrays using prefixItems keyword #275

What was done?

How Has This Been Tested?

With unit tests

Breaking Changes

No

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

@shuplenkov shuplenkov requested a review from shumkov August 31, 2021 12:18
@shuplenkov shuplenkov added this to the v0.21.0 milestone Aug 31, 2021
@shuplenkov shuplenkov changed the title array-indices fix: building indices on arrays using prefixItems Aug 31, 2021
@@ -189,8 +189,14 @@ module.exports = function validateDataContractFactory(

if (propertyType === 'array' && !isByteArray) {
const { items, prefixItems } = propertyDefinition;

if (prefixItems || items.type === 'object' || items.type === 'array') {
if (prefixItems && prefixItems.length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess empty prefixItems must be invalidated by schema validation.

@lgtm-com
Copy link

lgtm-com bot commented Aug 31, 2021

This pull request introduces 1 alert when merging da12217 into 2b0a86a - view on LGTM.com

new alerts:

  • 1 for Useless conditional

Comment on lines 192 to 199
if (prefixItems) {
if (
prefixItems.some((prefixItem) => prefixItem.type === 'object' || prefixItem.type === 'array')
|| !prefixItems.every((prefixItem) => prefixItem.type === prefixItems[0].type)
) {
invalidPropertyType = 'array';
}
} else if (items.type === 'object' || items.type === 'array') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (prefixItems) {
if (
prefixItems.some((prefixItem) => prefixItem.type === 'object' || prefixItem.type === 'array')
|| !prefixItems.every((prefixItem) => prefixItem.type === prefixItems[0].type)
) {
invalidPropertyType = 'array';
}
} else if (items.type === 'object' || items.type === 'array') {
const isInvalidPrefixItems = prefixItems &&
(
prefixItems.some((prefixItem) => prefixItem.type === 'object' || prefixItem.type === 'array') ||
!prefixItems.every((prefixItem) => prefixItem.type === prefixItems[0].type)
);
const isInvalidItemTypes = items.type === 'object' || items.type === 'array';
if (isInvalidPrefixItems || isInvalidItemTypes) {

Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! 👍

@shuplenkov shuplenkov merged commit 2eece92 into v0.21-dev Aug 31, 2021
@shuplenkov shuplenkov deleted the array-indices branch August 31, 2021 14:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants