Add delete metadata buttons #4890

IkramMaalej · 2021-12-17T12:07:43Z

Part of #4322
Fixes #4279
Fixes #4457
Fixes #4468
Fixes #4893
Fixes #4284

This PR :

offers the possibility to add, duplicate or delete a metadata as in metadata editor while creating a new process .
fixes all above announced issues

solth · 2021-12-17T22:14:39Z

Does this fix #4279?

solth · 2021-12-23T10:23:09Z

I get a PropertyNotFoundException when deleting a metadata that I just added:

delete-metadata-exception.mov

solth

Just a first part of the review. Will test more later.

One problem I encountered was that when I add a metadata group (for example "Person") during process creation and then add individutal metadata entries (for example "Display name" and "Family name") to the new group, the second entry will replace the first if I didn't add any values to the entries yet:

metadatagroup-entries.mov

Kitodo/src/main/java/org/kitodo/production/forms/createprocess/CreateProcessForm.java

Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessDetail.java

Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessMetadata.java

Kitodo/src/main/resources/messages/messages_de.properties

Kitodo/src/main/webapp/WEB-INF/resources/css/kitodo.css

matthias-ronge · 2022-01-10T12:56:06Z

Maybe this is because of additionallySelectedFields only contains the additionally selected field? When adding another sub-field, the metadata tree table is rebuilt and the first additionally selected field is no longer known. I think it is complicated to fix this

andre-hohmann · 2022-02-09T07:47:31Z

Summary

Cannot enter repeated keys in process creation #4279 -> is fixed
When adding sub-field in Metadata-Editor, existing sub-fields are doubled #4457 -> is fixed
Meta data editor - LABEL (mets-domain) key cannot be deleted but duplicated, even if it is "maxOccurs=1" #4468 -> is fixed
Meta Data Editor - when adding a whole keygroup, mandatory subkeys of similar groups get lost! #4465 -> i cannot assess this problem with our current configuration
The buttons for duplicating metadata in the subfields of metadata groups is always disabled #4893 -> is fixed
Doubling nested metadata entries does not copy the values #4284 -> is not fixed - see review below

Review

4284: The metadata group can be copied with its values, but when the metadata editor is closed and opened again, the copied metadata group is lost. When the process is validated an exception occurs - see below.
Add metadata during creation of process: If a metadata field is imported and the same metadata field is added manually to the imported metadata, the following behaviour occurs: The field is added behind the existing metadata field, but contains the value of the metadata field, which has been at this position before. The values of all following metadata fields are mixed up in the same way.
If a metadata field is added, which is not shown in the list of the imported metadata, everything works fine.
The goal should be that the metadata fields and its values should not be mixed up when existing metadata fields are added manually.

Exception message 4284

