From 17f3f734eb073c0b8f1d7b51cffc853e884b70a1 Mon Sep 17 00:00:00 2001 From: Heath Thomann Date: Mon, 27 Jun 2011 20:34:33 +0000 Subject: [PATCH] OPENJPA-2010: Check line number and source file before logging duplicate meta data warning - merged Mike's changes from trunk. git-svn-id: https://svn.apache.org/repos/asf/openjpa/branches/2.0.x@1140304 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/openjpa/jdbc/meta/MappingTool.java | 13 ++-- .../meta/AbstractCFMetaDataFactory.java | 20 +++-- .../apache/openjpa/meta/ClassMetaData.java | 8 +- .../org/apache/openjpa/meta/MetaDataTool.java | 4 +- .../apache/openjpa/meta/QueryMetaData.java | 10 ++- .../AnnotationPersistenceMetaDataParser.java | 23 ++++-- .../XMLPersistenceMetaDataParser.java | 76 ++++++++++++++++--- 7 files changed, 116 insertions(+), 38 deletions(-) diff --git a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java index 9c741d716..a87911b72 100644 --- a/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java +++ b/openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/meta/MappingTool.java @@ -527,11 +527,12 @@ public class MappingTool if (_mappingWriter != null) { output = new HashMap(); File tmp = new File("openjpatmp"); - for (int i = 0; i < mappings.length; i++) - mappings[i].setSource(tmp, SourceTracker.SRC_OTHER); - for (int i = 0; i < queries.length; i++) - queries[i].setSource(tmp, queries[i].getSourceScope(), - SourceTracker.SRC_OTHER); + for (int i = 0; i < mappings.length; i++) { + mappings[i].setSource(tmp, SourceTracker.SRC_OTHER, "openjpatmp"); + } + for (int i = 0; i < queries.length; i++) { + queries[i].setSource(tmp, queries[i].getSourceScope(), SourceTracker.SRC_OTHER, "openjpatmp"); + } for (int i = 0; i < seqs.length; i++) seqs[i].setSource(tmp, seqs[i].getSourceScope(), SourceTracker.SRC_OTHER); @@ -706,7 +707,7 @@ public class MappingTool && fmds[i].getDeclaredType() != Object.class) fmds[i].setDeclaredTypeCode(JavaTypes.PC); } - meta.setSource(_file, meta.getSourceType()); + meta.setSource(_file, meta.getSourceType(), _file == null ? "": _file.getPath() ); meta.setResolve(MODE_META, true); } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractCFMetaDataFactory.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractCFMetaDataFactory.java index 963410acd..b9be0ad8b 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractCFMetaDataFactory.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/AbstractCFMetaDataFactory.java @@ -381,10 +381,11 @@ public abstract class AbstractCFMetaDataFactory if (queries[i].getSourceMode() == MODE_QUERY || (mode & queries[i].getSourceMode()) == 0) continue; - if (queries[i].getSourceFile() == null) - queries[i].setSource(defaultSourceFile(queries[i], - clsNames), queries[i].getSourceScope(), - queries[i].getSourceType()); + if (queries[i].getSourceFile() == null) { + File defaultFile = defaultSourceFile(queries[i], clsNames); + queries[i].setSource(defaultFile, queries[i].getSourceScope(), queries[i].getSourceType(), + defaultFile == null ? "" : defaultFile.getPath()); + } if ((AccessController.doPrivileged( J2DoPrivHelper.existsAction(queries[i].getSourceFile()))) .booleanValue()) { @@ -423,9 +424,11 @@ public abstract class AbstractCFMetaDataFactory for (int i = 0; i < queries.length; i++) { if (queries[i].getSourceMode() != MODE_QUERY) continue; - if (queries[i].getSourceFile() == null) - queries[i].setSource(defaultSourceFile(queries[i], clsNames), - queries[i].getSourceScope(), queries[i].getSourceType()); + if (queries[i].getSourceFile() == null) { + File defaultFile = defaultSourceFile(queries[i], clsNames); + queries[i].setSource(defaultFile, queries[i].getSourceScope(), queries[i].getSourceType(), + defaultFile == null ? "" : defaultFile.getPath()); + } if ((AccessController.doPrivileged( J2DoPrivHelper.existsAction(queries[i].getSourceFile()))) .booleanValue()) { @@ -522,7 +525,8 @@ public abstract class AbstractCFMetaDataFactory * Set the current source file of the given metadata. */ protected void setSourceFile(ClassMetaData meta, File sourceFile) { - meta.setSource(sourceFile, meta.getSourceType()); + meta.setSource(sourceFile, meta.getSourceType(), sourceFile != null ? + sourceFile.getPath() : ""); } /** diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/ClassMetaData.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/ClassMetaData.java index 5933c4a97..e450db9fc 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/ClassMetaData.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/ClassMetaData.java @@ -147,6 +147,7 @@ public class ClassMetaData private final ValueMetaData _owner; private final LifecycleMetaData _lifeMeta = new LifecycleMetaData(this); private File _srcFile = null; + private String _srcName = null; private int _srcType = SRC_OTHER; private int _lineNum = 0; private int _colNum = 0; @@ -2393,9 +2394,10 @@ public class ClassMetaData return _srcType; } - public void setSource(File file, int srcType) { + public void setSource(File file, int srcType, String srcName) { _srcFile = file; _srcType = srcType; + _srcName = srcName; } public String getResourceName() { @@ -2762,5 +2764,9 @@ public class ClassMetaData return _cacheEnabled; return getPCSuperclassMetaData() != null ? getPCSuperclassMetaData().getCacheEnabled() : null; } + + public String getSourceName(){ + return _srcName; + } } diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataTool.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataTool.java index 9b2b9bc02..c913ce65a 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataTool.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/MetaDataTool.java @@ -175,7 +175,7 @@ public class MetaDataTool && fmds[i].getDeclaredType() != Object.class) fmds[i].setDeclaredTypeCode(JavaTypes.PC); } - meta.setSource(_file, meta.getSourceType()); + meta.setSource(_file, meta.getSourceType(), _file == null ? "" : _file.getPath()); _flush = true; } @@ -205,7 +205,7 @@ public class MetaDataTool output = new HashMap(); File tmp = new File("openjpatmp"); for (int i = 0; i < metas.length; i++) - metas[i].setSource(tmp, metas[i].getSourceType()); + metas[i].setSource(tmp, metas[i].getSourceType(), tmp.getPath()); } if (!mdf.store(metas, new QueryMetaData[0], new SequenceMetaData[0], MODE_META, output)) diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/QueryMetaData.java b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/QueryMetaData.java index 948f144cb..71b9aff91 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/meta/QueryMetaData.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/meta/QueryMetaData.java @@ -57,7 +57,8 @@ public class QueryMetaData private List _hintVals; private String _resultSetMappingName; private int _lineNum; - private int _colNum; + private int _colNum; + private String _srcName; /** * Construct with the given name. @@ -268,10 +269,11 @@ public class QueryMetaData return _srcType; } - public void setSource(File file, Object scope, int srcType) { + public void setSource(File file, Object scope, int srcType, String srcName) { _file = file; _scope = scope; _srcType = srcType; + _srcName = srcName; } public String getResourceName() { @@ -293,4 +295,8 @@ public class QueryMetaData public void setColNumber(int colNum) { _colNum = colNum; } + + public String getSourceName() { + return _srcName; + } } diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java index 862625c39..6f07a43e2 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AnnotationPersistenceMetaDataParser.java @@ -737,12 +737,18 @@ public class AnnotationPersistenceMetaDataParser */ private ClassMetaData getMetaData() { ClassMetaData meta = getRepository().getCachedMetaData(_cls); - if (meta != null && - ((isMetaDataMode() && (meta.getSourceMode() & MODE_META) != 0) || - (isMappingMode() && - (meta.getSourceMode() & MODE_MAPPING) != 0))) { - if (_log.isWarnEnabled()) + if (meta != null + && ((isMetaDataMode() + && (meta.getSourceMode() & MODE_META) != 0) + || (isMappingMode() && (meta.getSourceMode() & MODE_MAPPING) != 0) ) ) { + if (_log.isWarnEnabled()) { _log.warn(_loc.get("dup-metadata", _cls.getName())); + } + if(_log.isTraceEnabled()) { + _log.trace(String.format( + "MetaData originally obtained from file: %s under mode :%d with scope %s, and type :%d", + meta.getSourceName(), meta.getSourceMode(), meta.getSourceScope(), meta.getSourceType())); + } return null; } @@ -750,7 +756,8 @@ public class AnnotationPersistenceMetaDataParser meta = getRepository().addMetaData(_cls, getAccessCode(_cls)); meta.setEnvClassLoader(_envLoader); meta.setSourceMode(MODE_NONE); - meta.setSource(getSourceFile(), SourceTracker.SRC_ANNOTATIONS); + meta.setSource(getSourceFile(), SourceTracker.SRC_ANNOTATIONS, getSourceFile() == null ? "" + : getSourceFile().getPath()); } return meta; } @@ -1829,7 +1836,7 @@ public class AnnotationPersistenceMetaDataParser } meta.setSource(getSourceFile(), (el instanceof Class) ? el : null, - SourceTracker.SRC_ANNOTATIONS); + SourceTracker.SRC_ANNOTATIONS, getSourceFile() == null ? "" : getSourceFile().getPath()); if (isMetaDataMode()) meta.setSourceMode(MODE_META); else if (isMappingMode()) @@ -1906,7 +1913,7 @@ public class AnnotationPersistenceMetaDataParser meta.addHint(hint.name(), hint.value()); meta.setSource(getSourceFile(), (el instanceof Class) ? el : null, - SourceTracker.SRC_ANNOTATIONS); + SourceTracker.SRC_ANNOTATIONS, getSourceFile() == null ? "" : getSourceFile().getPath()); if (isMetaDataMode()) meta.setSourceMode(MODE_META); else if (isMappingMode()) diff --git a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java index 32c9bcdb6..3ace7a3ed 100644 --- a/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java +++ b/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/XMLPersistenceMetaDataParser.java @@ -870,12 +870,20 @@ public class XMLPersistenceMetaDataParser && ((isMetaDataMode() && (meta.getSourceMode() & MODE_META) != 0) || (isMappingMode() && (meta.getSourceMode() & MODE_MAPPING) != 0))) { - if (log.isWarnEnabled()) - log.warn(_loc.get("dup-metadata", _cls, getSourceName())); + if(isDuplicateClass(meta)) { + if (log.isWarnEnabled()) { + log.warn(_loc.get("dup-metadata", _cls, getSourceName())); + } + if(log.isTraceEnabled()) { + log.trace(String.format( + "MetaData originally obtained from source: %s under mode: %d with scope: %s, and type: %d", + meta.getSourceName(), meta.getSourceMode(), meta.getSourceScope(), meta.getSourceType())); + } + } _cls = null; return false; } - + int access = AccessCode.UNKNOWN; if (meta == null) { int accessCode = toAccessType(attrs.getValue("access")); @@ -895,17 +903,20 @@ public class XMLPersistenceMetaDataParser meta.setSourceMode(MODE_NONE); // parse annotations first so XML overrides them - if (_parser != null) + if (_parser != null) { _parser.parse(_cls); + } } access = meta.getAccessType(); boolean mappedSuper = "mapped-superclass".equals(elem); boolean embeddable = "embeddable".equals(elem); + if (isMetaDataMode()) { - meta.setSource(getSourceFile(), SourceTracker.SRC_XML); - meta.setSourceMode(MODE_META, true); Locator locator = getLocation().getLocator(); + meta.setSource(getSourceFile(), SourceTracker.SRC_XML, locator != null ? locator.getSystemId() : "" ); + meta.setSourceMode(MODE_META, true); + if (locator != null) { meta.setLineNumber(locator.getLineNumber()); meta.setColNumber(locator.getColumnNumber()); @@ -1686,7 +1697,7 @@ public class XMLPersistenceMetaDataParser Object cur = currentElement(); Object scope = (cur instanceof ClassMetaData) ? ((ClassMetaData) cur).getDescribedType() : null; - meta.setSource(getSourceFile(), scope, SourceTracker.SRC_XML); + meta.setSource(getSourceFile(), scope, SourceTracker.SRC_XML, locator == null ? "" : locator.getSystemId()); if (isMetaDataMode()) meta.setSourceMode(MODE_META); else if (isMappingMode()) @@ -1770,8 +1781,9 @@ public class XMLPersistenceMetaDataParser log.trace(_loc.get("parse-native-query", name)); QueryMetaData meta = getRepository().getCachedQueryMetaData(null, name); - if (meta != null && log.isWarnEnabled()) + if (meta != null && isDuplicateQuery(meta) ) { log.warn(_loc.get("override-query", name, currentLocation())); + } meta = getRepository().addQueryMetaData(null, name); meta.setDefiningType(_cls); @@ -1791,10 +1803,9 @@ public class XMLPersistenceMetaDataParser meta.setResultSetMappingName(val); Object cur = currentElement(); - Object scope = (cur instanceof ClassMetaData) - ? ((ClassMetaData) cur).getDescribedType() : null; - meta.setSource(getSourceFile(), scope, SourceTracker.SRC_XML); + Object scope = (cur instanceof ClassMetaData) ? ((ClassMetaData) cur).getDescribedType() : null; Locator locator = getLocation().getLocator(); + meta.setSource(getSourceFile(), scope, SourceTracker.SRC_XML, locator == null ? "" : locator.getSystemId()); if (locator != null) { meta.setLineNumber(locator.getLineNumber()); meta.setColNumber(locator.getColumnNumber()); @@ -2172,4 +2183,47 @@ public class XMLPersistenceMetaDataParser protected String normalizeCatalogName(String catName) { return catName; } + + /** + * Determines whether the ClassMetaData has been resolved more than once. Compares the current sourceName and + * linenumber to the ones used to originally resolve the metadata. + * + * @param meta The ClassMetaData to inspect. + * @return true if the source was has already been resolved from a different location. Otherwise return false + */ + protected boolean isDuplicateClass(ClassMetaData meta) { + if (!StringUtils.equals(getSourceName(), meta.getSourceName())) { + return true; + } + + if (getLineNum() != meta.getLineNumber()) { + return true; + } + return false; + } + + /** + * Determines whether the QueryMetaData has been resolved more than once. + * @param meta QueryMetaData that has already been resolved. + * @return true if the QueryMetaData was defined in a different place - e.g. another line in orm.xml. + */ + protected boolean isDuplicateQuery(QueryMetaData meta) { + if(! StringUtils.equals(getSourceName(), meta.getSourceName())) { + return true; + } + if(getLineNum() != meta.getLineNumber()) { + return true; + } + return false; + + } + + private int getLineNum() { + int lineNum = 0; + Locator loc = getLocation().getLocator(); + if(loc != null ) { + lineNum = loc.getLineNumber(); + } + return lineNum; + } }