diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDao.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDao.java index abbca9a3e..6e1b3b60d 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDao.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/JenaBaseDao.java @@ -187,11 +187,14 @@ public class JenaBaseDao extends JenaBaseDaoCon { existingValue = ((Literal)object).getString(); } } - if ( (existingValue!=null && value == null) || (existingValue!=null && value != null && !(existingValue.equals(value)) ) ) { - model.removeAll(res, dataprop, null); - } - if ( (existingValue==null && value != null && value.length()>0) || (existingValue!=null && value != null && value.length()>0 && !(existingValue.equals(value)) ) ) { - model.add(res, dataprop, value, XSDDatatype.XSDstring); + + if (value == null || value.length() == 0) { + model.removeAll(res, dataprop, null); + } else if (existingValue == null ) { + model.add(res, dataprop, value, XSDDatatype.XSDstring); + } else if (!existingValue.equals(value)) { + model.removeAll(res, dataprop, null); + model.add(res, dataprop, value, XSDDatatype.XSDstring); } } } @@ -258,9 +261,23 @@ public class JenaBaseDao extends JenaBaseDaoCon { * convenience method for use with functional datatype properties */ protected void updatePropertyIntValue(Resource res, Property dataprop, int value, Model model) { - if (dataprop != null) { - model.removeAll(res, dataprop, null); - model.add(res, dataprop, Integer.toString(value), XSDDatatype.XSDint); + + if (dataprop != null) { + Integer existingValue = null; + Statement stmt = res.getProperty(dataprop); + if (stmt != null) { + RDFNode object = stmt.getObject(); + if (object != null && object.isLiteral()){ + existingValue = ((Literal)object).getInt(); + } + } + + if (existingValue == null ) { + model.add(res, dataprop, Integer.toString(value), XSDDatatype.XSDint); + } else if (existingValue.intValue() != value) { + model.removeAll(res, dataprop, null); + model.add(res, dataprop, Integer.toString(value), XSDDatatype.XSDint); + } } } @@ -292,12 +309,12 @@ public class JenaBaseDao extends JenaBaseDaoCon { * convenience method for use with functional datatype properties */ protected void updatePropertyNonNegativeIntValue(Resource res, Property dataprop, int value, Model model) { - if (dataprop != null) { - model.removeAll(res, dataprop, null); - if (value>-1) { - model.add(res, dataprop, Integer.toString(value), XSDDatatype.XSDint); - } - } + + if (value < 0) + return; + + updatePropertyIntValue(res,dataprop,value,model); + } /** @@ -313,10 +330,25 @@ public class JenaBaseDao extends JenaBaseDaoCon { * convenience method for use with functional properties */ protected void updatePropertyFloatValue(Resource res, Property dataprop, Float value, Model model) { - if (dataprop != null) { - model.removeAll(res, dataprop, null); - if( value != null ) - model.add(res, dataprop, Float.toString(value), XSDDatatype.XSDfloat); + + if (dataprop != null) { + Float existingValue = null; + Statement stmt = res.getProperty(dataprop); + if (stmt != null) { + RDFNode object = stmt.getObject(); + if (object != null && object.isLiteral()){ + existingValue = ((Literal)object).getFloat(); + } + } + + if (value == null) { + model.removeAll(res, dataprop, null); + } else if (existingValue == null ) { + model.add(res, dataprop, Float.toString(value), XSDDatatype.XSDfloat); + } else if (existingValue != value) { + model.removeAll(res, dataprop, null); + model.add(res, dataprop, Float.toString(value), XSDDatatype.XSDfloat); + } } } @@ -364,16 +396,31 @@ public class JenaBaseDao extends JenaBaseDaoCon { protected synchronized void updatePropertyDateValue(Resource res, DatatypeProperty dataprop, Date value, Model model) { try { if (dataprop != null) { - model.removeAll(res, dataprop, null); - if (value != null) { - model.add(res, dataprop, xsdDateFormat.format(value), XSDDatatype.XSDdate); + if (value == null) { + model.removeAll(res, dataprop, null); + } else { + Date existingValue = null; + Statement stmt = res.getProperty(dataprop); + if (stmt != null) { + RDFNode object = stmt.getObject(); + if (object != null && object.isLiteral()){ + existingValue = (Date)((Literal)object).getValue(); + } + } + + if (existingValue == null ) { + model.add(res, dataprop, xsdDateFormat.format(value), XSDDatatype.XSDdate); + } else if (existingValue != value) { + model.removeAll(res, dataprop, null); + model.add(res, dataprop, xsdDateFormat.format(value), XSDDatatype.XSDdate); + } } } } catch (Exception e) { log.error("Error in updatePropertyDateValue"); } } - + /** * convenience method */ @@ -443,10 +490,45 @@ public class JenaBaseDao extends JenaBaseDaoCon { * convenience method for use with functional object properties */ protected void updatePropertyResourceURIValue(Resource res, ObjectProperty prop, String objectURI) { - Resource objectRes = getOntModel().getResource(objectURI); - if (prop != null && objectRes != null) { - res.removeAll(prop); - res.addProperty(prop, objectRes); + + Model model = res.getModel(); + + if (model != null) { + updatePropertyResourceURIValue(res, prop, objectURI, model); + } + } + + /** + * convenience method for use with functional properties + */ + protected void updatePropertyResourceURIValue(Resource res, Property prop, String uri, Model model) { + + if (prop != null) { + if (uri == null) { + model.removeAll(res, prop, null); + } else { + String badURIErrorStr = checkURI(uri); + if (badURIErrorStr != null) { + log.error(badURIErrorStr); + return; + } + + Resource existingValue = null; + Statement stmt = res.getProperty(prop); + if (stmt != null) { + RDFNode object = stmt.getObject(); + if (object != null && object.isResource()){ + existingValue = (Resource)object; + } + } + + if (existingValue == null ) { + model.add(res, prop, model.createResource(uri)); + } else if (!(existingValue.getURI()).equals(uri)) { + model.removeAll(res, prop, null); + model.add(res, prop, model.createResource(uri)); + } + } } } @@ -463,12 +545,59 @@ public class JenaBaseDao extends JenaBaseDaoCon { * convenience method for use with functional object properties */ protected void updatePropertyResourceValue(Resource res, Property prop, Resource objectRes) { - if (prop != null && objectRes != null) { - res.removeAll(prop); - res.addProperty(prop, objectRes); + + Model model = res.getModel(); + + if (model != null) { + updatePropertyResourceValue(res, prop, objectRes, model); } + } + /** + * convenience method for use with functional object properties + */ + protected void updatePropertyResourceValue(Resource res, Property prop, Resource objectRes, Model model) { + + if (prop != null) { + if (objectRes == null) { + model.removeAll(res, prop, null); + } else { + Resource existingValue = null; + Statement stmt = res.getProperty(prop); + if (stmt != null) { + RDFNode object = stmt.getObject(); + if (object != null && object.isResource()){ + existingValue = (Resource)object; + } + } + + if (existingValue == null ) { + model.add(res, prop, objectRes); + } else if (!existingValue.equals(objectRes)) { + model.removeAll(res, prop, null); + model.add(res, prop, objectRes); + } + } + } + } + + /** + * convenience method for updating the RDFS label + */ + protected void updateRDFSLabel(OntClass ontCls, String label) { + + if (label != null && label.length() > 0) { + + String existingValue = ontCls.getLabel((String) getDefaultLanguage()); + + if (existingValue == null || !existingValue.equals(label)) { + ontCls.setLabel(label, (String) getDefaultLanguage()); + } + } else { + ontCls.removeAll(RDFS.label); + } + } private String getLabel(String lang, ListlabelList) { Iterator labelIt = labelList.iterator(); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/VClassDaoJena.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/VClassDaoJena.java index dd308cf59..a64bdde55 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/VClassDaoJena.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/dao/jena/VClassDaoJena.java @@ -885,84 +885,34 @@ public class VClassDaoJena extends JenaBaseDao implements VClassDao { public void updateVClass(VClass cls, OntModel ontModel) { ontModel.enterCriticalSection(Lock.WRITE); - getOntModel().getBaseModel().notifyEvent(new EditEvent(getWebappDaoFactory().getUserURI(),true)); + ontModel.getBaseModel().notifyEvent(new EditEvent(getWebappDaoFactory().getUserURI(),true)); try { OntClass ontCls = ontModel.getOntClass(cls.getURI()); + if (ontCls != null) { - try { - if (cls.getName() != null && cls.getName().length() > 0) { - ontCls.setLabel(cls.getName(), (String) getDefaultLanguage()); - } else { - ontCls.removeAll(RDFS.label); - } - } catch (Exception e) { - log.error("error updating label for class "+cls.getURI()); - } - if (IN_CLASSGROUP != null) { - try { - if (cls.getGroupURI() != null) { - String badURIErrorStr = checkURI(cls.getGroupURI()); - if (badURIErrorStr == null) { - ontCls.addProperty(IN_CLASSGROUP, ResourceFactory.createResource(cls.getGroupURI())); - } else { - log.error(badURIErrorStr); - } - } else { - ontCls.removeAll(IN_CLASSGROUP); - } - } catch (Exception e) { - log.error("error linking class "+cls.getURI()+" to class group"); - } - } else { - log.error("vitro:inClassgroup property not found in RBox"); - } + updateRDFSLabel(ontCls, cls.getName()); + updatePropertyResourceURIValue(ontCls,IN_CLASSGROUP,cls.getGroupURI(),ontModel); updatePropertyStringValue(ontCls,SHORTDEF,cls.getShortDef(),ontModel); updatePropertyStringValue(ontCls,EXAMPLE_ANNOT,cls.getExample(),ontModel); updatePropertyStringValue(ontCls,DESCRIPTION_ANNOT,cls.getDescription(),ontModel); updatePropertyIntValue(ontCls,DISPLAY_LIMIT,cls.getDisplayLimit(),ontModel); updatePropertyIntValue(ontCls,DISPLAY_RANK_ANNOT,cls.getDisplayRank(),ontModel); updatePropertyFloatValue(ontCls, SEARCH_BOOST_ANNOT, cls.getSearchBoost(), ontModel); - if (HIDDEN_FROM_DISPLAY_BELOW_ROLE_LEVEL_ANNOT != null) { - try { - ontCls.removeAll(HIDDEN_FROM_DISPLAY_BELOW_ROLE_LEVEL_ANNOT); - if (cls.getHiddenFromDisplayBelowRoleLevel()!=null) { - String badURIErrorStr = checkURI(cls.getHiddenFromDisplayBelowRoleLevel().getURI()); - if (badURIErrorStr == null) { - ontCls.addProperty(HIDDEN_FROM_DISPLAY_BELOW_ROLE_LEVEL_ANNOT, ResourceFactory.createResource(cls.getHiddenFromDisplayBelowRoleLevel().getURI())); - } else { - log.error(badURIErrorStr); - } - } - } catch (Exception e) { - log.error("error linking class "+cls.getURI()+" to HIDDEN_FROM_DISPLAY_BELOW_ROLE_LEVEL edit role"); - } - } else { - log.error("vitro:hiddenFromDisplayBelowRoleLevelAnnot property not found in RBox"); + + if (cls.getHiddenFromDisplayBelowRoleLevel() != null) { + updatePropertyResourceURIValue(ontCls,HIDDEN_FROM_DISPLAY_BELOW_ROLE_LEVEL_ANNOT,cls.getHiddenFromDisplayBelowRoleLevel().getURI(),ontModel); + } + + if (cls.getProhibitedFromUpdateBelowRoleLevel() != null) { + updatePropertyResourceURIValue(ontCls,PROHIBITED_FROM_UPDATE_BELOW_ROLE_LEVEL_ANNOT,cls.getProhibitedFromUpdateBelowRoleLevel().getURI(),ontModel); } - if (PROHIBITED_FROM_UPDATE_BELOW_ROLE_LEVEL_ANNOT != null) { - try { - ontCls.removeAll(PROHIBITED_FROM_UPDATE_BELOW_ROLE_LEVEL_ANNOT); - if (cls.getProhibitedFromUpdateBelowRoleLevel()!=null) { - String badURIErrorStr = checkURI(cls.getProhibitedFromUpdateBelowRoleLevel().getURI()); - if (badURIErrorStr == null) { - ontCls.addProperty(PROHIBITED_FROM_UPDATE_BELOW_ROLE_LEVEL_ANNOT, ResourceFactory.createResource(cls.getProhibitedFromUpdateBelowRoleLevel().getURI())); - } else { - log.error(badURIErrorStr); - } - } - } catch (Exception e) { - log.error("error linking class "+cls.getURI()+" to PROHIBITED_FROM_UPDATE_BELOW_ROLE_LEVEL edit role"); - } - } else { - log.error("vitro:prohibitedFromUpdateBelowRoleLevelAnnot property not found in RBox"); - } updatePropertyStringValue(ontCls,PROPERTY_CUSTOMENTRYFORMANNOT,cls.getCustomEntryForm(),ontModel); updatePropertyStringValue(ontCls,PROPERTY_CUSTOMDISPLAYVIEWANNOT,cls.getCustomDisplayView(),ontModel); updatePropertyStringValue(ontCls,PROPERTY_CUSTOMSHORTVIEWANNOT,cls.getCustomShortView(),ontModel); updatePropertyStringValue(ontCls,PROPERTY_CUSTOMSEARCHVIEWANNOT,cls.getCustomSearchView(),ontModel); } else { - log.error("error: cannot find class "+cls.getURI()+" for updating"); + log.error("error: cannot find jena class "+cls.getURI()+" for updating"); } } finally { getOntModel().getBaseModel().notifyEvent(new EditEvent(getWebappDaoFactory().getUserURI(),false)); @@ -970,6 +920,7 @@ public class VClassDaoJena extends JenaBaseDao implements VClassDao { } } + private VClass vClassWebappFromOntClass(OntClass cls) { VClass vcw = new VClass(); getOntModel().enterCriticalSection(Lock.READ); diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/dao/jena/VClassDaoTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/dao/jena/VClassDaoTest.java new file mode 100644 index 000000000..e9cd60314 --- /dev/null +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/dao/jena/VClassDaoTest.java @@ -0,0 +1,145 @@ +/* $This file is distributed under the terms of the license in /doc/license.txt$ */ + +package edu.cornell.mannlib.vitro.webapp.dao.jena; + + +import org.junit.Assert; +import org.junit.Test; + +import com.hp.hpl.jena.ontology.OntClass; +import com.hp.hpl.jena.ontology.OntModel; +import com.hp.hpl.jena.ontology.OntModelSpec; +import com.hp.hpl.jena.rdf.model.Model; +import com.hp.hpl.jena.rdf.model.ModelFactory; + +import edu.cornell.mannlib.vitro.webapp.beans.VClass; +import edu.cornell.mannlib.vitro.webapp.dao.VitroVocabulary; + +/** + * + * + */ + +public class VClassDaoTest { + + @Test + // Test that the VClassDaoJena::updateVClass method will only update the jena model for + // those properties in VClass that have a different value from what is already in the + // jena model for that property. + // + // Specifically, VClass should not remove a statement from the model and then add the + // same statement back in. The reason for this is that in vivo the "immutable" properties + // are stored in a sub-model and the user-editable properties are stored in a super-model and + // all updates are performed against the super-model, so removing and then re-adding + // the same statement may result in a change of state (if the statement was in the sub-model + // it will migrate to the super-model) because of the way jena handles additions and + // deletions with respect to super and sub models. This migration of statements may cause + // undesirable behavior in the vivo/vitro application. + + public void modelIsolation(){ + + // 1. create two models and attach one as a sub-model of the other + // 2. populate the sub-model with one statement for each of the 14 properties represented in VClass + // 3. save the state of both the sub-model and the super-model + // 4. populate a VClass object with the data in the (combined) model and call the updateVClass method + // 5. verify that both the sub-model and the super-model are unchanged + + String class1URI = "http://test.vivo/AcademicDegree"; + + OntModel superModel = ModelFactory.createOntologyModel(OntModelSpec.OWL_MEM); // this simulates the user-editable ontology in vivo + OntModel subModel = ModelFactory.createOntologyModel(OntModelSpec.OWL_MEM); // this simulates the core ontology in vivo + superModel.addSubModel(subModel); + + String rdfsLabel = "this is the rdfs label"; + String lang = "en-US"; + + // populate sub-model + OntClass class1 = subModel.createClass(class1URI); + + class1.setLabel(rdfsLabel,lang); //rdfs:label + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.IN_CLASSGROUP), subModel.createResource("http://thisIsTheClassGroupURI")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.SHORTDEF), subModel.createTypedLiteral("this is the short definition")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.EXAMPLE_ANNOT), subModel.createTypedLiteral("this is the example - why is this a string?")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.DESCRIPTION_ANNOT), subModel.createTypedLiteral("this is the description")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.DISPLAY_LIMIT), subModel.createTypedLiteral(-1)); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.DISPLAY_RANK_ANNOT), subModel.createTypedLiteral(-11)); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.SEARCH_BOOST_ANNOT), subModel.createTypedLiteral(2.4f)); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.HIDDEN_FROM_DISPLAY_BELOW_ROLE_LEVEL_ANNOT), subModel.createResource("http://vitro.mannlib.cornell.edu/ns/vitro/role#curator")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.PROHIBITED_FROM_UPDATE_BELOW_ROLE_LEVEL_ANNOT), subModel.createResource("http://vitro.mannlib.cornell.edu/ns/vitro/role#selfEditor")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.PROPERTY_CUSTOMENTRYFORMANNOT), subModel.createTypedLiteral("this is the custom entry form annotation")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.PROPERTY_CUSTOMDISPLAYVIEWANNOT), subModel.createTypedLiteral("this is the custom display view annotation")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.PROPERTY_CUSTOMSHORTVIEWANNOT), subModel.createTypedLiteral("this is the custom short view annotation")); + class1.setPropertyValue(subModel.createProperty(VitroVocabulary.PROPERTY_CUSTOMSEARCHVIEWANNOT), subModel.createTypedLiteral("this is the custom search view annotation")); + + + // Save copies of sub-model and super-model + + // uncommment the next two lines to debug failures + //System.out.println("**Before updating VClass:"); + //printModels(superModel, subModel); + + OntModel origSubModel = ModelFactory.createOntologyModel(OntModelSpec.OWL_MEM); + origSubModel.add(subModel); + + superModel.remove(subModel); + OntModel origSuperModel = ModelFactory.createOntologyModel(OntModelSpec.OWL_MEM); + origSuperModel.add(superModel); + superModel.add(subModel); + + // Populate the VClass with the data in the sub-model and then update the combined model + WebappDaoFactoryJena wdfj = new WebappDaoFactoryJena(superModel); + VClassDaoJena vcdj = (VClassDaoJena) wdfj.getVClassDao(); + VClass vClass = vcdj.getVClassByURI(class1URI); // the VClass will be populated with the + // information already in the jena model. + + + Assert.assertEquals(vClass.getName(), class1.getLabel(lang)); // + + + vcdj.updateVClass(vClass); // we haven't changed any values here, so + // the models should be unchanged. + + // Verify that the sub-model and super-model are both unchanged + + // uncommment the next two lines to debug failures + //System.out.println("\n**After updating VClass:"); + //printModels(superModel,subModel); + + superModel.remove(subModel); + //modtime affects the diff but we don't care about that difference + wipeOutModTime(origSubModel); + wipeOutModTime(origSuperModel); + wipeOutModTime(subModel); + wipeOutModTime(superModel); + + Assert.assertTrue(subModel.isIsomorphicWith(origSubModel)); + Assert.assertTrue(superModel.isIsomorphicWith(origSuperModel)); + + } + + + void printModels(OntModel superModel, OntModel subModel) { + + // Detach the submodel for printing to get an accurate + // account of what is in each. + + superModel.remove(subModel); + + System.out.println("\nThe sub-model has " + subModel.size() + " statements:"); + System.out.println("---------------------------------------------------"); + subModel.writeAll(System.out,"N3",null); + + System.out.println("\nThe super-model has " + superModel.size() + " statements:"); + System.out.println("---------------------------------------------------"); + superModel.write(System.out,"N3",null); + + superModel.addSubModel(subModel); + + } + + + void wipeOutModTime(Model model){ + model.removeAll(null, model.createProperty(VitroVocabulary.MODTIME), null); + } + +}