EDIT

Katja Luther wrote:

I also checked the actual nightly build and your test DB and all values are shown.

Now I could reproduce this behaviour, if a second exact value with min/max is added, the type is set to null...

I checked again and found that in the default perspective (Taxonomic) it shows up correctly while in the "Descriptive Dataset" perspective the problem appears. Can you check this?

I got when saving

last remote method : http://test.e-taxonomy.eu:80/cdmserver/rem_conf_am/remoting/common.service
last remote request client time : 2021-09-20T14:07:25.08
last remote request response header time : Mon, 20 Sep 2021 12:07:25 GMT
client error time : 2021-09-20T14:07:25.127
login : admin
editor version : 5.27.0.202109200833
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.27.0.0.20210913
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_131
org.eclipse.e4.core.di.InjectionException: java.lang.NullPointerException
    at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:65)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:282)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:247)
    at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:90)
    at org.eclipse.e4.ui.internal.workbench.PartServiceSaveHandler.save(PartServiceSaveHandler.java:57)
    at org.eclipse.ui.internal.WorkbenchWindow$7.save(WorkbenchWindow.java:594)
    at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.savePart(PartServiceImpl.java:1390)
    at eu.etaxonomy.taxeditor.workbench.SaveHandler.execute(SaveHandler.java:38)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:55)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:282)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:264)
    at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:132)
    at org.eclipse.e4.core.commands.internal.HandlerServiceHandler.execute(HandlerServiceHandler.java:152)
    at org.eclipse.core.commands.Command.executeWithChecks(Command.java:494)
    at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:488)
    at org.eclipse.e4.core.commands.internal.HandlerServiceImpl.executeHandler(HandlerServiceImpl.java:210)
    at org.eclipse.e4.ui.workbench.renderers.swt.HandledContributionItem.executeItem(HandledContributionItem.java:433)
    at org.eclipse.e4.ui.workbench.renderers.swt.AbstractContributionItem.handleWidgetSelection(AbstractContributionItem.java:454)
    at org.eclipse.e4.ui.workbench.renderers.swt.AbstractContributionItem$3.handleEvent(AbstractContributionItem.java:482)
    at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
    at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4418)
    at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1079)
    at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4236)
    at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3824)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$4.run(PartRenderingEngine.java:1121)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
    at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1022)
    at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:150)
    at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:693)
    at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
    at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:610)
    at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:148)
    at eu.etaxonomy.taxeditor.Application.start(Application.java:20)
    at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
    at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388)
    at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:673)
    at org.eclipse.equinox.launcher.Main.basicRun(Main.java:610)
    at org.eclipse.equinox.launcher.Main.run(Main.java:1519)
Caused by: java.lang.NullPointerException
    at eu.etaxonomy.cdm.persistence.dto.TermDto.fromTerm(TermDto.java:85)
    at eu.etaxonomy.cdm.persistence.dto.TermDto.fromTerm(TermDto.java:73)
    at eu.etaxonomy.cdm.api.service.dto.StatisticalMeasurementValueDto.fromStatisticalMeasurementValue(StatisticalMeasurementValueDto.java:34)
    at eu.etaxonomy.cdm.api.service.dto.QuantitativeDataDto.fromQuantitativeData(QuantitativeDataDto.java:46)
    at eu.etaxonomy.cdm.api.service.dto.DescriptionBaseDto.fromDescription(DescriptionBaseDto.java:249)
    at eu.etaxonomy.taxeditor.editor.descriptiveDataSet.matrix.CharacterMatrixPart.save(CharacterMatrixPart.java:249)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:55)
    ... 48 more

Changes in data do currently not get persisted

Andreas Müller wrote:

Katja Luther wrote:

I also checked the actual nightly build and your test DB and all values are shown.

Now I could reproduce this behaviour, if a second exact value with min/max is added, the type is set to null...

I checked again and found that in the default perspective (Taxonomic) it shows up correctly while in the "Descriptive Dataset" perspective the problem appears. Can you check this?

Also with perspective "Descriptive Dataset" and on pesi-hpc I can not reproduce this behaviour...

Andreas Müller wrote:

Changes in data do currently not get persisted

Did you restart the server?

now the matrix is visible also in DDS perspective again. For whatever reason the problem seems to be solved.

Katja Luther wrote:

Andreas Müller wrote:

Changes in data do currently not get persisted

Did you restart the server?

AM:
It was restarted after your last cdmlib commit. Ahh, but first time cdmlib did not build. OK, I try again.

The error changed a bit but I still get an NPE regarding the StatMeasureValues.type:

