From 499bbb34335956fdeedd232a499a34e126f218bf Mon Sep 17 00:00:00 2001
From: Paul Harrison <paul.harrison@manchester.ac.uk>
Date: Wed, 7 Feb 2024 14:18:04 +0000
Subject: [PATCH] beginning of contained reference validation

---
 doc/VODML-stdrec.xml                          |  2 +-
 models/sample/test/jpatest.vo-dml.xml         |  2 +-
 models/sample/test/lifecycleTest.vo-dml.xml   | 28 ++++++++++++-
 models/sample/test/lifecycleTest.vodsl        | 11 ++++--
 .../dm/lifecycle/LifeCycleDetailedTest.java   | 18 +++++++--
 .../dm/lifecycle/LifecycleTestModelTest.java  |  2 +-
 tools/xslt/common-structure-functions.xsl     | 39 ++++++++++++++++++-
 tools/xslt/vo-dml-v1.0.sch.xml                |  7 ++--
 8 files changed, 94 insertions(+), 15 deletions(-)

diff --git a/doc/VODML-stdrec.xml b/doc/VODML-stdrec.xml
index bf8f396..906f32b 100644
--- a/doc/VODML-stdrec.xml
+++ b/doc/VODML-stdrec.xml
@@ -71,7 +71,7 @@ models allows these to be used in a homogeneous manner and allows a
 consistent definition of reuse of one model by another. The particular language
 defined here includes a consistent identification mechanism for model which
 allows these to be referenced in an explicit and uniform manner also from other
-contexts, in particular from othe IVOA standard formats such as VOTable.
+contexts, in particular from other IVOA standard formats such as VOTable.
 
 The language defined in this specification is named VO-DML (VO Data Modeling
 Language). VO-DML is a conceptual modeling language that is agnostic of
diff --git a/models/sample/test/jpatest.vo-dml.xml b/models/sample/test/jpatest.vo-dml.xml
index df818be..2098259 100644
--- a/models/sample/test/jpatest.vo-dml.xml
+++ b/models/sample/test/jpatest.vo-dml.xml
@@ -8,7 +8,7 @@
       <uri/>
       <title></title>
       <version>1.0</version>
-      <lastModified>2023-12-19T14:14:15Z</lastModified>
+      <lastModified>2023-12-19T15:16:26Z</lastModified>
       <import>
         <name>null</name><!--should not be needed in modern vo-dml -->
         <url>IVOA-v1.0.vo-dml.xml</url>
diff --git a/models/sample/test/lifecycleTest.vo-dml.xml b/models/sample/test/lifecycleTest.vo-dml.xml
index d58f72f..f964d81 100644
--- a/models/sample/test/lifecycleTest.vo-dml.xml
+++ b/models/sample/test/lifecycleTest.vo-dml.xml
@@ -9,7 +9,7 @@
       <title></title>
       <author>pharriso</author>
       <version>0.1</version>
-      <lastModified>2023-06-08T07:49:03Z</lastModified>
+      <lastModified>2024-01-23T08:37:51Z</lastModified>
       <import>
         <name>null</name><!--should not be needed in modern vo-dml -->
         <url>IVOA-v1.0.vo-dml.xml</url>
@@ -111,6 +111,18 @@
                <vodml-id>ATest2</vodml-id>
                <name>ATest2</name>
                <description></description>	    
+               <composition>
+                 <vodml-id>ATest2.atest</vodml-id>
+                 <name>atest</name>
+                 <description></description>	    
+                 <datatype>
+                    <vodml-ref>lifecycleTest:ATest</vodml-ref>
+                 </datatype>
+                 <multiplicity>
+                   <minOccurs>1</minOccurs>
+                   <maxOccurs>1</maxOccurs>
+                 </multiplicity>
+               </composition>
                <reference>
                  <vodml-id>ATest2.refagg</vodml-id>
                  <name>refagg</name>
@@ -120,7 +132,7 @@
                  </datatype>
                  <multiplicity>
                    <minOccurs>1</minOccurs>
-                   <maxOccurs>1</maxOccurs><!-- FIXME would like to support unbounded here -->
+                   <maxOccurs>1</maxOccurs>
                  </multiplicity>
                </reference>
                <reference>
@@ -152,5 +164,17 @@
                    <maxOccurs>-1</maxOccurs>
                  </multiplicity>
                </composition>
+               <reference>
+                 <vodml-id>ATest3.refBad</vodml-id>
+                 <name>refBad</name>
+                 <description></description>	    
+                 <datatype>
+                   <vodml-ref>lifecycleTest:ReferredLifeCycle</vodml-ref>
+                 </datatype>
+                 <multiplicity>
+                   <minOccurs>1</minOccurs>
+                   <maxOccurs>1</maxOccurs>
+                 </multiplicity>
+               </reference>
             </objectType>
 </vo-dml:model>