An error has occurred:
Date/time: 2022-02-09 08:27:09
User agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/97.0.4692.99 Safari/537.36
User IP: 134.100.172.21
Request URI: /cePreview/pages/metadataEditor.jsf
Ajax request: Yes
Status code: 500
Exception type: class java.lang.NullPointerException
Exception message:
Exception UUID:
Stack trace:
java.lang.NullPointerException
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:490)
	at java.base/java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:603)
	at java.base/java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:678)
	at java.base/java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:737)
	at java.base/java.util.stream.FindOps$FindOp.evaluateParallel(FindOps.java:160)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233)
	at java.base/java.util.stream.ReferencePipeline.findAny(ReferencePipeline.java:548)
	at org.kitodo.dataeditor.ruleset.NestedKeyView.retrieveOrCompute(NestedKeyView.java:468)
	at org.kitodo.dataeditor.ruleset.NestedKeyView.lambda$storeValues$3(NestedKeyView.java:460)
	at java.base/java.util.HashMap.computeIfAbsent(HashMap.java:1133)
	at org.kitodo.dataeditor.ruleset.NestedKeyView.storeValues(NestedKeyView.java:459)
	at org.kitodo.dataeditor.ruleset.NestedKeyView.createAuxiliaryTable(NestedKeyView.java:304)
	at org.kitodo.dataeditor.ruleset.NestedKeyView.getSortedVisibleMetadata(NestedKeyView.java:422)
	at org.kitodo.validation.metadata.MetadataValidation.checkForMandatoryQuantitiesOfTheMetadataRecursive(MetadataValidation.java:263)
	at org.kitodo.validation.metadata.MetadataValidation.checkMetadataRules(MetadataValidation.java:172)
	at org.kitodo.validation.metadata.MetadataValidation.validate(MetadataValidation.java:123)
	at org.kitodo.production.services.validation.MetadataValidationService.validate(MetadataValidationService.java:181)
	at org.kitodo.production.forms.dataeditor.DataEditorForm.validate(DataEditorForm.java:409)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.apache.el.parser.AstValue.invoke(AstValue.java:246)
	at org.apache.el.MethodExpressionImpl.invoke(MethodExpressionImpl.java:266)
	at org.jboss.weld.util.el.ForwardingMethodExpression.invoke(ForwardingMethodExpression.java:40)
	at org.jboss.weld.el.WeldMethodExpression.invoke(WeldMethodExpression.java:50)
	at org.jboss.weld.util.el.ForwardingMethodExpression.invoke(ForwardingMethodExpression.java:40)
	at org.jboss.weld.el.WeldMethodExpression.invoke(WeldMethodExpression.java:50)
	at org.apache.myfaces.view.facelets.el.ContextAwareTagMethodExpression.invoke(ContextAwareTagMethodExpression.java:96)
	at javax.faces.event.MethodExpressionActionListener.processAction(MethodExpressionActionListener.java:83)
	at javax.faces.event.ActionEvent.processListener(ActionEvent.java:58)
	at javax.faces.component.UIComponentBase.broadcast(UIComponentBase.java:429)
	at javax.faces.component.UICommand.broadcast(UICommand.java:103)
	at javax.faces.component.UIViewRoot._broadcastAll(UIViewRoot.java:1255)
	at javax.faces.component.UIViewRoot.broadcastEvents(UIViewRoot.java:420)
	at javax.faces.component.UIViewRoot._process(UIViewRoot.java:1741)
	at javax.faces.component.UIViewRoot.processApplication(UIViewRoot.java:935)
	at org.apache.myfaces.lifecycle.InvokeApplicationExecutor.execute(InvokeApplicationExecutor.java:42)
	at org.apache.myfaces.lifecycle.LifecycleImpl.executePhase(LifecycleImpl.java:195)
	at org.apache.myfaces.lifecycle.LifecycleImpl.execute(LifecycleImpl.java:142)
	at javax.faces.webapp.FacesServlet.service(FacesServlet.java:204)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.kitodo.production.servletfilter.EncodingFilter.doFilter(EncodingFilter.java:69)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:317)
	at org.kitodo.production.security.SecurityObjectAccessFilter.doFilter(SecurityObjectAccessFilter.java:86)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.invoke(FilterSecurityInterceptor.java:127)
	at org.springframework.security.web.access.intercept.FilterSecurityInterceptor.doFilter(FilterSecurityInterceptor.java:91)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.access.ExceptionTranslationFilter.doFilter(ExceptionTranslationFilter.java:114)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.session.SessionManagementFilter.doFilter(SessionManagementFilter.java:137)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.authentication.AnonymousAuthenticationFilter.doFilter(AnonymousAuthenticationFilter.java:111)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.servletapi.SecurityContextHolderAwareRequestFilter.doFilter(SecurityContextHolderAwareRequestFilter.java:170)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.savedrequest.RequestCacheAwareFilter.doFilter(RequestCacheAwareFilter.java:63)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.session.ConcurrentSessionFilter.doFilter(ConcurrentSessionFilter.java:155)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter.doFilter(AbstractAuthenticationProcessingFilter.java:200)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.authentication.logout.LogoutFilter.doFilter(LogoutFilter.java:116)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.header.HeaderWriterFilter.doFilterInternal(HeaderWriterFilter.java:66)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.context.SecurityContextPersistenceFilter.doFilter(SecurityContextPersistenceFilter.java:105)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.context.request.async.WebAsyncManagerIntegrationFilter.doFilterInternal(WebAsyncManagerIntegrationFilter.java:56)
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107)
	at org.springframework.security.web.FilterChainProxy$VirtualFilterChain.doFilter(FilterChainProxy.java:331)
	at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:214)
	at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:177)
	at org.springframework.web.filter.DelegatingFilterProxy.invokeDelegate(DelegatingFilterProxy.java:347)
	at org.springframework.web.filter.DelegatingFilterProxy.doFilter(DelegatingFilterProxy.java:263)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.logging.log4j.web.Log4jServletFilter.doFilter(Log4jServletFilter.java:71)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:196)
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:97)
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:661)
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:135)
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:81)
	at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:698)
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78)
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:364)
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:624)
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:831)
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1650)
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191)
	at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659)
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
	at java.base/java.lang.Thread.run(Thread.java:834)
