Survey node defs index: added index by node def name (#251)

* survey: added survey node defs index by name * code cleanup * fixed DeepScan issue * added tests * code cleanup --------- Co-authored-by: Stefano Ricci <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
openforis · Nov 19, 2024 · 9c3bf38 · 9c3bf38
1 parent 3110dfa
commit 9c3bf38
Show file tree

Hide file tree

Showing 8 changed files with 128 additions and 45 deletions.
diff --git a/src/record/recordExpressionEvaluator/node/identifier/identifier.ts b/src/record/recordExpressionEvaluator/node/identifier/identifier.ts
@@ -8,13 +8,9 @@ import { Survey, Surveys } from '../../../../survey'
 import { RecordExpressionContext } from '../../context'
 import { NodesFinder } from './nodesFinder'
 import { SystemError } from '../../../../error'
-import { FieldValidators } from '../../../../validation'
 import { NodeValueExtractor } from '../../nodeValueExtractor'
 import { Record } from '../../../record'
 
-const isValidNodeDefName = (nodeDefName: string) =>
-  FieldValidators.name('expression.invalidNodeDefName')('name', { name: nodeDefName }).valid
-
 const getNodesOrValues = (params: {
   survey: Survey
   referencedNodes: Node[]
@@ -59,8 +55,9 @@ const evaluateIdentifierOnNode = (params: {
   const nodeDefObject = Surveys.getNodeDefByUuid({ survey, uuid: nodeDefObjectUuid })
 
   // node value prop (native)
-  if (value?.[propName] !== undefined) {
-    return value[propName]
+  const valueProp = value?.[propName]
+  if (valueProp !== undefined) {
+    return valueProp
   }
   if (NodeDefs.isAttribute(nodeDefObject)) {
     // node value prop (Arena specific value property)
@@ -74,12 +71,12 @@ const evaluateIdentifierOnNode = (params: {
     }
   }
 
-  if (!isValidNodeDefName(propName)) {
+  const nodeDefReferenced = Surveys.findNodeDefByName({ survey, name: propName })
+
+  if (!nodeDefReferenced) {
     throw new SystemError('expression.identifierNotFound', { name: propName })
   }
 
-  const nodeDefReferenced = Surveys.getNodeDefByName({ survey, name: propName })
-
   if (nodeObject.nodeDefUuid === nodeDefReferenced.uuid) {
     // the referenced node is the current node itself
     return nodeObject

diff --git a/src/record/recordExpressionEvaluator/recordExpressionEvaluator.test.ts b/src/record/recordExpressionEvaluator/recordExpressionEvaluator.test.ts
@@ -42,6 +42,7 @@ describe('RecordExpressionEvaluator', () => {
     record = createTestRecord({ user, survey })
   }, 10000)
   const queries: Query[] = [
+    { expression: 'invalid_node_name + 1', error: new SystemError('expression.identifierNotFound') },
     { expression: 'cluster_id + 1', result: 13 },
     { expression: 'cluster_id != 1', result: true },
     // !12 == null under strict logical negation semantics
@@ -75,6 +76,8 @@ describe('RecordExpressionEvaluator', () => {
     { expression: 'isEmpty(gps_model)', result: false },
     // remarks is empty
     { expression: 'isEmpty(remarks)', result: false },
+    // plot: invalid child name
+    { expression: 'plot.invalid_node_name + 1', error: new SystemError('expression.identifierNotFound') },
     // plot count is 3
     { expression: 'plot.length', result: 3 },
     // access multiple entities with index

diff --git a/src/survey/survey.ts b/src/survey/survey.ts
@@ -1,4 +1,4 @@
-import { ArenaObject } from '../common'
+import { ArenaObject, Dictionary } from '../common'
 import { AuthGroup } from '../auth'
 import { Labels, LanguageCode } from '../language'
 import { SRS } from '../srs'
@@ -48,6 +48,7 @@ export interface SurveyProps {
 export interface SurveyNodeDefsIndex {
   rootDefUuid?: string
   childDefUuidPresenceByParentUuid?: { [parentUuid: string]: { [nodeDefUuid: string]: boolean } }
+  nodeDefUuidByName?: Dictionary<string>
 }
 
 export interface Survey extends ArenaObject<SurveyProps> {
@@ -72,7 +73,7 @@ export interface Survey extends ArenaObject<SurveyProps> {
    */
   taxonomies?: { [taxonomyUuid: string]: Taxonomy }
   /**
-   * Refernce data cache (category items and taxa).
+   * Reference data cache (category items and taxa).
    */
   refData?: SurveyRefData
 

diff --git a/src/survey/surveys/_nodeDefs/nodeDefsIndex.ts b/src/survey/surveys/_nodeDefs/nodeDefsIndex.ts
@@ -1,11 +1,12 @@
-import { NodeDef } from '../../../nodeDef'
+import { NodeDef, NodeDefs } from '../../../nodeDef'
 import { Objects } from '../../../utils'
 import { Survey } from '../../survey'
 import { getNodeDefsArray } from './nodeDefsReader'
 
 const keys = {
   nodeDefsIndex: 'nodeDefsIndex',
   childDefUuidPresenceByParentUuid: 'childDefUuidPresenceByParentUuid',
+  nodeDefUuidByName: 'nodeDefUuidByName',
   rootDefUuid: 'rootDefUuid',
 }
 
@@ -23,41 +24,58 @@ export const addNodeDefToIndex =
   (survey: Survey): Survey => {
     const { sideEffect = false } = options || {}
 
-    if (nodeDef.parentUuid) {
+    const { analysis, parentUuid, temporary, uuid } = nodeDef
+    const name = NodeDefs.getName(nodeDef)
+
+    const surveyUpdated: Survey = Objects.assocPath({
+      obj: survey,
+      path: [keys.nodeDefsIndex, keys.nodeDefUuidByName, name],
+      value: nodeDef.uuid,
+      sideEffect,
+    })
+
+    if (parentUuid) {
       return Objects.assocPath({
-        obj: survey,
-        path: [keys.nodeDefsIndex, keys.childDefUuidPresenceByParentUuid, nodeDef.parentUuid, nodeDef.uuid],
+        obj: surveyUpdated,
+        path: [keys.nodeDefsIndex, keys.childDefUuidPresenceByParentUuid, parentUuid, uuid],
         value: true,
         sideEffect,
       })
     }
-    if (!nodeDef.temporary && !nodeDef.analysis) {
+    if (!temporary && !analysis) {
       // nodeDef is root
       return Objects.assocPath({
-        obj: survey,
+        obj: surveyUpdated,
         path: [keys.nodeDefsIndex, keys.rootDefUuid],
-        value: nodeDef.uuid,
+        value: uuid,
         sideEffect,
       })
     }
-    return survey
+    return surveyUpdated
   }
 
 // ==== DELETE
 
 export const deleteNodeDefIndex =
   (nodeDef: NodeDef<any>) =>
   (survey: Survey): Survey => {
+    const { parentUuid, uuid } = nodeDef
+    const name = NodeDefs.getName(nodeDef)
+
     let surveyUpdated: Survey = Objects.dissocPath({
       obj: survey,
-      path: [keys.nodeDefsIndex, keys.childDefUuidPresenceByParentUuid, nodeDef.uuid],
+      path: [keys.nodeDefsIndex, keys.childDefUuidPresenceByParentUuid, uuid],
+    })
+
+    surveyUpdated = Objects.dissocPath({
+      obj: surveyUpdated,
+      path: [keys.nodeDefsIndex, keys.nodeDefUuidByName, name],
     })
 
-    const parentUuid = nodeDef.parentUuid
     if (parentUuid) {
       surveyUpdated = Objects.dissocPath({
         obj: surveyUpdated,
-        path: [keys.nodeDefsIndex, keys.childDefUuidPresenceByParentUuid, parentUuid, nodeDef.uuid],
+        path: [keys.nodeDefsIndex, keys.childDefUuidPresenceByParentUuid, parentUuid, uuid],
       })
     } else {
       // node def is root

diff --git a/src/survey/surveys/index.ts b/src/survey/surveys/index.ts
@@ -6,6 +6,7 @@ import {
 } from './dependencies'
 
 import {
+  findNodeDefByName,
   findNodeDefByUuid,
   findNodeDefsByUuids,
   getNodeDefByName,
@@ -65,8 +66,6 @@ import {
 } from './surveysGetters'
 
 export const Surveys = {
-  findNodeDefByUuid,
-  findNodeDefsByUuids,
   getName,
   getLanguages,
   getDefaultLanguage,
@@ -85,6 +84,9 @@ export const Surveys = {
   getTaxonomyByName,
   getTaxonomyByUuid,
 
+  findNodeDefByName,
+  findNodeDefByUuid,
+  findNodeDefsByUuids,
   getNodeDefByName,
   getNodeDefsByUuids,
   getNodeDefByUuid,

diff --git a/src/survey/surveys/nodeDefs.ts b/src/survey/surveys/nodeDefs.ts
@@ -18,10 +18,40 @@ import { TraverseMethod } from '../../common'
 
 export const getNodeDefsArray = NodeDefsReader.getNodeDefsArray
 
+export const findNodeDefByUuid = (params: {
+  survey: Survey
+  uuid: string
+}): NodeDef<NodeDefType, NodeDefProps> | undefined => {
+  const { survey, uuid } = params
+  return survey.nodeDefs?.[uuid]
+}
+
+export const getNodeDefByUuid = (params: { survey: Survey; uuid: string }): NodeDef<NodeDefType, NodeDefProps> => {
+  const { survey, uuid } = params
+  const nodeDef = findNodeDefByUuid({ survey, uuid })
+  if (!nodeDef) throw new SystemError('survey.nodeDefUuidNotFound', { uuid })
+  return nodeDef
+}
+
+export const findNodeDefByName = (params: {
+  survey: Survey
+  name: string
+}): NodeDef<NodeDefType, NodeDefProps> | undefined => {
+  const { survey, name } = params
+  const nodeDefUuidByNameIndex = survey.nodeDefsIndex?.nodeDefUuidByName
+  if (nodeDefUuidByNameIndex) {
+    const uuid = nodeDefUuidByNameIndex[name]
+    return findNodeDefByUuid({ survey, uuid })
+  }
+  return getNodeDefsArray(survey).find((nodeDef) => nodeDef.props.name === name)
+}
+
 export const getNodeDefByName = (params: { survey: Survey; name: string }): NodeDef<NodeDefType, NodeDefProps> => {
   const { survey, name } = params
-  const nodeDef = getNodeDefsArray(survey).find((nodeDef) => nodeDef.props.name === name)
-  if (!nodeDef) throw new SystemError('survey.nodeDefNameNotFound', { name })
+  const nodeDef = findNodeDefByName({ survey, name })
+  if (!nodeDef) {
+    throw new SystemError('survey.nodeDefNameNotFound', { name })
+  }
   return nodeDef
 }
 
@@ -44,24 +74,6 @@ export const findNodeDefsByUuids = (params: {
   }, [])
 }
 
-export const getNodeDefByUuid = (params: { survey: Survey; uuid: string }): NodeDef<NodeDefType, NodeDefProps> => {
-  const { survey, uuid } = params
-  const nodeDef = survey.nodeDefs?.[uuid]
-  if (!nodeDef) throw new SystemError('survey.nodeDefUuidNotFound', { uuid })
-  return nodeDef
-}
-
-export const findNodeDefByUuid = (params: {
-  survey: Survey
-  uuid: string
-}): NodeDef<NodeDefType, NodeDefProps> | undefined => {
-  try {
-    return getNodeDefByUuid(params)
-  } catch (error) {
-    return undefined
-  }
-}
-
 export const getNodeDefParent = (params: {
   survey: Survey
   nodeDef: NodeDef<NodeDefType, NodeDefProps>

diff --git a/src/survey/surveys/nodeDefsIndex.test.ts b/src/survey/surveys/nodeDefsIndex.test.ts
@@ -0,0 +1,48 @@
+import { NodeDefs } from '../../nodeDef'
+import { Survey } from '../../survey'
+import { SurveyBuilder, SurveyObjectBuilders } from '../../tests/builder/surveyBuilder'
+import { createTestAdminUser } from '../../tests/data'
+
+const { entityDef, integerDef } = SurveyObjectBuilders
+
+let survey: Survey
+
+describe('Survey Node Definitionss index', () => {
+  beforeAll(async () => {
+    const user = createTestAdminUser()
+
+    survey = new SurveyBuilder(
+      user,
+      entityDef('cluster', integerDef('cluster_id').key(), entityDef('plot', integerDef('plot_id').key()).multiple())
+    ).build()
+  }, 10000)
+
+  test('root def UUID', () => {
+    const clusterUuid = survey.nodeDefsIndex?.rootDefUuid
+    expect(clusterUuid).toBeDefined()
+  })
+
+  test('node defs by name', () => {
+    const { nodeDefUuidByName, rootDefUuid } = survey.nodeDefsIndex ?? {}
+    const clusterUuid2 = nodeDefUuidByName?.['cluster']
+    expect(rootDefUuid).toBe(clusterUuid2)
+
+    // cluster_id
+    const clusterIdUuid = nodeDefUuidByName?.['cluster_id']
+    expect(clusterIdUuid).toBeDefined()
+    expect(clusterIdUuid).not.toBe(rootDefUuid)
+
+    const clusterIdDef = survey.nodeDefs?.[clusterIdUuid!]
+    expect(clusterIdDef).toBeDefined()
+    expect(NodeDefs.getName(clusterIdDef!)).toBe('cluster_id')
+
+    // plot_id
+    const plotIdUuid = nodeDefUuidByName?.['plot_id']
+    expect(plotIdUuid).toBeDefined()
+    expect(plotIdUuid).not.toBe(rootDefUuid)
+
+    const plotIdDef = survey.nodeDefs?.[plotIdUuid!]
+    expect(plotIdDef).toBeDefined()
+    expect(NodeDefs.getName(plotIdDef!)).toBe('plot_id')
+  })
+})
diff --git a/src/tests/builder/surveyBuilder/nodeDefBuilder.ts b/src/tests/builder/surveyBuilder/nodeDefBuilder.ts
@@ -24,12 +24,14 @@ export abstract class NodeDefBuilder {
   }
 
   protected createNodeDef(params: { nodeDefParent?: NodeDefEntity } = {}): NodeDef<NodeDefType, NodeDefProps> {
-    return NodeDefFactory.createInstance({
+    const nodeDef = NodeDefFactory.createInstance({
       nodeDefParent: params.nodeDefParent,
       type: this.type,
       props: this.props,
       propsAdvanced: this.propsAdvanced,
     })
+    nodeDef.temporary = false
+    return nodeDef
   }
 
   abstract build(params: { survey: Survey; nodeDefParent?: NodeDefEntity }): { [uuid: string]: NodeDef<NodeDefType> }