diff --git a/models/sample/test/lifecycleTest.vodsl b/models/sample/test/lifecycleTest.vodsl
index caefb86..bcbb995 100644
--- a/models/sample/test/lifecycleTest.vodsl
+++ b/models/sample/test/lifecycleTest.vodsl
@@ -26,15 +26,20 @@ otype ATest { //this does things as per current rules
 
 otype ATest2 {
 	
-	refagg @+ references ReferredTo  ""; /*  not liked because of "aggregation pattern", but really ok - 
-	                                     it is more a lifecycle matter really - as long as there is nothing that can 
+	/*  would like to make the multiplicity of below one-or-more - however this is not liked because 
+	 *  of "aggregation pattern", but really ok for references - 
+     it is more a lifecycle matter really - as long as there is nothing that can 
 	                                     * delete the referredTo otype */
 	                                     
-	refcont references ReferredLifeCycle ""; // potentially not OK (perhaps should be an error) - here the referred otype is also contained, so the 
+	refagg references ReferredTo  "";                                      
+	atest : ATest as composition "";
+	                                     
+	refcont references ReferredLifeCycle ""; // potentially not OK (should be an error if referenced by something outside the object tree) - here the referred otype is also contained, so the 
 	}
 
 otype ATest3 { /*  fails the "unique composition rule" in schematron if the one of the Contained instances is also contained in ATest,
  *  but this should be unlikely if a contained type instance is created at the same time as its container instance.
 	*/
 	contained : Contained @+ as composition ""; //vodsl tooling does not flag this- but the vodml schematron does.
+	refBad references ReferredLifeCycle ""; //this should be an error
 }
\ No newline at end of file
diff --git a/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifeCycleDetailedTest.java b/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifeCycleDetailedTest.java
index 83c92d6..1e56fa5 100644
--- a/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifeCycleDetailedTest.java
+++ b/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifeCycleDetailedTest.java
@@ -9,7 +9,7 @@
 
 package org.ivoa.dm.lifecycle;
 
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 import java.util.Arrays;
 import java.util.List;
@@ -54,8 +54,8 @@ void setUp() throws Exception {
             a.refandcontained = refcont;
             
         });
-        atest2 = new ATest2(referredTo, refcont.get(0));
-        atest3 = new ATest3(contained);
+        atest2 = new ATest2(atest, referredTo, refcont.get(0));
+        atest3 = new ATest3(contained, refcont.get(0));//TODO this will create contradictions.... how best to test
         
         model = new LifecycleTestModel();
         model.addContent(atest);
@@ -94,6 +94,18 @@ void MultiContainedJPATest() {
          em.getTransaction().commit();
          dumpDbData(em, "lifecycle_dump.sql");
     }
+    
+    @Test
+    void copyTest() {
+        ATest atestprime = new ATest(atest);
+        atest.ref1.test1 = 4;
+        assertEquals(4, atestprime.ref1.test1); //the reference should have changed
+        atest.contained.get(0).test2 = "changed";
+        assertEquals("firstcontained",atestprime.contained.get(0).test2); // new objects created for the contained so changing original should not affect the prime
+        
+        ATest2 atest2prime = new ATest2(atest2);
+         
+    }
    
 
 }
diff --git a/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifecycleTestModelTest.java b/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifecycleTestModelTest.java
index 165e69b..bcd2105 100644
--- a/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifecycleTestModelTest.java
+++ b/tools/gradletooling/sample/src/test/java/org/ivoa/dm/lifecycle/LifecycleTestModelTest.java
@@ -71,7 +71,7 @@ public LifecycleTestModel createModel() {
             a.refandcontained = refcont;
             
         });
-        atest2 = new ATest2(referredTo, refcont.get(0));
+        atest2 = new ATest2(atest,referredTo, refcont.get(0));
         
         LifecycleTestModel model = new LifecycleTestModel();
         model.addContent(atest);
diff --git a/tools/xslt/common-structure-functions.xsl b/tools/xslt/common-structure-functions.xsl
index 15a9338..942e979 100644
--- a/tools/xslt/common-structure-functions.xsl
+++ b/tools/xslt/common-structure-functions.xsl
@@ -24,7 +24,7 @@ note - only define functions in here as it is included in the schematron rules
     <!-- does the vodml-ref exist? -->
     <xsl:function name="vf:vo-dml-ref-exists" as="xsd:boolean">
         <xsl:param name="vodml-ref" />