Caused by: java.lang.NullPointerException
	at org.kitodo.dataeditor.ruleset.NestedKeyView.lambda$retrieveOrCompute$4(NestedKeyView.java:468)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:176)
	at java.base/java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1631)
	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:127)
	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:502)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:488)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
	at java.base/java.util.stream.FindOps$FindTask.doLeaf(FindOps.java:319)
	at java.base/java.util.stream.AbstractShortCircuitTask.compute(AbstractShortCircuitTask.java:115)
	at java.base/java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:746)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)

IkramMaalej · 2022-02-09T12:07:36Z

@andre-hohmann

4284: The metadata group can be copied with its values, but when the metadata editor is closed and opened again, the copied metadata group is lost.

Did you mean that the metadata editor is closed with 'Save & exit' button or with 'Exit' button?

andre-hohmann · 2022-02-09T13:19:54Z

I mean 'Save & exit'.

I guess you ask, because it works for you. Therefore i tested a bit and found out that it only happens, when the metadata fields are only added and not changed.
When i add at least one character in one sub-field of the duplicated metadata group, the whole metadata group is saved correctly and is shown after opening the metadata editor again.

IkramMaalej · 2022-02-11T11:45:51Z

@andre-hohmann

I mean 'Save & exit'.

I guess you ask, because it works for you. Therefore i tested a bit and found out that it only happens, when the metadata fields are only added and not changed. When i add at least one character in one sub-field of the duplicated metadata group, the whole metadata group is saved correctly and is shown after opening the metadata editor again.

What you have described above is the intended logic so far, because for storing the metadata we are using an object that doesn't allow duplicates (2 identical objects). But if we now want to save duplicates, then we have to adjust the logic accordingly.

andre-hohmann · 2022-02-11T12:15:55Z

I do not need duplicate objects, but i wanted to describe my observation. Usually, it is a bad sign, if (meta)data disappears. Maybe some users copy some elements and want them to adjust later, ...

In such cases, it would be helpful to show a message. In this case for example:

The duplicate metadata object is not saved when the editor closed.

Then it is clear that the following behaviour is intended.

matthias-ronge · 2022-02-14T17:42:32Z

@IkramMaalej is right: The metadata is stored internally as a set (in the mathematical sense). A set is defined as that its elements have no order, and a set cannot contain the same element more than once. Java automatically applies these rules to the metadata. This means that all elements of the set are compared with each other, and if an element is identical to another, i.e. it occurs more than once, it is deduplicated. For metadata, that's absolutely fine if the same thing doesn't occur multiple times.

The contract of the set does assure to you, that no metadata, that is not 100% identical, will ever be deleted.

This is not a bug.

andre-hohmann · 2022-02-14T18:08:38Z

@matthias-ronge : Thanks a lot for clarification. Then #4284 is solved, too. I do not know how to change the previous comments, but maybe that is the way to make the transparent discussions.

Thus, from my point of view, everything in the PR works fine except of:

Meta Data Editor - when adding a whole keygroup, mandatory subkeys of similar groups get lost! #4465, which i cannot assess.

andre-hohmann

#4465 is not assessed.

…cessForm

…editing the metadata tree table

IkramMaalej requested a review from solth December 17, 2021 12:07

IkramMaalej marked this pull request as draft December 22, 2021 16:37

IkramMaalej force-pushed the add-delete-Metadata-buttons branch from 09baa9d to f57bd53 Compare December 22, 2021 19:24

IkramMaalej marked this pull request as ready for review December 22, 2021 19:54

