Community
Participate
Working Groups
Build Identifier: 20100917-0705 1. Validation for converterClass attribute of converter The given converter class must implement the EclipseLink org.eclipse.persistence.mappings.converters.Converter interface. 2. Validation for converter attribute of structure converter The given converter class must implement the EclipseLink org.eclipse.persistence.mappings.converters.Converter interface. Reproducible: Always Steps to Reproduce: Annotation a java entity with the @Converter or @StructConverter Annotation or Specify the <converter> or <struct-converter> element in a mapping XML
Created attachment 185665 [details] proposed bug fix patch Attached a proposed bug fix patch.
Here is some feedback on the patch, some of which we have discussed. I haven't actually tested the patch yet, so these are just code observations. JDTTools. typeImplementsInterface(IJavaProject javaProject, IType type, IType interfase) - the second condition tested (typeName.equals(interfaceName)) should be a separate error case, not an answer of true to this method. resolveSuperInterfaceNames(...) - returned Iterable will only contain the first match for any set of resolved types. All matches should be added to cover the case where the type short name is ambiguous. - regarding the use of type.getSuperInterfaceNames(). After considering the trade-offs between using this method versus an ITypeHierarchy, I think using the method from the patch is reasonable given its performance benefits. The creation of a TypeHierarchy object can be long running, and as a result should only be done if necessary. This approach may have to be reconsidered in the future if gaps are found in the current approach. o.e.j.eclipselink.core.internal.context.orm - with these changes, it appears to be time to create a new class in the hierarchy to represent converters where a converter class is specified. This patch introduces a decent amount of duplicate code (in the validation logic for OrmEclipseLinkCustomConverter and OrmEclipseLinkStructConverter) and there are other parts of these two classes that are already the same. I suggest creating an abstract OrmEclipseLinkConverterClassConverter class to hold the common code between the 2 classes mentioned above. This should remove a good deal of duplicate code between the two classes. o.e.j.eclipselink.core.internal.resource.java.* o.e.j.eclipselink.core.resource.java - The changes contained in these packages appear to be from an earlier attempt at a solution.
Created attachment 186213 [details] patch with additional recommended changes
(In reply to comment #3) > Created attachment 186213 [details] > patch with additional recommended changes There appears to be a problem getting a text range for the StructConverter case for Java. I am getting the right problem in the Problems view, but not in the editor, and the problem in the Problems view does not indicate a line location.
Created attachment 186690 [details] proposed bug fix patch fixed issue with text range.
As we discussed, there is some duplicate code inside ASTTools after applying the patch. Please refactor ASTTools so that this duplicate code is eliminated.
Created attachment 187228 [details] proposed bug fix patch Implemented suggested re-factoring
Patch has been committed to head.
