fix parser issues related to multi-layered facets

issues:

- fix typos in ermrestCompactPath and _createParsedJoinFromStr
- make sure Location constructor uses the correct join. the function
  was assuming that the first portion of the url always has a facet,
  but that might not be true.

js/parser.js

@@ -268,14 +268,13 @@
         }
 
         // pathParts: <joins/facet/cfacet/filter/>
-        var aliasJoinRegExp = /\$.*/,
-            joinRegExp = /(?:left|right|full|^)\((.*)\)=\((.*:.*:.*)\)/,
+        var joinRegExp = /(?:left|right|full|^)\((.*)\)=\((.*:.*:.*)\)/,
             facetsRegExp = /\*::facets::(.+)/,
             customFacetsRegExp = /\*::cfacets::(.+)/;
 
-        var schemaTable = parts[0], table = this._rootTableName, schema = this._rootSchemaName;
-        var self = this, pathParts = [], alias, match, prevJoin = false;
-        var facets, cfacets, filter, filtersString, searchTerm, join, joins = [];
+        var table = this._rootTableName, schema = this._rootSchemaName;
+        var self = this, pathParts = [], alias, match, prevJoin = false, startWithT1 = false, aliasNumber;
+        var facets, cfacets, filter, filtersString, join, joins = [];
 
         // go through each of the parts
         parts.forEach(function (part, index) {
@@ -291,9 +290,23 @@
                 // there wasn't any join before, so this is the start of new path,
                 // so we should create an object for the previous one.
                 if (!prevJoin && index !== 1) {
-                    // we're creating this for the previous section, so we should use the previous index
-                    // Alias will be T, T1, T2, ... (notice we don't have T0)
-                    alias = module._parserAliases.JOIN_TABLE_PREFIX + (pathParts.length > 0 ? pathParts.length : "");
+                    /**
+                     * we're creating this alias for the previous section, so we should use the previous index
+                     * Alias will be T, T1, T2, ... (notice we don't have T0)
+                     */
+                    aliasNumber = pathParts.length;
+
+                    /**
+                     * T is always preserved for the initial schema:table. if the first pathPart doesn't have any joins
+                     * and has facet/filters, then it's for the schema:table and so we should start with T.
+                     * but if has join, then it means that the schema:table didn't have any filter/facets. so we have to start from T1,
+                     * and the rest of the parts should just add one more
+                     */
+                    if (pathParts.length === 0 && joins.length > 0) {
+                        startWithT1 = true;
+                    }
+                    if (startWithT1) aliasNumber++;
+                    alias = module._parserAliases.JOIN_TABLE_PREFIX + (aliasNumber > 0 ? aliasNumber : "");
                     pathParts.push(new PathPart(alias, joins, schema, table, facets, cfacets, filter, filtersString));
                     filter = undefined; filtersString = undefined; cfacets = undefined; facets = undefined; join = undefined; joins = [];
                 }
@@ -672,7 +685,7 @@
                 };
             });
 
-            // no rightJoin: s:t/<parths>
+            // no rightJoin: s:t/<parts>
             if (rightJoinIndex === -1) {
                 uri = self.rootTableAlias + ":=";
                 if (self.rootSchemaName) {
@@ -693,14 +706,14 @@
                     if (self.pathParts[i].joins.length > 0) {
                         // since we're reversing, we have to make sure we're using the
                         // alias of the previous pathpart
-                        alias = i > 0 ? self.pathPart[i-1].alias : self.rootTableAlias;
+                        alias = i > 0 ? self.pathParts[i-1].alias : self.rootTableAlias;
                         uri += "/" + joinsStr(self.pathParts[i], alias, i, true);
                     }
                 }
 
                 // if there was pathParts before facet with null, change back to the facet with null
                 if (self.pathParts[rightJoinIndex].joins.length > 0) {
-                    uri += "/$" + self.pathParts[i].alias;
+                    uri += "/$" + self.pathParts[rightJoinIndex].alias;
                 }
             }
 
@@ -1403,6 +1416,10 @@
              *
              * when there aren't any joins, this could be the schema:table, so we have to start with T.
              * but if there are joins, we can start with T1, as T alias is always the first schema:table.
+             *
+             * this is how we're using the alias property:
+             * - if it doesn't have any joins, it's the alias of the previous path or root (so facet/)
+             * - if it has join, it's the alias that we're using to name the projected table that the join represents.
              */
             if (lastPart) {
                 var aliasNumber = "";
@@ -1449,6 +1466,8 @@
      * Container for the path part, It will have the following attributes:
      * - joins: an array of ParsedJoin objects.
      * - alias: The alias that the facets should refer to
+     *   - if there aren't any joins, it's the alias of the previous path or root.
+     *   - otherwise, it's the alias that we're using to name the projected table that the join represents.
      * - schema: The schema that the join ends up with
      * - table: the table that the joins end up with
      * - facets: the facets object
@@ -1717,7 +1736,7 @@
      * @returns {ParsedJoin}
      */
     function _createParsedJoinFromStr (linking, table, schema) {
-        var fromSchemaTable = schema ? [table,schema].join(":") : table;
+        var fromSchemaTable = schema ? [schema,table].join(":") : table;
         var fromCols = linking[1].split(",");
         var toParts = linking[2].match(/([^:]*):([^:]*):([^\)]*)/);
         var toCols = toParts[3].split(",");

test/specs/parser/tests/01.parser.js

@@ -691,11 +691,24 @@ exports.execute = function(options) {
                         "ermrestCompactPath missmatch"
                     );
                 });
+
+                it ("parser should handle not having any filter on the first layer.", function () {
+                    uri = baseUri + `/(id)=(s:table2:id)/!(RID=%3A%3Anull%3A%3A&RCB=%3A%3Anull%3A%3A)/(id3)=(s:table3:id)/*::facets::${validBlob2}/(id4)=(s:table4:id)`;
+                    location = options.ermRest.parse(uri);
+                    expect(location).toBeDefined("location is not defined");
+                    expect(location.uri).toEqual(uri, "uri missmatch");
+                    expect(location.ermrestCompactPath).toEqual(
+                        "T:=parse_schema:parse_table/T1:=(id)=(s:table2:id)/!(RID=%3A%3Anull%3A%3A&RCB=%3A%3Anull%3A%3A)/T2:=(id3)=(s:table3:id)/accession=2/$T2/some-other-column::ciregexp::test2/$T2/M:=(id4)=(s:table4:id)",
+                        "ermrestCompactPath missmatch"
+                    );
+                });
             });
 
-            // relies on the previous test
             describe("for changing facets in location, ", function () {
                 it("Location.facets setter should be able to change the facet and update other APIs.", function() {
+                    uri = baseUri + "/*::facets::" + validBlob + "/some_col=v1/(id)=(s:otherTable:id)/(id2)=(s:otherTable2:id)" +
+                          "/*::facets::" + validBlob2 + "/some_col2=v2/(id)=(s:yetAnotherTable:id)" + "/*::facets::" + validBlob3;
+                    location = options.ermRest.parse(uri);
                     location.facets = facetObj;
 
                     uri = baseUri + "/*::facets::" + validBlob + "/some_col=v1/(id)=(s:otherTable:id)/(id2)=(s:otherTable2:id)" +
@@ -1536,6 +1549,7 @@ exports.execute = function(options) {
                     // {"and": [ {"source": "column"} ]}
                     expectError("N4IghgdgJiBcDaoDOB7ArgJwMYFM4ixQBs0BbCEAXwF1Kg");
                 });
+
             });
         });