IkramMaalej force-pushed the add-delete-Metadata-buttons branch 2 times, most recently from 695c75f to a712808 Compare December 22, 2021 22:25

IkramMaalej force-pushed the add-delete-Metadata-buttons branch 2 times, most recently from 6be640d to 064cad0 Compare January 5, 2022 19:46

solth requested changes Jan 7, 2022

View reviewed changes

IkramMaalej force-pushed the add-delete-Metadata-buttons branch 2 times, most recently from ba5648c to 30d2eea Compare January 11, 2022 08:41

IkramMaalej requested a review from solth January 12, 2022 09:40

IkramMaalej force-pushed the add-delete-Metadata-buttons branch from 753211e to fc1baf1 Compare January 19, 2022 18:05

IkramMaalej force-pushed the add-delete-Metadata-buttons branch 2 times, most recently from 14cf54e to 10147dd Compare February 8, 2022 13:35

IkramMaalej requested a review from andre-hohmann February 16, 2022 12:09

andre-hohmann approved these changes Feb 24, 2022

View reviewed changes

IkramMaalej force-pushed the add-delete-Metadata-buttons branch from 9423e20 to 85bd82e Compare February 25, 2022 08:31

IkramMaalej added 2 commits February 25, 2022 09:43

Implement 'add process metadata' button

84935dd

Make 'delete' and 'duplicate' a metadata buttons visible in createPro…

b9195da

…cessForm

IkramMaalej added 17 commits February 25, 2022 09:43

Improve design

7ad86e9

Implement 'MetadataTreeTableInterface.java'

e32e8d3

Fix when duplicate button is disabled or not

255efda

Design improvement

60dd8a9

Fix add, delete and duplicate metadata buttons

b61bd9a

Fix 'PropertyNotFoundException' when deleting a just added metadata

badc252

Fix duplicate a metadata group

f92332c

Code improvement

649bcec

Fix disappearance of the newly added metadata with empty value after …

01082a8

…editing the metadata tree table

Code improvement

6589c15

Fix 'InvalidMetadataValueException'

f5d09ee

Fix cannot delete 'mets-domain' metadata

f59a60b

Improve design

bbac786

Fix deletion of imported metadata when creating a process

347a40d

Fix NPE when validating a process after adding a new metadata group

3fcbf71

Fix add metadata when creating a new process

7da845b

Fix rebase errors

feb5299

IkramMaalej force-pushed the add-delete-Metadata-buttons branch from 85bd82e to feb5299 Compare February 25, 2022 08:45

Kathrin-Huber merged commit ed3b6c6 into kitodo:master Feb 28, 2022

IkramMaalej deleted the add-delete-Metadata-buttons branch March 17, 2022 08:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add delete metadata buttons #4890

Add delete metadata buttons #4890

IkramMaalej commented Dec 17, 2021 •

edited by Kathrin-Huber

Loading

solth commented Dec 17, 2021

solth commented Dec 23, 2021

solth left a comment

matthias-ronge commented Jan 10, 2022

andre-hohmann commented Feb 9, 2022

IkramMaalej commented Feb 9, 2022

andre-hohmann commented Feb 9, 2022

IkramMaalej commented Feb 11, 2022

andre-hohmann commented Feb 11, 2022

matthias-ronge commented Feb 14, 2022

andre-hohmann commented Feb 14, 2022

andre-hohmann left a comment

Add delete metadata buttons #4890

Add delete metadata buttons #4890

Conversation

IkramMaalej commented Dec 17, 2021 • edited by Kathrin-Huber Loading

solth commented Dec 17, 2021

solth commented Dec 23, 2021

solth left a comment

Choose a reason for hiding this comment

matthias-ronge commented Jan 10, 2022

andre-hohmann commented Feb 9, 2022

Summary

Review

Exception message 4284

IkramMaalej commented Feb 9, 2022

andre-hohmann commented Feb 9, 2022

IkramMaalej commented Feb 11, 2022

andre-hohmann commented Feb 11, 2022

matthias-ronge commented Feb 14, 2022

andre-hohmann commented Feb 14, 2022

andre-hohmann left a comment

Choose a reason for hiding this comment

IkramMaalej commented Dec 17, 2021 •

edited by Kathrin-Huber

Loading