fix: resolve issue with improperly detecting a valid model as invalid (…

…#121)
openfga · Dec 14, 2023 · cca4c43 · cca4c43
2 parents 8dfc3b8 + 0a764e6
commit cca4c43
Show file tree

Hide file tree

Showing 8 changed files with 114 additions and 66 deletions.
diff --git a/.github/workflows/main-go.yaml b/.github/workflows/main-go.yaml
@@ -12,11 +12,13 @@ on:
       - 'pkg/go/**'
       - 'OpenFGAParser.g4'
       - 'OpenFGALexer.g4'
+      - 'tests'
   pull_request:
     paths:
       - 'pkg/go/**'
       - 'OpenFGAParser.g4'
       - 'OpenFGALexer.g4'
+      - 'tests'
 
 permissions:
   contents: read

diff --git a/.github/workflows/main-js.yaml b/.github/workflows/main-js.yaml
@@ -12,11 +12,13 @@ on:
       - 'pkg/js/**'
       - 'OpenFGAParser.g4'
       - 'OpenFGALexer.g4'
+      - 'tests'
   pull_request:
     paths:
       - 'pkg/js/**'
       - 'OpenFGAParser.g4'
       - 'OpenFGALexer.g4'
+      - 'tests'
 
 permissions:
   contents: read

diff --git a/README.md b/README.md
@@ -37,11 +37,11 @@ OpenFGA is designed to make it easy for application builders to model their perm
 ## About This Repo
 This repo contains everything needed to interact with the OpenFGA Authorization Models schema versions 1.1+, in multiple languages (currently, Go and JS are supported).
 
-| Feature    | Implemented in ANTLR        |
-|------------|-----------------------------|
-| Basic DSL  | ✅  |
-| Nesting    | ❌ (planned)                 |
-| Conditions | ❌ (planned)                 |
+| Feature    | Implemented in ANTLR |
+|------------|----------------------|
+| Basic DSL  | ✅                    |
+| Nesting    | ✅ (partial, see #113 |
+| Conditions | ✅                    |
 
 | Feature                                               | Go          | JS          |
 |-------------------------------------------------------|-------------|-------------|

diff --git a/pkg/js/CHANGELOG.md b/pkg/js/CHANGELOG.md
@@ -1,14 +1,28 @@
 # Changelog
 
+## v0.2.0-beta.7
+
+### [v0.2.0-beta.7](https://github.com/openfga/language/releases/tag/vv0.2.0-beta.6...v0.2.0-beta.7) (2023-12-13)
+
+Fixed:
+- Issue in validating entrypoint or loop with some models (#120)
+
 ## v0.2.0-beta.6
 
-### [v0.2.0-beta.6](https://github.com/openfga/language/releases/tag/vv0.2.0-beta.3...v0.2.0-beta.4) (2023-12-11)
+### [v0.2.0-beta.6](https://github.com/openfga/language/releases/tag/vv0.2.0-beta.5...v0.2.0-beta.6) (2023-12-11)
 
 last commit: 8b692a44e937beae8693cc155b205ffc5b732fbe
 
 Added:
 - Initial limited support for mixing operators [#107](https://github.com/openfga/language/pull/107)
 
+## v0.2.0-beta.5
+
+### [v0.2.0-beta.5](https://github.com/openfga/language/releases/tag/vv0.2.0-beta.4...v0.2.0-beta.5) (2023-10-04)
+
+Added:
+- Initial support for Conditions when transforming from DSL to JSON (https://github.com/openfga/language/pull/75)
+
 ## v0.2.0-beta.4
 
 ### [v0.2.0-beta.4](https://github.com/openfga/language/releases/tag/vv0.2.0-beta.3...v0.2.0-beta.4) (2023-10-04)

diff --git a/pkg/js/package-lock.json b/pkg/js/package-lock.json
diff --git a/pkg/js/package.json b/pkg/js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@openfga/syntax-transformer",
-  "version": "0.2.0-beta.6",
+  "version": "0.2.0-beta.7",
   "description": "",
   "license": "Apache-2.0",
   "main": "./dist/index.js",
@@ -33,12 +33,12 @@
     "antlr4": "^4.13.1"
   },
   "devDependencies": {
-    "@openfga/sdk": "^0.3.0-beta.1",
+    "@openfga/sdk": "^0.3.0",
     "@types/jest": "^29.5.11",
     "@types/js-yaml": "^4.0.9",
     "@types/node": "^20.10.4",
-    "@typescript-eslint/eslint-plugin": "^6.13.2",
-    "@typescript-eslint/parser": "^6.13.2",
+    "@typescript-eslint/eslint-plugin": "^6.14.0",
+    "@typescript-eslint/parser": "^6.14.0",
     "eslint": "^8.55.0",
     "eslint-config-prettier": "^9.1.0",
     "eslint-plugin-import": "^2.29.0",

diff --git a/pkg/js/validator/validate-dsl.ts b/pkg/js/validator/validate-dsl.ts
@@ -175,7 +175,7 @@ function allowableTypes(typeName: Record<string, TypeDefinition>, type: string,
 // for the type/relation, whether there are any unique entry points, and if a loop is found
 // if there are unique entry points (i.e., direct relations) then it will return true
 // otherwise, it will follow its children to see if there are unique entry points
-// if there is a loop durig traversal, the function will return a boolean indicating so
+// if there is a loop during traversal, the function will return a boolean indicating so
 function hasEntryPointOrLoop(
   typeMap: Record<string, TypeDefinition>,
   typeName: string,
@@ -221,7 +221,7 @@ function hasEntryPointOrLoop(
         return [false, false];
       }
 
-      if (visited[decodedType][decodedRelation]) {
+      if (visited[decodedType]?.[decodedRelation]) {
         continue;
       }
 
@@ -276,7 +276,13 @@ function hasEntryPointOrLoop(
           continue;
         }
 
-        const [hasEntry, _] = hasEntryPointOrLoop(typeMap, assignableType, computedRelationName, assignableRelation, visited);
+        const [hasEntry, _] = hasEntryPointOrLoop(
+          typeMap,
+          assignableType,
+          computedRelationName,
+          assignableRelation,
+          visited,
+        );
         if (hasEntry) {
           return [true, false];
         }
@@ -306,7 +312,13 @@ function hasEntryPointOrLoop(
   } else if (rewrite.difference) {
     const visited = deepCopy(visitedRecords);
 
-    const [hasEntryBase, loopBase] = hasEntryPointOrLoop(typeMap, typeName, relationName, rewrite.difference.base, visited);
+    const [hasEntryBase, loopBase] = hasEntryPointOrLoop(
+      typeMap,
+      typeName,
+      relationName,
+      rewrite.difference.base,
+      visited,
+    );
     if (!hasEntryBase) {
       return [false, loopBase];
     }
@@ -640,7 +652,13 @@ function modelValidation(
       // parse through each of the relations to do validation
       for (const relationName in typeDef.relations) {
         const currentRelation = typeMap[typeName].relations;
-        const [hasEntry, loop] = hasEntryPointOrLoop(typeMap, typeName, relationName, currentRelation![relationName], {});
+        const [hasEntry, loop] = hasEntryPointOrLoop(
+          typeMap,
+          typeName,
+          relationName,
+          currentRelation![relationName],
+          {},
+        );
         if (!hasEntry) {
           const typeIndex = getTypeLineNumber(typeName, lines);
           const lineIndex = getRelationLineNumber(relationName, lines, typeIndex);

diff --git a/tests/data/dsl-syntax-validation-cases.yaml b/tests/data/dsl-syntax-validation-cases.yaml
@@ -1392,3 +1392,15 @@
         end: 60
       metadata:
         symbol: "]"
+
+- name: a valid model should not error
+  dsl: |
+    model
+      schema 1.1
+    type user
+    type document
+      relations
+        define editor: [team#member]
+    type team
+      relations
+        define member: [user]