Changeset user info #369
Changeset user info #369
Conversation
…mputer, JenaIngestController
ghost
linked an issue
There was a problem hiding this comment.
@litvinovg thanks for the contribution. Can you please take a look into my comments?
| @@ -659,7 +660,7 @@ private String processRenameResourceRequest(VitroRequest vreq) { | |||
| vreq.setAttribute("title","Rename Resource"); | |||
| return RENAME_RESOURCE; | |||
| } else { | |||
| String result = doRename(oldNamespace, newNamespace); | |||
| String result = doRename(oldNamespace, newNamespace, vreq); | |||
There was a problem hiding this comment.
Do you have some vision that some properties from VitroRequest might be needed into doRename method? I mean why you are not passing only N3EditUtils.getEditorURI(vreq) as you did in the ChangeSet addRemoval method?
| @@ -315,7 +317,7 @@ private long operateOnModel(WebappDaoFactory webappDaoFactory, | |||
| OntModelSelector ontModelSelector, | |||
| boolean remove, | |||
| boolean makeClassgroups, | |||
| String userURI) { | |||
| String userURI, String userId) { | |||
There was a problem hiding this comment.
one parameter per line (it might be fixed by code style formatting, but please fix this if it is possible now :-) )
| public static void applyChangesToWriteModel(AdditionsAndRetractions changes, RDFService rdfService, | ||
| String defaultGraphUri, String editorUri) { | ||
| appyChangesToWriteModel(changes, editorUri, rdfService, defaultGraphUri); | ||
| } | ||
|
|
||
| @Deprecated | ||
| public static void applyChangesToWriteModel(AdditionsAndRetractions changes, Model writeModel, String editorUri) { | ||
| RDFService rdfService = new RDFServiceModel(writeModel); | ||
| appyChangesToWriteModel(changes, editorUri, rdfService, null); | ||
| } | ||
|
|
||
| private static void appyChangesToWriteModel(AdditionsAndRetractions changes, String editorUri, | ||
| RDFService rdfService, String graphUri) { | ||
| ChangeSet cs = rdfService.manufactureChangeSet(); | ||
| cs.addPreChangeEvent(new BulkUpdateEvent(null, true)); | ||
| cs.addPostChangeEvent(new BulkUpdateEvent(null, false)); | ||
|
|
||
| ByteArrayOutputStream additionsStream = new ByteArrayOutputStream(); | ||
| changes.getAdditions().write(additionsStream, "N3"); | ||
| InputStream additionsInputStream = new ByteArrayInputStream(additionsStream.toByteArray()); | ||
|
|
||
| ByteArrayOutputStream retractionsStream = new ByteArrayOutputStream(); | ||
| changes.getRetractions().write(retractionsStream, "N3"); | ||
| InputStream retractionsInputStream = new ByteArrayInputStream(retractionsStream.toByteArray()); | ||
|
|
||
| cs.addAddition(additionsInputStream, RDFServiceUtils.getSerializationFormatFromJenaString("N3"), graphUri, editorUri); | ||
| cs.addRemoval(retractionsInputStream, RDFServiceUtils.getSerializationFormatFromJenaString("N3"), graphUri, editorUri); | ||
|
|
||
| try { | ||
| rdfService.changeSetUpdate(cs); | ||
| } catch (RDFServiceException e) { | ||
| log.error(e, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Not sure I understand the reason for existence (decoupling) of the first and third methods in this part. Can you please double check can we merge that into one method? Also, I am not sure it is a good practice having methods only different in order of parameters.
There was a problem hiding this comment.
Should be fixed now
There was a problem hiding this comment.
I removed deprecated method as it is not used anywhere in the code and creation RDFService from model is not considered as a good practice.
| private String getClassUri() { | ||
| return "java:" + this.getClass().getCanonicalName(); | ||
| } |
There was a problem hiding this comment.
so, here the information about the user who requested changes will be actually java class, right? For me it is ok to log that as well.
| import edu.cornell.mannlib.vitro.webapp.rdfservice.impl.RDFServiceUtils; | ||
| import edu.cornell.mannlib.vitro.webapp.rdfservice.impl.jena.model.RDFServiceModel; | ||
|
|
||
| public class RDFServiceNotifictaionTest { |
There was a problem hiding this comment.
there is a typo in the name of the class - Notification
| @@ -24,4 +24,9 @@ public OntModel getModel(HttpServletRequest request, ServletContext context) { | |||
|
|
|||
| public static final ModelSelector selector = new StandardModelSelector(); | |||
|
|
|||
| @Override | |||
| public String getDefaultGraphUri() { | |||
| return ABOX_UNION; | |||
There was a problem hiding this comment.
I found some errors in forms while using ABOX_UNION.
I haven't found any problems with ABOX_ASSERTIONS as default graph.
@brianjlowe Do you think it is ok to use ABOX_ASSERTIONS as a default graph?
|
Superseded by #390 |
VIVO GitHub issue
What does this pull request do?
Provide information about actor who requested model changes to change listeners.
What's new?
Extended ModelChange interface with getUserId and setUserId methods
Modified ModelChangeImpl, ChangeSetImpl to provide utility methods that requires userId.
Deprecated methods that don't require userId.
Use RDFService to write changes in ProcessRdfForm defined by n3 generators, deprecated old public method.
Provide information with ChangeSets in DeleteIndividualController, JenaIngestController, TranslationConverter, ABoxRecomputer, RDFUploadController.
Modified ModelSelector's interface and implementations to provide information about graphs defined in EditConfigurationVTwo
Added tests to cover RDFService model changes notifications
How should this be tested?
Interested parties
@brianjlowe @chenejac