-        <xsl:value-of select="count($models/key('ellookup',$vodml-ref)) =1 " />
+        <xsl:sequence select="count($models/key('ellookup',$vodml-ref)) =1 " />
     </xsl:function>
 
     <!-- return the base types for current type - note that this does not return the types in strict hierarchy order (not sure why!) -->
@@ -328,6 +328,43 @@ note - only define functions in here as it is included in the schematron rules
         </xsl:choose>
 
     </xsl:function>
+
+    <!-- returns all the contained type ids -->
+    <xsl:function name="vf:containedTypeIds" as="xsd:string*">
+        <xsl:param name="vodml-ref"/>
+        <xsl:choose>
+            <xsl:when test="$models/key('ellookup',$vodml-ref)">
+                <xsl:variable name="el" as="element()">
+                    <xsl:copy-of select="$models/key('ellookup',$vodml-ref)" />
+                </xsl:variable>
+                <xsl:choose>
+                    <xsl:when test="$el/composition">
+                        <xsl:sequence select="($el/composition/datatype/vodml-ref, for $v in $el/composition/datatype/vodml-ref return  vf:containedTypeIds($v))"/>
+                    </xsl:when>
+                </xsl:choose>
+            </xsl:when>
+            <xsl:otherwise>
+                <xsl:message terminate="yes">type <xsl:value-of select="$vodml-ref"/> not in considered models for base types</xsl:message>
+            </xsl:otherwise>
+        </xsl:choose>
+    </xsl:function>
+
+    <xsl:function name="vf:isTypeContained" as="xsd:boolean">
+        <xsl:param name="type-vodml-ref"  as="xsd:string" />
+        <xsl:param name="root-vodml-ref" as="xsd:string"/>
+        <xsl:variable name="root" select="$models/key('ellookup',$root-vodml-ref)"/>
+        <xsl:choose>
+            <xsl:when test="$root/composition">
+                <xsl:sequence  select="$type-vodml-ref = vf:containedTypeIds($root-vodml-ref)"/>
+            </xsl:when>
+            <xsl:otherwise>
+                <xsl:sequence select="false()"/>
+            </xsl:otherwise>
+        </xsl:choose>
+    </xsl:function>
+
+
+
     <xsl:function name="vf:attributeIsDtype" as="xsd:boolean">
         <xsl:param name="attr" as="element()"/>
         <xsl:sequence select="$models/key('ellookup',$attr/datatype/vodml-ref)/name() = 'dataType'"/>
diff --git a/tools/xslt/vo-dml-v1.0.sch.xml b/tools/xslt/vo-dml-v1.0.sch.xml
index 526a6c5..bcb3604 100644
--- a/tools/xslt/vo-dml-v1.0.sch.xml
+++ b/tools/xslt/vo-dml-v1.0.sch.xml
@@ -141,6 +141,7 @@ type <sch:value-of select="datatype/vodml-ref"/> of <sch:value-of select="$owner
 
   <sch:rule context="reference">
     <sch:let name="owner" value="vf:asvodmlref(current())"/>
+      <sch:let name="parent" value="vf:asvodmlref(current()/ancestor-or-self::objectType)"/>
       <sch:assert test="vf:vo-dml-ref-exists(datatype/vodml-ref)" flag="error">
       vodml-ref  <sch:value-of select="datatype/vodml-ref"/> for type of  <sch:value-of select="$owner"/> does not exist
   </sch:assert>
@@ -150,9 +151,9 @@ type <sch:value-of select="datatype/vodml-ref"/> of <sch:value-of select="$owner
 type <sch:value-of select="datatype/vodml-ref"/> of reference <sch:value-of select="$owner"/> is not an object type but a '<sch:value-of select="$target"/>'
     </sch:assert>
       <!--rules implementing a reference should not be used in composition - this does not work well with the current tooling, however, if references allowed unlimited multiplicities then...     -->
-      <sch:report test="$composition" flag="warning">
-Reference <sch:value-of select="./datatype/vodml-ref"/> used in <sch:value-of select="./vodml-id"/> is already in composition <sch:value-of select="$composition/vodml-id"/> which has lifecycle implications.
-      </sch:report>
+      <sch:assert test="$composition and not(vf:isTypeContained($target,$parent)) " flag="error">
+Reference <sch:value-of select="./datatype/vodml-ref"/> used in <sch:value-of select="./vodml-id"/> is already in composition <sch:value-of select="$composition/vodml-id"/> which has lifecycle implications
+      </sch:assert>
    </sch:rule>
 
   <sch:rule context="objectType/composition">