Merge pull request intermine#51 from uosl/feature/subclasses

Support subclasses by passing type-constraints with model

.travis.yml

@@ -10,7 +10,7 @@ addons:
 before_script:
   # Use a newer version of Python
   - pyenv versions
-  - pyenv global 3.6
+  - pyenv global 3.6.7
   # Setup biotestmine to test against
   - pip3 install intermine-boot
   - intermine_boot start local --build-im --im-branch bluegenes

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 1.2.0 (2020-10-06)
+
+- Support subclasses by passing type-constraints with model [#51](https://github.com/intermine/imcljs/pull/51)
+    - Enables `imcljs.path/walk` to traverse subclasses specified via type constraints, and enables other functions dependent on it to handle subclasses correctly
+- Do not add constraint code to type constraints when sterilizing query [#51](https://github.com/intermine/imcljs/pull/51)
+
 ## 1.1.0 (2020-02-20)
 
 - Support new web services [#42](https://github.com/intermine/imcljs/pull/42)

project.clj

@@ -1,4 +1,4 @@
-(defproject org.intermine/imcljs "1.1.0"
+(defproject org.intermine/imcljs "1.2.0"
   :description "imcljs"
   :license {:name "Eclipse Public License"
             :url "http://www.eclipse.org/legal/epl-v10.html"}

src/cljc/imcljs/path.cljc

@@ -66,47 +66,94 @@
        (filter identity)
        first))
 
+(defn- walk-rec
+  [model [class-kw & [path & remaining]] trail curr-path path->subclass]
+  ;; Notice that this recursive function consumes two elements of the path at a
+  ;; time. The reason for this is that if `path` happens to be an attribute, we
+  ;; need to know its class `class-kw` to be able to find it.
+  (let [;; At any point, a subclass constraint can override the default class.
+        class-kw (get path->subclass curr-path class-kw)
+        class (get-in model [:classes class-kw])
+        _ (assert (map? class) "Path traverses nonexistent class")
+        ;; Search the class for the property `path` to find the next referenced class.
+        {reference :referencedType :as class-value}
+        (get (apply merge (map class [:attributes :references :collections])) path)
+        ;; This is `curr-path` for the next recursion.
+        next-path (conj curr-path path)]
+    (if remaining
+      ;; If we don't have a reference, we can't go on and so return nil.
+      (when reference
+        (recur model
+               ;; We cons the reference so we know the parent class in case the
+               ;; next recursion's `path` happens to be an attribute. In effect
+               ;; we only consume one element of the path at a time.
+               (cons (keyword reference) remaining)
+               (conj trail class)
+               next-path
+               path->subclass))
+      ;; Because we consume two elements of the path at a time, we have to
+      ;; repeat some logic in the termination case (hence we add two elements).
+      (conj trail
+            class
+            ;; The path can end with a subclass, so we check with `next-path`.
+            (if-let [subclass (get path->subclass next-path)]
+              ;; All the extra stuff done above to `class-kw` need not be
+              ;; repeated, as we've now consumed the entire path.
+              (get-in model [:classes subclass])
+              (if reference
+                ;; Usually the next recursion would get the class from reference.
+                (get-in model [:classes (keyword reference)])
+                ;; If there's no reference, this means the last element of the
+                ;; path is an attribute.
+                class-value))))))
+
 (defn walk
   "Return a vector representing each part of path.
   If any part of the path is unresolvable then a nil is returned.
   (walk im-model `Gene.organism.shortName`)
   => [{:name `Gene`, :collections {...}, :attributes {...}}
       {:name `Organism`, :collections {...} :attributes {...}
-      {:name `shortName`, :type `java.lang.String`}]"
-  ([model path]
-   (let [p (if (string? path) (split-path path) (map keyword path))]
-     (if (= 1 (count p))
-       [(get-in model [:classes (first p)])]
-       (walk model p []))))
-  ([model [class-kw & [path & remaining]] trail]
-   (let [cv (class-value model class-kw path)]
-     (if remaining
-       (cond
-         (contains? cv :referencedType)
-         (recur model
-                (cons (keyword (:referencedType cv)) remaining)
-                (conj trail (get-in model [:classes class-kw]))))
-       (conj trail (get-in model [:classes class-kw])
-             (if (contains? cv :referencedType)
-               (get-in model [:classes (keyword (:referencedType cv))])
-               cv))))))
+      {:name `shortName`, :type `java.lang.String`}]
+  If the path traverses a subclass, you'll need to add a `:type-constraints`
+  key to `model` with a value like
+      [{:path `Gene.interactions.participant2`, :type `Gene`}]
+  for the path to be resolvable.
+      (walk im-model-with-type-constraints
+       `Gene.interactions.participant2.proteinAtlasExpression.tissue.name`)"
+  [model path]
+  (let [p (if (string? path) (split-path path) (map keyword path))]
+    (if (= 1 (count p))
+      [(get-in model [:classes (first p)])]
+      (walk-rec model p [] [(first p)]
+                (->> (:type-constraints model)
+                     (filter #(contains? % :type)) ; In case there are other constraints there.
+                     (reduce (fn [m {:keys [path type]}]
+                               (assoc m (split-path path) (keyword type)))
+                             {}))))))
+
 (defn data-type
   "Return the java type of a path representing an attribute.
   (attribute-type im-model `Gene.organism.shortName`)
-  => java.lang.String"
+  => java.lang.String
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (:type (last (walk model path))))
 
 (defn class
   "Returns the class represented by the path.
   (class im-model `Gene.homologues.homologue.symbol`)
-  => :Gene"
+  => :Gene
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (let [l (last (take-while #(does-not-contain? % :type) (walk model path)))]
     (keyword (or (:referencedType l) (keyword (:name l))))))
 
 (defn relationships
-  "Returns all relationships (references and collections) for a given string path."
+  "Returns all relationships (references and collections) for a given string path.
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (apply merge (map (get-in model [:classes (class model path)]) [:references :collections])))
 
@@ -121,7 +168,7 @@
           (clojure.string/replace #"^." #(clojure.string/upper-case %))))
 
 (defn display-name
-  "Returns a vector of friendly names representing the path
+  "Returns a vector of friendly names representing the path.
   ; TODO make this work with subclasses"
   ([model path]
    (let [p (if (string? path) (split-path path) path)]
@@ -137,7 +184,9 @@
          collected+)))))
 
 (defn attributes
-  "Returns all attributes for a given string path."
+  "Returns all attributes for a given string path.
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (apply merge (map (get-in model [:classes (class model path)]) [:attributes])))
 
@@ -146,23 +195,29 @@
   (class im-model `Gene.diseases`)
   => true
   (class im-model `Gene.diseases.name`)
-  => false"
+  => false
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (let [walked (walk model path)]
     (not (contains? (last walked) :type))))
 
 (defn trim-to-last-class
   "Returns a path string trimmed to the last class
   (trim-to-last-class im-model `Gene.homologues.homologue.symbol`)
-  => Gene.homologues.homologue"
+  => Gene.homologues.homologue
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (let [done (take-while #(does-not-contain? % :type) (walk model path))]
     (join-path (take (count done) (split-path path)))))
 
 (defn adjust-path-to-last-class
   "Returns a path adjusted to its last class
   (adjust-path-to-last-class im-model `Gene.organism.name`)
-  => Organism.name"
+  => Organism.name
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (let [attribute? (not (class? model path))
         walked     (reverse (walk model path))]
@@ -171,19 +226,27 @@
       (str (:name (nth walked 0))))))
 
 (defn friendly
-  "Returns a path as a strong"
+  "Returns a path as a strong
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   ([model path & [exclude-root?]]
    (reduce
     (fn [total next]
       (str total (if total " > ") (or (:displayName next) (:name next))))
     nil
-    (if exclude-root? (rest (walk model path)) (walk model path)))))
+    (if exclude-root?
+      (rest (walk model path))
+      (walk model path)))))
 
 (defn one-of? [col value]
   (some? (some #{value} col)))
 
 (defn subclasses
-  "Returns subclasses of the class"
+  "Returns direct subclasses of the class.
+  Tip: To get descendant subclasses, you will need to create a graph out of all
+  the classes' extends key, which is costly and outside the scope of imcljs.
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `walk` for more information)."
   [model path]
   (let [path-class (class model path)]
     (->> model

src/cljc/imcljs/query.cljc

@@ -89,8 +89,11 @@
   (if (contains? query :where)
     (update query :where
             (fn [constraints]
-              (reduce (fn [total {:keys [code] :as constraint}]
-                        (if (some? code)
+              (reduce (fn [total {:keys [code type] :as constraint}]
+                        (if (or (some? code)
+                                ;; Type (aka subclass) constraints may not
+                                ;; participate in the constraint logic.
+                                (some? type))
                           (conj total constraint)
                           (let [existing-codes      (set (remove nil? (concat (map :code constraints) (map :code total))))
                                 next-available-code (first (filter (complement blank?) (difference alphabet existing-codes)))]
@@ -153,7 +156,9 @@
   "Deconstructs a query by its views and groups them by class.
   (deconstruct-by-class model query)
   {:Gene {Gene.homologues.homologue {:from Gene :select [Gene.homologues.homologue.id] :where [...]}
-         {Gene {:from Gene :select [Gene.id] :where [...]}}}"
+         {Gene {:from Gene :select [Gene.id] :where [...]}}}
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `imcljs.path/walk` for more information)."
   [model query]
   (let [query (sterilize-query query)]
     (reduce (fn [path-map next-path]
@@ -164,7 +169,9 @@
 
 (defn group-views-by-class
   "Group the views of a query by their Class and provide a query
-  to retrieve just that column of data"
+  to retrieve just that column of data.
+  Make sure to add :type-constraints to the model if the path traverses a subclass
+  (see docstring of `imcljs.path/walk` for more information)."
   [model query]
   (let [query (sterilize-query query)]
     (reduce (fn [path-map next-path]

test/cljs/imcljs/path_test.cljs

@@ -24,6 +24,25 @@
             (is (= (map :name walked) '("Gene" "Protein" "Gene" "OntologyAnnotation" "OntologyTerm" "name")))
             (done)))))))
 
+(deftest walk-subclasses-with-type-constraints
+  (testing "Should be able to walk a path with multiple subclasses requiring type constraints and return parts of the model"
+    (async done
+      (go
+        (let [model (assoc (<! (fetch/model service))
+                           :type-constraints [{:path "Gene.childFeatures" :type "MRNA"}
+                                              {:path "Gene.childFeatures.CDSs.transcript" :type "TRNA"}])]
+          (let [walked (path/walk model "Gene.childFeatures.CDSs.transcript.name")]
+            (is (= (map :name walked) '("Gene" "MRNA" "CDS" "TRNA" "name")))
+            (done))))))
+  (testing "Should walk subclass even if it's the last part of the path"
+    (async done
+      (go
+        (let [model (assoc (<! (fetch/model service))
+                           :type-constraints [{:path "Gene.childFeatures" :type "MRNA"}])]
+          (let [walked (path/walk model "Gene.childFeatures")]
+            (is (= (map :name walked) '("Gene" "MRNA")))
+            (done)))))))
+
 (deftest walk-root
   (testing "Should be able to walk a path that is a single root and return parts of the model"
     (async done