last remote method : http://test.e-taxonomy.eu:80/cdmserver/rem_conf_am/remoting/progressmonitor.service
last remote request client time : 2021-09-20T15:55:03.484
last remote request response header time : Mon, 20 Sep 2021 13:55:03 GMT
client error time : 2021-09-20T15:55:03.5
login : admin
editor version : 5.27.0.202109201248
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.27.0.0.20210913
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_131
java.lang.NullPointerException
    at eu.etaxonomy.cdm.api.service.dto.QuantitativeDataDto.getSpecificStatisticalValue(QuantitativeDataDto.java:76)
    at eu.etaxonomy.cdm.api.service.dto.RowWrapperDTO.generateQuantitativeDataString(RowWrapperDTO.java:141)
    at eu.etaxonomy.cdm.api.service.dto.RowWrapperDTO.generateDisplayString(RowWrapperDTO.java:133)
    at eu.etaxonomy.cdm.api.service.dto.RowWrapperDTO.<init>(RowWrapperDTO.java:61)
    at eu.etaxonomy.cdm.api.service.dto.SpecimenRowWrapperDTO.<init>(SpecimenRowWrapperDTO.java:38)
    at eu.etaxonomy.cdm.api.service.DescriptiveDataSetService.createSpecimenRowWrapper(DescriptiveDataSetService.java:425)
    at eu.etaxonomy.cdm.api.service.DescriptiveDataSetService.createSpecimenRowWrapper(DescriptiveDataSetService.java:432)
    at eu.etaxonomy.cdm.api.service.DescriptiveDataSetService.getRowWrapper(DescriptiveDataSetService.java:157)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:333)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:190)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
    at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:99)
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:283)
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:213)
    at com.sun.proxy.$Proxy737.getRowWrapper(Unknown Source)
    at eu.etaxonomy.cdm.api.service.longrunningService.LongRunningTasksService$1.doRun(LongRunningTasksService.java:72)
    at eu.etaxonomy.cdm.common.monitor.RemotingProgressMonitorThread.run(RemotingProgressMonitorThread.java:42)

Andreas Müller wrote:

The error changed a bit but I still get an NPE regarding the StatMeasureValues.type:

I checked the above and it is still that type=null data is created. Please check yourself with nightly and search for null values in the DB by

SELECT *
FROM StatisticalMeasurementValue smv
WHERE smv.type_id IS NULL

after editing StatMeasureValues.

The smv.type_id IS NULL issue seems to be solved now. But there is another critical issue: the unit_id is often null for quantitative data though a unit is shown in the popup dialogue for entering data. In the matrix the unit sometimes appears, sometimes it does not. This needs to be checked.

Andreas Müller wrote:

The smv.type_id IS NULL issue seems to be solved now. But there is another critical issue: the unit_id is often null for quantitative data though a unit is shown in the popup dialogue for entering data. In the matrix the unit sometimes appears, sometimes it does not. This needs to be checked.

this is fixed with the next commit.

We will need an update script to try to update existing data (in greece-bupleurum there are 787 quantitative data without unit_id)

If the unit is changed via the quantitative data editing dialog and persisted this change does not appear in the database

Katja Luther wrote:

Andreas Müller wrote:

Values are not persisted when entered into a taxon description (literatur description or default description). Or maybe there is another reason. I tested on "AM test dataset with the topmost 3 literatur description and a newly created default description.

This is critical and needs to be fixed before the release.

this is fixed with the last cdmlib commit.

When testing I got

last remote method : http://test.e-taxonomy.eu:80/cdmserver/rem_conf_am/remoting/description.service
last remote request client time : 2021-09-23T13:41:56.755
last remote request response header time : Thu, 23 Sep 2021 11:41:57 GMT
client error time : 2021-09-23T13:41:56.958
login : admin
editor version : 5.27.0.202109230931
server : test.e-taxonomy.eu (edit-test) / rem_conf_am
schema version : 5.27.1.0.20210922
os : Windows Server 2012 R2 6.3 amd64
java : 1.8.0_131
org.eclipse.e4.core.di.InjectionException: java.lang.NullPointerException
    at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:65)
    at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:282)
        ...
Caused by: java.lang.NullPointerException
    at eu.etaxonomy.cdm.api.service.DescriptionServiceImpl.mergeDescriptions(DescriptionServiceImpl.java:509)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:333)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:190)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:157)
    at org.springframework.transaction.interceptor.TransactionInterceptor$1.proceedWithInvocation(TransactionInterceptor.java:99)
    at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:283)
    at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
    at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:213)
    at com.sun.proxy.$Proxy694.mergeDescriptions(Unknown Source)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    ... 48 more

Andreas Müller wrote:

We will need an update script to try to update existing data (in greece-bupleurum there are 787 quantitative data without unit_id)

I created a new ticket for this: #9785

need to be tested:

• add new specimen, add character data -> save , add more character data -> save -> check for doubled entries -> works
• delete character data -> works
• add more than one statistical value -> works
• add literature description -> save -> add data to literature description -> works but sometimes the literature description is added in a new subtree, closing and reopening solves the problem and this problem does not appear if there exist a taxon description already
• add character data to specimen description with default description -> the color change still appears only when the matrix is reloaded. this needs some more work.
• add default description -> save -> add data to default description -> save -> works
• add literature description when editor is dirty -> save -> works

Katja Luther wrote:

need to be tested:

what does this mean? Did you test it or is it still to be done?

Andreas Müller wrote:

Katja Luther wrote:

need to be tested:

what does this mean? Did you test it or is it still to be done?

I do it right now. If you have other issues, need to be tested, you can add it.

Andreas Müller wrote:

Adding a new literature to a taxon does show the taxon row 2x if a specimen description for this taxon existed before already:

Upps I just see that this is mentioned in #9772#note-43 already. Sorry.
Is this issue difficult to fix?

Some more tests (with Lactucinae):

found two issues, both are already fixed:

• avoid NPE for quantitative data without measurement unit (for example number of chromosomes)
• before starting aggregation check for changes in matrix.

This generally seems to work now as expected.

I guess we need to create a couple of follow-up tickets. I could think about the following tickets:

• tests for all functionality related to this ticket and related to the correct handling RowWrapper related issues
• open issues (see commen 43)
• further DTOs to use (create a list of model instances that are still in use for the matrix)

Can you please also leave a note which model objects have been replaced by this ticket?

And can you check the (new) code, if there is still commented code, unnecessary code and code with unnecessary warnings?

Can you also check for other open matrix tickets and add the new tag "matrix" to them so we can check for all open matrix tickets.

Some open issues for the marix might be that there is still the "open in ..." context menu missing which might be very helpful.
Also it would be helpful if the taxon navigator reacts on a selected taxon if the "mit Taxon verknüpfen" button is checked in taxon naviator.
Also it would be great (at least for reviewing) if the records could also show id/uuid of row objects (taxon, specimen). Either in the row at the end (like the bulkeditor) or in the supplementatl data view.

Sooner or later we should also start to implement the details view again to be able to edit some more advanced data like the modifiers. This worked in earlier versions but was removed by PP when implementing a faster version. As we already handle some DTOs in details view I don't think it should be tooo difficult to reenable it. But definetely not a no brainer.

Can you check for existing tickets for all the above issues and create them if not existing?

Please check also if code is clean. E.g. I found DescriptiveDatasetService.getDescriptiveDataSetDtoByUuid() which includes DAO code like HQL where clauses. This should definetly move to persistence or is there a reason for having in in service layer?

Same in TaxonNodeServiceImpl.getTaxonNodeDtos().

By the way I wonder if all these HQL statements are necessary. Have you checked if you could use the methods in TaxonNodeFilterDaoHibernateImpl? These methods are well tested and will be maintained in case the TaxonNodeFilter is extended in future. As the TaxonNodeFilter was originally implemented for DDS I guess it could be used at some places instead of using own HQL code. Otherwise we get redundant code for this very complex type of filter which will be difficult to maintain.

But I haven't checked carefully so maybe I am wrong and in the given context the TNfilter can't be used.

Andreas Müller wrote:

Same in TaxonNodeServiceImpl.getTaxonNodeDtos().

By the way I wonder if all these HQL statements are necessary. Have you checked if you could use the methods in TaxonNodeFilterDaoHibernateImpl? These methods are well tested and will be maintained in case the TaxonNodeFilter is extended in future. As the TaxonNodeFilter was originally implemented for DDS I guess it could be used at some places instead of using own HQL code. Otherwise we get redundant code for this very complex type of filter which will be difficult to maintain.

But I haven't checked carefully so maybe I am wrong and in the given context the TNfilter can't be used.

The taxonNodeFilter is used to get the uuids of the filtered taxonnodes, these queries are used to get the dtos for the uuids.

Andreas Müller wrote:

Please check also if code is clean. E.g. I found DescriptiveDatasetService.getDescriptiveDataSetDtoByUuid() which includes DAO code like HQL where clauses. This should definetly move to persistence or is there a reason for having in in service layer?

done

Andreas Müller wrote:

Can you please also leave a note which model objects have been replaced by this ticket?

And can you check the (new) code, if there is still commented code, unnecessary code and code with unnecessary warnings?

In the former implementation the DDS was used as cdm object (containing taxonnodes and descriptions) and the descriptions included in rowWrappers where replaced by DTOs