From 6169c3079f3508164c0a7a21cde5cec1f4bd3b06 Mon Sep 17 00:00:00 2001 From: Koen Aers Date: Thu, 23 Oct 2025 19:05:16 +0200 Subject: [PATCH 1/5] HBX-3178: Improve the implementation of TableIdentifier Signed-off-by: Koen Aers --- .../tool/api/reveng/TableIdentifier.java | 56 +++++++++---------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/orm/src/main/java/org/hibernate/tool/api/reveng/TableIdentifier.java b/orm/src/main/java/org/hibernate/tool/api/reveng/TableIdentifier.java index 8c2b4e5817..189579290e 100644 --- a/orm/src/main/java/org/hibernate/tool/api/reveng/TableIdentifier.java +++ b/orm/src/main/java/org/hibernate/tool/api/reveng/TableIdentifier.java @@ -19,31 +19,33 @@ import org.hibernate.mapping.Table; +import java.util.Objects; + /** * Identifier used for the full name of tables. * @author max * */ public class TableIdentifier { - + public static TableIdentifier create(Table table) { return new TableIdentifier(table.getCatalog(), table.getSchema(), table.getName() ); } - + public static TableIdentifier create(String catalog, String schema, String name) { return new TableIdentifier(catalog, schema, name); } - + private final String catalog; private final String schema; private final String name; - + private TableIdentifier(String catalog, String schema, String name) { this.catalog = (catalog==null?null:catalog.intern() ); this.schema = (schema==null?null:schema.intern() ); - this.name = (name==null?null:name.intern() ); + this.name = (name==null?null:name.intern() ); } - + public String getCatalog() { return catalog; } @@ -53,47 +55,41 @@ public String getName() { public String getSchema() { return schema; } - + public boolean equals(Object obj) { - return obj instanceof TableIdentifier && equals( (TableIdentifier)obj); + return obj instanceof TableIdentifier + && isEqualIdentifier( (TableIdentifier)obj); } - - public boolean equals(TableIdentifier otherIdentifier) { + + private boolean isEqualIdentifier(TableIdentifier otherIdentifier) { if (otherIdentifier==null) return false; if (this==otherIdentifier) return true; - - if (equals(this.name, otherIdentifier.name) ) { - if(equals(this.schema, otherIdentifier.schema) ) { - return equals(this.catalog, otherIdentifier.catalog); + + if (Objects.equals(this.name, otherIdentifier.name) ) { + if(Objects.equals(this.schema, otherIdentifier.schema) ) { + return Objects.equals(this.catalog, otherIdentifier.catalog); } } - return false; } - + public int hashCode() { int result = 13; result = 37 * result + ( name==null ? 0 : name.hashCode() ); result = 37 * result + ( schema==null ? 0 : schema.hashCode() ); result = 37 * result + ( catalog==null ? 0 : catalog.hashCode() ); - + return result; } - - private boolean equals(String left, String right) { - if (left==right) return true; - if (left==null || right==null) return false; - return left.equals(right); - } - + public String toString() { - StringBuffer buf = new StringBuffer() - .append( "TableIdentifier" ) - .append('('); - if ( getCatalog()!=null ) buf.append( getCatalog() + "." ); - if ( getSchema()!=null ) buf.append( getSchema()+ "."); + StringBuilder buf = new StringBuilder() + .append( "TableIdentifier" ) + .append('('); + if ( getCatalog()!=null ) buf.append( getCatalog() ).append( "." ); + if ( getSchema()!=null ) buf.append( getSchema() ).append( "." ); buf.append( getName() ).append(')'); return buf.toString(); } - + } From 922ae971f33d1f60468a87add206a649e5ac2a83 Mon Sep 17 00:00:00 2001 From: Koen Aers Date: Fri, 24 Oct 2025 12:56:35 +0200 Subject: [PATCH 2/5] HBX-3179: Fix logging (and other issues) in TemplateHelper and TemplateProducer Signed-off-by: Koen Aers --- .../export/common/AbstractExporter.java | 136 +++---- .../export/common/TemplateHelper.java | 349 ++++++++---------- .../export/common/TemplateProducer.java | 50 ++- 3 files changed, 244 insertions(+), 291 deletions(-) diff --git a/orm/src/main/java/org/hibernate/tool/internal/export/common/AbstractExporter.java b/orm/src/main/java/org/hibernate/tool/internal/export/common/AbstractExporter.java index 4ca1e3919f..e41965b3f4 100644 --- a/orm/src/main/java/org/hibernate/tool/internal/export/common/AbstractExporter.java +++ b/orm/src/main/java/org/hibernate/tool/internal/export/common/AbstractExporter.java @@ -44,34 +44,34 @@ public abstract class AbstractExporter implements Exporter, ExporterConstants { protected Logger log = Logger.getLogger(this.getClass()); - + private TemplateHelper vh; - private Properties properties = new Properties(); + private final Properties properties = new Properties(); private Metadata metadata = null; private Iterator> iterator; - private Cfg2HbmTool c2h; - private Cfg2JavaTool c2j; + private final Cfg2HbmTool c2h; + private final Cfg2JavaTool c2j; public AbstractExporter() { c2h = new Cfg2HbmTool(); c2j = new Cfg2JavaTool(); - getProperties().put(ARTIFACT_COLLECTOR, new DefaultArtifactCollector()); - getProperties().put(TEMPLATE_PATH, new String[0]); + properties.put(ARTIFACT_COLLECTOR, new DefaultArtifactCollector()); + properties.put(TEMPLATE_PATH, new String[0]); } - + protected MetadataDescriptor getMetadataDescriptor() { return (MetadataDescriptor)getProperties().get(METADATA_DESCRIPTOR); } - + public Metadata getMetadata() { if (metadata == null) { metadata = buildMetadata(); } return metadata; } - + protected File getOutputDirectory() { return (File)getProperties().get(DESTINATION_FOLDER); } @@ -79,35 +79,35 @@ protected File getOutputDirectory() { public Properties getProperties() { return properties; } - + public ArtifactCollector getArtifactCollector() { return (ArtifactCollector)getProperties().get(ARTIFACT_COLLECTOR); } - + public String getName() { return this.getClass().getName(); } - + public Cfg2HbmTool getCfg2HbmTool() { return c2h; } - + public Cfg2JavaTool getCfg2JavaTool() { return c2j; } - + public void start() { setTemplateHelper( new TemplateHelper() ); setupTemplates(); setupContext(); doStart(); - cleanUpContext(); + cleanUpContext(); setTemplateHelper(null); getArtifactCollector().formatFiles(); } - + abstract protected void doStart(); - + protected void cleanUpContext() { if(getProperties()!=null) { iterator = getProperties().entrySet().iterator(); @@ -116,31 +116,31 @@ protected void cleanUpContext() { Object value = transformValue(element.getValue()); String key = element.getKey().toString(); if(key.startsWith(ExporterSettings.PREFIX_KEY)) { - getTemplateHelper().removeFromContext(key.substring(ExporterSettings.PREFIX_KEY.length()), value); + getTemplateHelper().removeFromContext(key.substring(ExporterSettings.PREFIX_KEY.length())); } - getTemplateHelper().removeFromContext(key, value); + getTemplateHelper().removeFromContext(key); } } if(getOutputDirectory()!=null) { - getTemplateHelper().removeFromContext("outputdir", getOutputDirectory()); + getTemplateHelper().removeFromContext("outputdir"); } String[] templatePath = (String[])getProperties().get(TEMPLATE_PATH); if(templatePath!=null) { - getTemplateHelper().removeFromContext("template_path", templatePath); + getTemplateHelper().removeFromContext("template_path"); + } + getTemplateHelper().removeFromContext("exporter"); + getTemplateHelper().removeFromContext("artifacts"); + if(getMetadata() != null) { + getTemplateHelper().removeFromContext("md"); + getTemplateHelper().removeFromContext("props"); + getTemplateHelper().removeFromContext("tables"); } - getTemplateHelper().removeFromContext("exporter", this); - getTemplateHelper().removeFromContext("artifacts", getArtifactCollector()); - if(getMetadata() != null) { - getTemplateHelper().removeFromContext("md", metadata); - getTemplateHelper().removeFromContext("props", getProperties()); - getTemplateHelper().removeFromContext("tables", metadata.collectTableMappings()); - } - getTemplateHelper().removeFromContext("c2h", getCfg2HbmTool()); - getTemplateHelper().removeFromContext("c2j", getCfg2JavaTool()); + getTemplateHelper().removeFromContext("c2h"); + getTemplateHelper().removeFromContext("c2j"); } protected void setupContext() { - getTemplateHelper().setupContext(); + getTemplateHelper().setupContext(); getTemplateHelper().putInContext("exporter", this); getTemplateHelper().putInContext("c2h", getCfg2HbmTool()); getTemplateHelper().putInContext("c2j", getCfg2JavaTool()); @@ -149,7 +149,7 @@ protected void setupContext() { } String[] templatePath = (String[])getProperties().get(TEMPLATE_PATH); if(templatePath!=null) { - getTemplateHelper().putInContext("template_path", templatePath); + getTemplateHelper().putInContext("template_path", templatePath); } if(getProperties()!=null) { iterator = getProperties().entrySet().iterator(); @@ -170,16 +170,16 @@ protected void setupContext() { catch (Exception e) { throw new RuntimeException("Exception when instantiating tool " + element.getKey() + " with " + value,e); } - } - } + } + } } } getTemplateHelper().putInContext("artifacts", getArtifactCollector()); - if(getMetadata() != null) { - getTemplateHelper().putInContext("md", metadata); - getTemplateHelper().putInContext("props", getProperties()); - getTemplateHelper().putInContext("tables", metadata.collectTableMappings()); - } + if(getMetadata() != null) { + getTemplateHelper().putInContext("md", metadata); + getTemplateHelper().putInContext("props", getProperties()); + getTemplateHelper().putInContext("tables", metadata.collectTableMappings()); + } } protected void setupTemplates() { @@ -187,9 +187,9 @@ protected void setupTemplates() { if(log.isDebugEnabled()) { log.debug(getClass().getName() + " outputdir:" + getOutputDirectory() + " path: " + toString(templatePath) ); } - getTemplateHelper().init(getOutputDirectory(), templatePath); + getTemplateHelper().init(getOutputDirectory(), templatePath); } - + protected void setTemplateHelper(TemplateHelper vh) { this.vh = vh; } @@ -197,40 +197,40 @@ protected void setTemplateHelper(TemplateHelper vh) { protected TemplateHelper getTemplateHelper() { return vh; } - + protected File getFileForClassName(File baseDir, String className, String extension) { - String filename = StringHelper.unqualify(className) + extension; - String packagename = StringHelper.qualifier(className); - return new File(getDirForPackage(baseDir, packagename), filename); - } - + String filename = StringHelper.unqualify(className) + extension; + String packagename = StringHelper.qualifier(className); + return new File(getDirForPackage(baseDir, packagename), filename); + } + protected Metadata buildMetadata() { return getMetadataDescriptor().createMetadata(); } - private File getDirForPackage(File baseDir, String packageName) { - String p = packageName == null ? "" : packageName; - return new File( baseDir, p.replace('.', File.separatorChar) ); - } + private File getDirForPackage(File baseDir, String packageName) { + String p = packageName == null ? "" : packageName; + return new File( baseDir, p.replace('.', File.separatorChar) ); + } private String toString(Object[] a) { - if (a == null) - return "null"; - if (a.length == 0) - return "[]"; - StringBuffer buf = new StringBuffer(); - for (int i = 0; i < a.length; i++) { - if (i == 0) - buf.append('['); - else - buf.append(", "); - - buf.append(String.valueOf(a[i])); - } - buf.append("]"); - return buf.toString(); - } - + if (a == null) + return "null"; + if (a.length == 0) + return "[]"; + StringBuilder buf = new StringBuilder(); + for (int i = 0; i < a.length; i++) { + if (i == 0) + buf.append('['); + else + buf.append(", "); + + buf.append(String.valueOf(a[i])); + } + buf.append("]"); + return buf.toString(); + } + private Object transformValue(Object value) { if("true".equals(value)) { return Boolean.TRUE; diff --git a/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateHelper.java b/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateHelper.java index d931786c99..3857d742d0 100644 --- a/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateHelper.java +++ b/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateHelper.java @@ -37,7 +37,6 @@ import freemarker.cache.ClassTemplateLoader; import freemarker.cache.FileTemplateLoader; import freemarker.cache.MultiTemplateLoader; -import freemarker.cache.URLTemplateLoader; import freemarker.cache.TemplateLoader; import freemarker.ext.beans.BeansWrapperBuilder; import freemarker.template.Configuration; @@ -45,7 +44,6 @@ import freemarker.template.SimpleHash; import freemarker.template.Template; import freemarker.template.TemplateDateModel; -import freemarker.template.TemplateException; import freemarker.template.TemplateModel; import freemarker.template.TemplateModelException; @@ -59,10 +57,9 @@ * */ public class TemplateHelper { - + static final Logger log = Logger.getLogger(TemplateHelper.class); - - private String templatePrefix; + private File outputDirectory; protected Configuration freeMarkerEngine; @@ -70,115 +67,111 @@ public class TemplateHelper { protected SimpleHash context; public TemplateHelper() { - + } - - public void init(File outputDirectory, String[] templatePaths) { - this.outputDirectory = outputDirectory; - - context = new SimpleHash(new BeansWrapperBuilder(Configuration.VERSION_2_3_0).build()); - freeMarkerEngine = new Configuration(Configuration.VERSION_2_3_0); - - List loaders = new ArrayList(); - - for (int i = 0; i < templatePaths.length; i++) { - File file = new File(templatePaths[i]); - if(file.exists()) { - if (file.isDirectory()) { - try { - loaders.add(new FileTemplateLoader(file)); - } catch (IOException e) { - throw new RuntimeException("Problems with templatepath " + file, e); - } - } else if (file.getName().endsWith(".zip") || file.getName().endsWith(".jar")) { - final URLClassLoader classLoaderForZip; - try { - classLoaderForZip = new URLClassLoader(new URL[]{ file.toURI().toURL() }, null); - } catch (MalformedURLException e) { - throw new RuntimeException("template path " + file + " is not a valid zip file", e); - } - - loaders.add(new ClassTemplateLoader(classLoaderForZip, "/")); - } else { - log.warn("template path " + file + " is not a directory"); - } - } else { - log.warn("template path " + file + " does not exist"); - } + + public void init(File outputDirectory, String[] templatePaths) { + this.outputDirectory = outputDirectory; + + context = new SimpleHash(new BeansWrapperBuilder(Configuration.VERSION_2_3_0).build()); + freeMarkerEngine = new Configuration(Configuration.VERSION_2_3_0); + + List loaders = new ArrayList<>(); + + for ( String templatePath : templatePaths ) { + File file = new File( templatePath ); + if ( file.exists() ) { + if ( file.isDirectory() ) { + try { + loaders.add( new FileTemplateLoader( file ) ); + } + catch (IOException e) { + throw new RuntimeException( "Problems with templatepath " + file, e ); + } + } + else if ( file.getName().endsWith( ".zip" ) || file.getName().endsWith( ".jar" ) ) { + final URLClassLoader classLoaderForZip; + try { + classLoaderForZip = new URLClassLoader( new URL[] {file.toURI().toURL()}, null ); + } + catch (MalformedURLException e) { + throw new RuntimeException( "template path " + file + " is not a valid zip file", e ); + } + + loaders.add( new ClassTemplateLoader( classLoaderForZip, "/" ) ); + } + else { + log.warn( "template path " + file + " is not a directory" ); + } + } + else { + log.warn( "template path " + file + " does not exist" ); + } } - loaders.add(new ClassTemplateLoader(this.getClass(),"/")); // the template names are like pojo/Somewhere so have to be a rooted classpathloader - - freeMarkerEngine.setTemplateLoader(new MultiTemplateLoader((TemplateLoader[]) loaders.toArray(new TemplateLoader[loaders.size()]))); - - } - - - public class Templates { - - public void createFile(String content, String fileName) { - Writer fw = null; - try { - fw = new BufferedWriter(new FileWriter(new File(getOutputDirectory(), fileName))); - fw.write(content); - } catch(IOException io) { - throw new RuntimeException("Problem when writing to " + fileName, io); - } finally { - if(fw!=null) { - try { - fw.flush(); - fw.close(); - } catch(IOException io ) { - //TODO: warn - } - } - } - } - } - - public File getOutputDirectory() { + loaders.add(new ClassTemplateLoader(this.getClass(),"/")); // the template names are like pojo/Somewhere so have to be a rooted classpathloader + + freeMarkerEngine.setTemplateLoader(new MultiTemplateLoader( loaders.toArray( new TemplateLoader[0] ) )); + + } + + + public class Templates { + + public void createFile(String content, String fileName) { + Writer fw = null; + try { + fw = new BufferedWriter(new FileWriter(new File(getOutputDirectory(), fileName))); + fw.write(content); + } catch(IOException io) { + throw new RuntimeException("Problem when writing to " + fileName, io); + } finally { + if(fw!=null) { + try { + fw.flush(); + fw.close(); + } catch(IOException io ) { + //TODO: warn + } + } + } + } + } + + public File getOutputDirectory() { return outputDirectory; } - - - public void putInContext(String key, Object value) { - log.trace("putInContext " + key + "=" + value); - if(value == null) throw new IllegalStateException("value must not be null for " + key); - Object replaced = internalPutInContext(key,value); - if(replaced!=null) { - log.warn( "Overwriting " + replaced + " when setting " + key + " to " + value + "."); - } - } - - public void removeFromContext(String key, Object expected) { - log.trace("removeFromContext " + key + "=" + expected); - Object replaced = internalRemoveFromContext(key); - if(replaced==null) throw new IllegalStateException(key + " did not exist in template context."); - /*if(replaced!=expected) { //FREEMARKER-TODO: how can i validate this ? or maybe not needed to validate since mutation is considered bad ? - throw new IllegalStateException("expected " + key + " to be bound to " + expected + " but was to " + replaced); - }*/ - } - - public void ensureExistence(File destination) { - // if the directory exists, make sure it is a directory - File dir = destination.getAbsoluteFile().getParentFile(); - if ( dir.exists() && !dir.isDirectory() ) { - throw new RuntimeException("The path: " + dir.getAbsolutePath() + " exists, but is not a directory"); - } // else make the directory and any non-existent parent directories - else if ( !dir.exists() ) { - if ( !dir.mkdirs() ) { - if(dir.getName().equals(".")) { // Workaround that Linux/JVM apparently can't handle mkdirs of File's with current dir references. - if(dir.getParentFile().mkdirs()) { - return; - } - } - throw new RuntimeException( "unable to create directory: " + dir.getAbsolutePath() ); - } - } - } - - protected String getTemplatePrefix() { - return templatePrefix; + + + public void putInContext(String key, Object value) { + if(value == null) throw new IllegalStateException("value must not be null for " + key); + Object replaced = internalPutInContext(key,value); + if(replaced!=null) { + log.warn( "Overwriting context: " + replaced + "."); + } + } + + public void removeFromContext(String key) { + Object replaced = internalRemoveFromContext(key); + if(replaced==null) throw new IllegalStateException(key + " did not exist in template context."); + } + + public void ensureExistence(File destination) { + // if the directory exists, make sure it is a directory + File dir = destination.getAbsoluteFile().getParentFile(); + if ( dir.exists() && !dir.isDirectory() ) { + throw new RuntimeException("The path: " + dir.getAbsolutePath() + " exists, but is not a directory"); + } // else make the directory and any non-existent parent directories + else if ( !dir.exists() ) { + if ( !dir.mkdirs() ) { + if(dir.getName().equals(".")) { // Workaround that Linux/JVM apparently can't handle mkdirs of File's with current dir references. + if(dir.getParentFile().mkdirs()) { + return; + } + } + throw new RuntimeException( "unable to create directory: " + dir.getAbsolutePath() ); + } + } } protected SimpleHash getContext() { @@ -186,110 +179,76 @@ protected SimpleHash getContext() { } public void processString(String template, Writer output) { - - try { - Reader r = new StringReader(template); + + try { + Reader r = new StringReader(template); Template t = new Template("unknown", r, freeMarkerEngine); - - t.process(getContext(), output); - } - catch (IOException e) { - throw new RuntimeException("Error while processing template string", e); - } - catch (TemplateException te) { - throw new RuntimeException("Error while processing template string", te); - } - catch (Exception e) { - throw new RuntimeException("Error while processing template string", e); - } + + t.process(getContext(), output); + } + catch (Exception e) { + throw new RuntimeException("Error while processing template string", e); + } } - - public void setupContext() { - getContext().put("version", Version.versionString()); - getContext().put("ctx", getContext() ); //TODO: I would like to remove this, but don't know another way to actually get the list possible "root" keys for debugging. - getContext().put("templates", new Templates()); - - getContext().put("date", new SimpleDate(new Date(), TemplateDateModel.DATETIME)); - - } - - protected Object internalPutInContext(String key, Object value) { - TemplateModel model = null; + + public void setupContext() { + getContext().put("version", Version.versionString()); + getContext().put("ctx", getContext() ); //TODO: I would like to remove this, but don't know another way to actually get the list possible "root" keys for debugging. + getContext().put("templates", new Templates()); + + getContext().put("date", new SimpleDate(new Date(), TemplateDateModel.DATETIME)); + + } + + protected Object internalPutInContext(String key, Object value) { + TemplateModel model; try { model = getContext().get(key); } catch (TemplateModelException e) { throw new RuntimeException("Could not get key " + key, e); } - getContext().put(key, value); - return model; - } - - protected Object internalRemoveFromContext(String key) { - TemplateModel model = null; + getContext().put(key, value); + return model; + } + + protected Object internalRemoveFromContext(String key) { + TemplateModel model; try { model = getContext().get(key); } catch (TemplateModelException e) { throw new RuntimeException("Could not get key " + key, e); } - getContext().remove(key); - return model; - } - - /** look up the template named templateName via the paths and print the content to the output */ - public void processTemplate(String templateName, Writer output, String rootContext) { - if(rootContext == null) { - rootContext = "Unknown context"; - } - - try { - Template template = freeMarkerEngine.getTemplate(templateName); - template.process(getContext(), output); - } - catch (IOException e) { - throw new RuntimeException("Error while processing " + rootContext + " with template " + templateName, e); - } - catch (TemplateException te) { - throw new RuntimeException("Error while processing " + rootContext + " with template " + templateName, te); - } - catch (Exception e) { - throw new RuntimeException("Error while processing " + rootContext + " with template " + templateName, e); - } - } - - - /** - * Check if the template exists. Tries to search with the templatePrefix first and then secondly without the template prefix. - * - * @param name - * @return - */ - /*protected String getTemplateName(String name) { - if(!name.endsWith(".ftl")) { - name = name + ".ftl"; - } - - if(getTemplatePrefix()!=null && templateExists(getTemplatePrefix() + name)) { - return getTemplatePrefix() + name; - } - - if(templateExists(name)) { - return name; - } - - throw new ExporterException("Could not find template with name: " + name); - }*/ - - public boolean templateExists(String templateName) { - TemplateLoader templateLoader = freeMarkerEngine.getTemplateLoader(); - - try { + getContext().remove(key); + return model; + } + + /** look up the template named templateName via the paths and print the content to the output */ + public void processTemplate(String templateName, Writer output, String rootContext) { + if(rootContext == null) { + rootContext = "Unknown context"; + } + + try { + Template template = freeMarkerEngine.getTemplate(templateName); + template.process(getContext(), output); + } + catch (Exception e) { + throw new RuntimeException("Error while processing " + rootContext + " with template " + templateName, e); + } + } + + + public boolean templateExists(String templateName) { + TemplateLoader templateLoader = freeMarkerEngine.getTemplateLoader(); + + try { return templateLoader.findTemplateSource(templateName)!=null; } catch (IOException e) { throw new RuntimeException("templateExists for " + templateName + " failed", e); } - } + } } diff --git a/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateProducer.java b/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateProducer.java index 225a408c8a..ef46146fca 100644 --- a/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateProducer.java +++ b/orm/src/main/java/org/hibernate/tool/internal/export/common/TemplateProducer.java @@ -22,7 +22,6 @@ import java.io.FileWriter; import java.io.IOException; import java.io.StringWriter; -import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; @@ -34,33 +33,33 @@ public class TemplateProducer { private static final Logger log = Logger.getLogger(TemplateProducer.class); private final TemplateHelper th; - private ArtifactCollector ac; - + private final ArtifactCollector ac; + public TemplateProducer(TemplateHelper th, ArtifactCollector ac) { this.th = th; this.ac = ac; } - + public void produce(Map additionalContext, String templateName, File destination, String identifier, String fileType, String rootContext) { - + String tempResult = produceToString( additionalContext, templateName, rootContext ); - - if(tempResult.trim().length()==0) { + + if( tempResult.trim().isEmpty() ) { log.warn("Generated output is empty. Skipped creation for file " + destination); return; } FileWriter fileWriter = null; try { - - th.ensureExistence( destination ); - + + th.ensureExistence( destination ); + ac.addFile(destination, fileType); log.debug("Writing " + identifier + " to " + destination.getAbsolutePath() ); fileWriter = new FileWriter(destination); - fileWriter.write(tempResult); - } + fileWriter.write(tempResult); + } catch (Exception e) { - throw new RuntimeException("Error while writing result to file", e); + throw new RuntimeException("Error while writing result to file", e); } finally { if(fileWriter!=null) { try { @@ -69,21 +68,20 @@ public void produce(Map additionalContext, String templateName, F } catch (IOException e) { log.warn("Exception while flushing/closing " + destination,e); - } + } } } - + } private String produceToString(Map additionalContext, String templateName, String rootContext) { - Map contextForFirstPass = additionalContext; - putInContext( th, contextForFirstPass ); + putInContext( th, additionalContext ); StringWriter tempWriter = new StringWriter(); BufferedWriter bw = new BufferedWriter(tempWriter); // First run - writes to in-memory string th.processTemplate(templateName, bw, rootContext); - removeFromContext( th, contextForFirstPass ); + removeFromContext( th, additionalContext ); try { bw.flush(); } @@ -94,18 +92,14 @@ private String produceToString(Map additionalContext, String temp } private void removeFromContext(TemplateHelper templateHelper, Map context) { - Iterator> iterator = context.entrySet().iterator(); - while ( iterator.hasNext() ) { - Entry element = iterator.next(); - templateHelper.removeFromContext((String) element.getKey(), element.getValue()); + for ( Entry element : context.entrySet() ) { + templateHelper.removeFromContext( element.getKey() ); } } private void putInContext(TemplateHelper templateHelper, Map context) { - Iterator> iterator = context.entrySet().iterator(); - while ( iterator.hasNext() ) { - Entry element = iterator.next(); - templateHelper.putInContext((String) element.getKey(), element.getValue()); + for ( Entry element : context.entrySet() ) { + templateHelper.putInContext( element.getKey(), element.getValue() ); } } @@ -114,10 +108,10 @@ public void produce(Map additionalContext, String templateName, F fileType = fileType.substring(fileType.indexOf('.')+1); produce(additionalContext, templateName, outputFile, identifier, fileType, null); } - + public void produce(Map additionalContext, String templateName, File outputFile, String identifier, String rootContext) { String fileType = outputFile.getName(); fileType = fileType.substring(fileType.indexOf('.')+1); produce(additionalContext, templateName, outputFile, identifier, fileType, rootContext); - } + } } From 4ff3211ae87b3933258343d8d905e78e57cc4453 Mon Sep 17 00:00:00 2001 From: Koen Aers Date: Fri, 24 Oct 2025 14:06:35 +0200 Subject: [PATCH 3/5] HBX-3180: Improve the implementation of OverrideRepository Signed-off-by: Koen Aers --- .../wrp/OverrideRepositoryWrapperTest.java | 37 ++- .../reveng/strategy/OverrideRepository.java | 288 ++++++++---------- 2 files changed, 152 insertions(+), 173 deletions(-) diff --git a/jbt/src/test/java/org/hibernate/tool/orm/jbt/api/wrp/OverrideRepositoryWrapperTest.java b/jbt/src/test/java/org/hibernate/tool/orm/jbt/api/wrp/OverrideRepositoryWrapperTest.java index f0f0977a43..fecd259da4 100644 --- a/jbt/src/test/java/org/hibernate/tool/orm/jbt/api/wrp/OverrideRepositoryWrapperTest.java +++ b/jbt/src/test/java/org/hibernate/tool/orm/jbt/api/wrp/OverrideRepositoryWrapperTest.java @@ -17,19 +17,16 @@ */ package org.hibernate.tool.orm.jbt.api.wrp; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertSame; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.File; import java.io.FileWriter; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.util.List; import org.hibernate.mapping.Table; import org.hibernate.tool.api.reveng.RevengStrategy; +import org.hibernate.tool.api.reveng.TableIdentifier; +import org.hibernate.tool.internal.export.common.GenericExporter; import org.hibernate.tool.internal.reveng.strategy.DelegatingStrategy; import org.hibernate.tool.internal.reveng.strategy.OverrideRepository; import org.hibernate.tool.internal.reveng.strategy.TableFilter; @@ -39,6 +36,8 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.*; + public class OverrideRepositoryWrapperTest { private static final String HIBERNATE_REVERSE_ENGINEERING_XML = @@ -47,7 +46,7 @@ public class OverrideRepositoryWrapperTest { " '-//Hibernate/Hibernate Reverse Engineering DTD 3.0//EN' "+ " 'http://hibernate.org/dtd/hibernate-reverse-engineering-3.0.dtd'>"+ " "+ - " "+ + "
"+ " "; private OverrideRepositoryWrapper overrideRepositoryWrapper = null; @@ -72,20 +71,18 @@ public void testAddFile() throws Exception { FileWriter fileWriter = new FileWriter(file); fileWriter.write(HIBERNATE_REVERSE_ENGINEERING_XML); fileWriter.close(); - Field tablesField = wrappedOverrideRepository.getClass().getDeclaredField("tables"); - tablesField.setAccessible(true); - Object object = tablesField.get(wrappedOverrideRepository); - List tables = (List)object; - assertNotNull(tables); - assertTrue(tables.isEmpty()); + Field tableToClassNameField = wrappedOverrideRepository.getClass().getDeclaredField("tableToClassName"); + tableToClassNameField.setAccessible(true); + Object object = tableToClassNameField.get(wrappedOverrideRepository); + Method getMethod = object.getClass().getDeclaredMethod("get", TableIdentifier.class); + assertNotNull(getMethod); + getMethod.setAccessible(true); + TableIdentifier ti = TableIdentifier.create(null, null, "FOO"); + Object className = getMethod.invoke(object,ti); + assertNull(className); overrideRepositoryWrapper.addFile(file); - object = tablesField.get(wrappedOverrideRepository); - tables = (List)object; - assertNotNull(tables); - assertFalse(tables.isEmpty()); - Table table = (Table)tables.get(0); - assertNotNull(table); - assertEquals("FOO", table.getName()); + className = getMethod.invoke(object,ti); + assertEquals("TheFoo", className); } @Test diff --git a/orm/src/main/java/org/hibernate/tool/internal/reveng/strategy/OverrideRepository.java b/orm/src/main/java/org/hibernate/tool/internal/reveng/strategy/OverrideRepository.java index 9fa15b3bd5..a1b7356fec 100644 --- a/orm/src/main/java/org/hibernate/tool/internal/reveng/strategy/OverrideRepository.java +++ b/orm/src/main/java/org/hibernate/tool/internal/reveng/strategy/OverrideRepository.java @@ -62,7 +62,6 @@ public class OverrideRepository { final private List tableFilters; - final private List
tables; final private Map> foreignKeys; // key: TableIdentifier element: List of foreignkeys that references the Table final private Map typeForColumn; @@ -108,7 +107,6 @@ public OverrideRepository() { //this.defaultSchema = null; typeMappings = new HashMap>(); tableFilters = new ArrayList(); - tables = new ArrayList
(); foreignKeys = new HashMap>(); typeForColumn = new HashMap(); propertyNameForColumn = new HashMap(); @@ -130,17 +128,15 @@ public OverrideRepository() { foreignKeyToInverseEntityInfo = new HashMap(); } - public OverrideRepository addFile(File xmlFile) { + public void addFile(File xmlFile) { log.info( "Override file: " + xmlFile.getPath() ); try { - addInputStream( new FileInputStream( xmlFile ) ); + addInputStream( xmlFile ); } catch ( Exception e ) { log.error( "Could not configure overrides from file: " + xmlFile.getPath(), e ); throw new MappingException( "Could not configure overrides from file: " + xmlFile.getPath(), e ); } - return this; - } /** @@ -161,23 +157,22 @@ public OverrideRepository addResource(String path) throws MappingException { } } - public OverrideRepository addInputStream(InputStream xmlInputStream) throws MappingException { try { final List errors = new ArrayList(); ErrorHandler errorHandler = new ErrorHandler() { @Override public void warning(SAXParseException exception) throws SAXException { - log.warn("warning while parsing xml", exception); + log.warn("warning while parsing xml", exception); } @Override public void error(SAXParseException exception) throws SAXException { - errors.add(exception); + errors.add(exception); } @Override public void fatalError(SAXParseException exception) throws SAXException { - error(exception); - } + error(exception); + } }; DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); DocumentBuilder db = dbf.newDocumentBuilder(); @@ -194,9 +189,23 @@ public void fatalError(SAXParseException exception) throws SAXException { log.error( "Could not configure overrides from input stream", e ); throw new MappingException( e ); } + } + + public void addInputStream(File file) throws MappingException { + InputStream xmlInputStream = null; + try { + xmlInputStream = new FileInputStream( file ); + addInputStream( xmlInputStream ); + } + catch ( Exception e ) { + log.error( "Could not configure overrides from input stream", e ); + throw new MappingException( e ); + } finally { try { - xmlInputStream.close(); + if (xmlInputStream != null) { + xmlInputStream.close(); + } } catch ( IOException ioe ) { log.error( "could not close input stream", ioe ); @@ -204,9 +213,8 @@ public void fatalError(SAXParseException exception) throws SAXException { } } - private OverrideRepository add(Document doc) { + private void add(Document doc) { OverrideBinder.bindRoot(this, doc); - return this; } private String getPreferredHibernateType(int sqlType, int length, int precision, int scale, boolean nullable) { @@ -220,11 +228,9 @@ private String getPreferredHibernateType(int sqlType, int length, int precision, private String scanForMatch(int sqlType, int length, int precision, int scale, boolean nullable, List l) { if(l!=null) { - Iterator iterator = l.iterator(); - while (iterator.hasNext() ) { - SQLTypeMapping element = iterator.next(); - if(element.getJDBCType()!=sqlType) return null; - if(element.match(sqlType, length, precision, scale, nullable) ) { + for ( SQLTypeMapping element : l ) { + if ( element.getJDBCType() != sqlType ) return null; + if ( element.match( sqlType, length, precision, scale, nullable ) ) { return element.getHibernateType(); } } @@ -232,15 +238,10 @@ private String scanForMatch(int sqlType, int length, int precision, int scale, b return null; } - public OverrideRepository addTypeMapping(SQLTypeMapping sqltype) { + public void addTypeMapping(SQLTypeMapping sqltype) { TypeMappingKey key = new TypeMappingKey(sqltype); - List list = typeMappings.get(key); - if(list==null) { - list = new ArrayList(); - typeMappings.put(key, list); - } + List list = typeMappings.computeIfAbsent( key, k -> new ArrayList<>() ); list.add(sqltype); - return this; } static class TypeMappingKey { @@ -260,8 +261,7 @@ public TypeMappingKey(int sqlType, int length) { public boolean equals(Object obj) { if(obj==null) return false; - if(!(obj instanceof TypeMappingKey)) return false; - TypeMappingKey other = (TypeMappingKey) obj; + if(!(obj instanceof TypeMappingKey other)) return false; return type==other.type && length==other.length; @@ -277,11 +277,9 @@ public String toString() { } protected String getPackageName(TableIdentifier identifier) { - Iterator iterator = tableFilters.iterator(); - while(iterator.hasNext() ) { - TableFilter tf = iterator.next(); - String value = tf.getPackage(identifier); - if(value!=null) { + for ( TableFilter tf : tableFilters ) { + String value = tf.getPackage( identifier ); + if ( value != null ) { return value; } } @@ -296,19 +294,14 @@ protected boolean excludeTable(TableIdentifier identifier) { TableFilter tf = iterator.next(); Boolean value = tf.exclude(identifier); if(value!=null) { - return value.booleanValue(); + return value; } - if(!tf.getExclude().booleanValue()) { + if(!tf.getExclude() ) { hasInclude = true; } } - // can probably be simplified - but like this to be very explicit ;) - if(hasInclude) { - return true; // exclude all by default when at least one include specified - } else { - return false; // if nothing specified or just excludes we include everything - } + return hasInclude; } public void addTableFilter(TableFilter filter) { @@ -382,16 +375,16 @@ public String tableToClassName(TableIdentifier tableIdentifier) { String className = tableToClassName.get(tableIdentifier); if(className!=null) { - if(className.indexOf( "." )>=0) { - return className; - } else { - String packageName = getPackageName(tableIdentifier); - if(packageName==null) { - return className; - } else { - return StringHelper.qualify(packageName, className); - } - } + if( className.contains( "." ) ) { + return className; + } else { + String packageName = getPackageName(tableIdentifier); + if(packageName==null) { + return className; + } else { + return StringHelper.qualify(packageName, className); + } + } } String packageName = getPackageName(tableIdentifier); @@ -471,9 +464,9 @@ public String foreignKeyToEntityName(String keyname, TableIdentifier fromTable, public String foreignKeyToInverseEntityName(String keyname, - TableIdentifier fromTable, List fromColumnNames, - TableIdentifier referencedTable, - List referencedColumnNames, boolean uniqueReference) { + TableIdentifier fromTable, List fromColumnNames, + TableIdentifier referencedTable, + List referencedColumnNames, boolean uniqueReference) { String property = foreignKeyToInverseName.get(keyname); if(property==null) { @@ -493,14 +486,14 @@ public String foreignKeyToCollectionName(String keyname, TableIdentifier fromTab } public boolean excludeForeignKeyAsCollection( - String keyname, - TableIdentifier fromTable, - List fromColumns, - TableIdentifier referencedTable, + String keyname, + TableIdentifier fromTable, + List fromColumns, + TableIdentifier referencedTable, List referencedColumns) { Boolean bool = foreignKeyInverseExclude.get(keyname); if(bool!=null) { - return bool.booleanValue(); + return bool; } else { return super.excludeForeignKeyAsCollection( keyname, fromTable, fromColumns, referencedTable, referencedColumns ); @@ -508,14 +501,14 @@ public boolean excludeForeignKeyAsCollection( } public boolean excludeForeignKeyAsManytoOne( - String keyname, - TableIdentifier fromTable, - List fromColumns, TableIdentifier - referencedTable, + String keyname, + TableIdentifier fromTable, + List fromColumns, TableIdentifier + referencedTable, List referencedColumns) { Boolean bool = (Boolean) foreignKeyToOneExclude.get(keyname); if(bool!=null) { - return bool.booleanValue(); + return bool; } else { return super.excludeForeignKeyAsManytoOne( keyname, fromTable, fromColumns, referencedTable, referencedColumns ); @@ -581,17 +574,15 @@ protected Map tableToMetaAttributes(TableIdentifier identi } private MultiValuedMap findGeneralAttributes(TableIdentifier identifier) { - Iterator iterator = tableFilters.iterator(); - while(iterator.hasNext() ) { - TableFilter tf = iterator.next(); - MultiValuedMap value = tf.getMetaAttributes(identifier); - if(value!=null) { + for ( TableFilter tf : tableFilters ) { + MultiValuedMap value = tf.getMetaAttributes( identifier ); + if ( value != null ) { return value; } } return null; } - + private Map toMetaAttributes(MultiValuedMap mvm) { Map result = new HashMap(); for (MapIterator iter = mvm.mapIterator(); iter.hasNext();) { @@ -600,13 +591,13 @@ private Map toMetaAttributes(MultiValuedMap existing = foreignKeys.get(identifier); - if(existing==null) { - existing = new ArrayList(); - foreignKeys.put(identifier, existing); - } + List existing = foreignKeys.computeIfAbsent( identifier, k -> new ArrayList<>() ); existing.add( fk ); } if(StringHelper.isNotEmpty(wantedClassName)) { - TableIdentifier tableIdentifier = TableIdentifier.create(table); - String className = wantedClassName; + TableIdentifier tableIdentifier = TableIdentifier.create(table); + String className = wantedClassName; /* If wantedClassName specifies a package, it is given by
config so do no more. */ - if(!wantedClassName.contains(".")) { - /* Now look for the package name specified by + if(!wantedClassName.contains(".")) { + /* Now look for the package name specified by config. */ - String packageName = getPackageName(tableIdentifier); - if (packageName != null && !packageName.isBlank()) { - className = packageName + "." + wantedClassName; - } - } + String packageName = getPackageName(tableIdentifier); + if (packageName != null && !packageName.isBlank()) { + className = packageName + "." + wantedClassName; + } + } tableToClassName.put(tableIdentifier, className); } - tables.add(table); - } + } static class TableColumnKey { - private TableIdentifier query; - private String name; - + private final TableIdentifier query; + private final String name; + TableColumnKey(TableIdentifier query, String name){ this.query = query; this.name = name; @@ -670,13 +656,10 @@ public boolean equals(Object obj) { } else if (!name.equals(other.name)) return false; if (query == null) { - if (other.query != null) - return false; - } else if (!query.equals(other.query)) - return false; - return true; + return other.query == null; + } else return query.equals( other.query ); } - + } public void setTypeNameForColumn(TableIdentifier identifier, String columnName, String type) { @@ -763,8 +746,8 @@ public void addMetaAttributeInfo(Table table, MultiValuedMap map) { if(map!=null && !map.isEmpty()) { columnMetaAttributes.put(new TableColumnKey( tableIdentifier, name ), map); @@ -778,54 +761,53 @@ calls nullifyDefaultCatalogAndSchema(table) before doing this TableToClassName l are not null. */ - private class TableToClassName { - Map map = new HashMap(); - - private String get(TableIdentifier tableIdentifier) { - TableMapper mapper = map.get(tableIdentifier.getName()); - if (mapper != null) { - if (mapper.catalog == null || tableIdentifier.getCatalog() == null || - mapper.catalog.equals(tableIdentifier.getCatalog())){ - if (mapper.schema == null || tableIdentifier.getSchema() == null || - mapper.schema.equals(tableIdentifier.getSchema())){ - if (mapper.packageName.length() == 0) { - return mapper.className; - } else { - return mapper.packageName + "." + mapper.className; - } - } - } - } - return null; - } - - private void put(TableIdentifier tableIdentifier, String wantedClassName) { - TableMapper tableMapper = new TableMapper( - tableIdentifier.getCatalog(), - tableIdentifier.getSchema(), - tableIdentifier.getName(), - wantedClassName); - map.put(tableIdentifier.getName(), tableMapper); - } - } - - private class TableMapper { - String catalog; - String schema; - String className; - String packageName; - - private TableMapper(String catalog, String schema, String name, String wantedClassName) { - this.catalog = catalog; - this.schema = schema; - if (wantedClassName.contains(".")) { - int nameStartPos = wantedClassName.lastIndexOf("."); - this.className = wantedClassName.substring(nameStartPos+1); - this.packageName = wantedClassName.substring(0, nameStartPos); - } else { - this.className = wantedClassName; - this.packageName = ""; - } - } - } + private static class TableToClassName { + Map map = new HashMap(); + + private String get(TableIdentifier tableIdentifier) { + TableMapper mapper = map.get(tableIdentifier.getName()); + if (mapper != null) { + if (mapper.catalog == null || tableIdentifier.getCatalog() == null || + mapper.catalog.equals(tableIdentifier.getCatalog())){ + if (mapper.schema == null || tableIdentifier.getSchema() == null || + mapper.schema.equals(tableIdentifier.getSchema())){ + if ( mapper.packageName.isEmpty() ) { + return mapper.className; + } else { + return mapper.packageName + "." + mapper.className; + } + } + } + } + return null; + } + + private void put(TableIdentifier tableIdentifier, String wantedClassName) { + TableMapper tableMapper = new TableMapper( + tableIdentifier.getCatalog(), + tableIdentifier.getSchema(), + wantedClassName ); + map.put(tableIdentifier.getName(), tableMapper); + } + } + + private static class TableMapper { + String catalog; + String schema; + String className; + String packageName; + + private TableMapper(String catalog, String schema, String wantedClassName) { + this.catalog = catalog; + this.schema = schema; + if (wantedClassName.contains(".")) { + int nameStartPos = wantedClassName.lastIndexOf("."); + this.className = wantedClassName.substring(nameStartPos+1); + this.packageName = wantedClassName.substring(0, nameStartPos); + } else { + this.className = wantedClassName; + this.packageName = ""; + } + } + } } From dba7438caabb8363cc883eff11b84adb5bf032f1 Mon Sep 17 00:00:00 2001 From: Koen Aers Date: Fri, 24 Oct 2025 14:37:54 +0200 Subject: [PATCH 4/5] HBX-3182: Improve the implementation of DefaultJavaPrettyPrinterStrategy Signed-off-by: Koen Aers --- .../hibernate/tool/ant/JavaFormatterTask.java | 109 +++++++----------- .../DefaultJavaPrettyPrinterStrategy.java | 6 +- .../tool/ant/JavaFormatter/TestCase.java | 36 ++---- .../tool/ant/JavaFormatter/build.xml | 31 ----- 4 files changed, 48 insertions(+), 134 deletions(-) diff --git a/ant/src/main/java/org/hibernate/tool/ant/JavaFormatterTask.java b/ant/src/main/java/org/hibernate/tool/ant/JavaFormatterTask.java index 9e2ce49b88..ba055b3351 100644 --- a/ant/src/main/java/org/hibernate/tool/ant/JavaFormatterTask.java +++ b/ant/src/main/java/org/hibernate/tool/ant/JavaFormatterTask.java @@ -22,10 +22,8 @@ import java.io.FileInputStream; import java.io.IOException; import java.util.ArrayList; -import java.util.Iterator; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.Properties; import org.apache.tools.ant.BuildException; @@ -37,77 +35,49 @@ public class JavaFormatterTask extends Task { - private List fileSets = new ArrayList(); + private final List fileSets = new ArrayList(); private boolean failOnError; - private File configurationFile; - + public void addConfiguredFileSet(FileSet fileSet) { fileSets.add(fileSet); } - public void setConfigurationFile(File configurationFile) { - this.configurationFile = configurationFile; - } - private Properties readConfig(File cfgfile) throws IOException { - BufferedInputStream stream = null; - try { - stream = new BufferedInputStream(new FileInputStream(cfgfile)); - final Properties settings = new Properties(); - settings.load(stream); - return settings; - } catch (IOException e) { - throw e; - } finally { - if (stream != null) { - try { - stream.close(); - } catch (IOException e) { - - } - } - } - } + try (BufferedInputStream stream = new BufferedInputStream(new FileInputStream(cfgfile))) { + final Properties settings = new Properties(); + settings.load(stream); + return settings; + } + } public void execute() throws BuildException { - Map settings = null; - if(configurationFile!=null) { - try { - settings = readConfig( configurationFile ); - } - catch (IOException e) { - throw new BuildException("Could not read configurationfile " + configurationFile, e); - } - } - File[] files = getFiles(); int failed = 0; if(files.length>0) { - DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(settings); - for (int i = 0; i < files.length; i++) { - File file = files[i]; - try { - boolean ok = formatter.formatFile( file ); - if(!ok) { - failed++; - getProject().log(this, "Formatting failed - skipping " + file, Project.MSG_WARN); - } else { - getProject().log(this, "Formatted " + file, Project.MSG_VERBOSE); - } - } catch(RuntimeException ee) { - failed++; - if(failOnError) { - throw new BuildException("Java formatting failed on " + file, ee); - } else { - getProject().log(this, "Java formatting failed on " + file + ", " + ee.getLocalizedMessage(), Project.MSG_ERR); - } - } - } + DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(); + for (File file : files) { + try { + boolean ok = formatter.formatFile(file); + if (!ok) { + failed++; + getProject().log(this, "Formatting failed - skipping " + file, Project.MSG_WARN); + } else { + getProject().log(this, "Formatted " + file, Project.MSG_VERBOSE); + } + } catch (RuntimeException ee) { + failed++; + if (failOnError) { + throw new BuildException("Java formatting failed on " + file, ee); + } else { + getProject().log(this, "Java formatting failed on " + file + ", " + ee.getLocalizedMessage(), Project.MSG_ERR); + } + } + } } getProject().log( this, "Java formatting of " + files.length + " files completed. Skipped " + failed + " file(s).", Project.MSG_INFO ); @@ -117,23 +87,22 @@ public void execute() throws BuildException { private File[] getFiles() { List files = new LinkedList(); - for ( Iterator i = fileSets.iterator(); i.hasNext(); ) { + for (FileSet fs : fileSets) { - FileSet fs = i.next(); - DirectoryScanner ds = fs.getDirectoryScanner( getProject() ); + DirectoryScanner ds = fs.getDirectoryScanner(getProject()); - String[] dsFiles = ds.getIncludedFiles(); - for (int j = 0; j < dsFiles.length; j++) { - File f = new File(dsFiles[j]); - if ( !f.isFile() ) { - f = new File( ds.getBasedir(), dsFiles[j] ); - } + String[] dsFiles = ds.getIncludedFiles(); + for (String dsFile : dsFiles) { + File f = new File(dsFile); + if (!f.isFile()) { + f = new File(ds.getBasedir(), dsFile); + } - files.add( f ); - } - } + files.add(f); + } + } - return (File[]) files.toArray(new File[files.size()]); + return (File[]) files.toArray(new File[0]); } } diff --git a/orm/src/main/java/org/hibernate/tool/api/java/DefaultJavaPrettyPrinterStrategy.java b/orm/src/main/java/org/hibernate/tool/api/java/DefaultJavaPrettyPrinterStrategy.java index a3bfd0b89a..91f7d54e64 100644 --- a/orm/src/main/java/org/hibernate/tool/api/java/DefaultJavaPrettyPrinterStrategy.java +++ b/orm/src/main/java/org/hibernate/tool/api/java/DefaultJavaPrettyPrinterStrategy.java @@ -26,8 +26,6 @@ import com.google.googlejavaformat.java.FormatterException; public class DefaultJavaPrettyPrinterStrategy { - - public DefaultJavaPrettyPrinterStrategy(Map settings) {} public boolean formatFile(File file) { try { @@ -40,5 +38,5 @@ public boolean formatFile(File file) { throw new RuntimeException(e); } } - -} + +} \ No newline at end of file diff --git a/test/nodb/src/test/java/org/hibernate/tool/ant/JavaFormatter/TestCase.java b/test/nodb/src/test/java/org/hibernate/tool/ant/JavaFormatter/TestCase.java index a15db3d860..e8fe8804db 100644 --- a/test/nodb/src/test/java/org/hibernate/tool/ant/JavaFormatter/TestCase.java +++ b/test/nodb/src/test/java/org/hibernate/tool/ant/JavaFormatter/TestCase.java @@ -71,7 +71,7 @@ public void testJavaFormatFile() { .findFirstString("public", simpleOne) .contains("SimpleOne")); - DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(null); + DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(); formatter.formatFile( simpleOne ); assertTrue(FileUtil @@ -108,7 +108,7 @@ public void testJavaJdk5FormatFile() { assertFalse(log.contains("Exception")); - DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(null); + DefaultJavaPrettyPrinterStrategy formatter = new DefaultJavaPrettyPrinterStrategy(); assertTrue( formatter.formatFile(simple5One), "formatting should pass when using default settings"); @@ -170,10 +170,10 @@ public void testConfig() { String log = AntUtil.getLog(project); assertFalse(log.contains("Exception")); - + assertTrue(simpleOne.exists()); assertTrue(simple5One.exists()); - + assertFalse(FileUtil .findFirstString("public", simpleOne) .contains("SimpleOne")); @@ -181,42 +181,20 @@ public void testConfig() { .findFirstString("public", simple5One) .contains("Simple5One")); - project.executeTarget("configtest"); + project.executeTarget("formatfiles"); log = AntUtil.getLog(project); assertFalse(log.contains("Exception")); - + assertTrue(FileUtil .findFirstString("public", simpleOne) .contains("SimpleOne")); assertTrue(FileUtil .findFirstString("public", simple5One) .contains("Simple5One")); - + assertTrue(simpleOne.delete()); assertTrue(simple5One.delete()); - - project.executeTarget("copyfiles"); - log = AntUtil.getLog(project); - assertFalse(log.contains("Exception")); - assertFalse(FileUtil - .findFirstString("public", simpleOne) - .contains("SimpleOne")); - assertFalse(FileUtil - .findFirstString("public", simple5One) - .contains("Simple5One")); - - project.executeTarget("noconfigtest"); - log = AntUtil.getLog(project); - assertFalse(log.contains("Exception")); - - assertTrue(FileUtil - .findFirstString("public", simpleOne) - .contains("SimpleOne")); - assertTrue(FileUtil - .findFirstString("public", simple5One) - .contains("Simple5One")); - } } diff --git a/test/nodb/src/test/resources/org/hibernate/tool/ant/JavaFormatter/build.xml b/test/nodb/src/test/resources/org/hibernate/tool/ant/JavaFormatter/build.xml index 363309925b..9f6b5299f6 100644 --- a/test/nodb/src/test/resources/org/hibernate/tool/ant/JavaFormatter/build.xml +++ b/test/nodb/src/test/resources/org/hibernate/tool/ant/JavaFormatter/build.xml @@ -55,36 +55,5 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - From 8b976790bee44dfe7fa9407612e008449bcdf918 Mon Sep 17 00:00:00 2001 From: Koen Aers Date: Fri, 24 Oct 2025 14:39:50 +0200 Subject: [PATCH 5/5] HBX-3183: Improve the implementation of AbstractMetaDataDialect Signed-off-by: Koen Aers --- .../dialect/AbstractMetaDataDialect.java | 149 +++++++++--------- 1 file changed, 76 insertions(+), 73 deletions(-) diff --git a/orm/src/main/java/org/hibernate/tool/internal/reveng/dialect/AbstractMetaDataDialect.java b/orm/src/main/java/org/hibernate/tool/internal/reveng/dialect/AbstractMetaDataDialect.java index 3065a5c732..dd4e1eadae 100644 --- a/orm/src/main/java/org/hibernate/tool/internal/reveng/dialect/AbstractMetaDataDialect.java +++ b/orm/src/main/java/org/hibernate/tool/internal/reveng/dialect/AbstractMetaDataDialect.java @@ -42,22 +42,22 @@ public abstract class AbstractMetaDataDialect implements RevengDialect { protected final Logger log = Logger.getLogger(this.getClass()); - + private Connection connection; private DatabaseMetaData metaData; - + private ConnectionProvider connectionProvider = null; public void configure( ConnectionProvider connectionProvider) { - this.connectionProvider = connectionProvider; + this.connectionProvider = connectionProvider; } - + public void close() { metaData = null; if(connection != null) { try { - connectionProvider.closeConnection(connection); + connectionProvider.closeConnection(connection); } catch (SQLException e) { throw new RuntimeException("Problem while closing connection", e); @@ -65,70 +65,74 @@ public void close() { connection = null; } } - connectionProvider = null; + connectionProvider = null; } - + protected DatabaseMetaData getMetaData() { if (metaData == null) { try { - metaData = getConnection().getMetaData(); - } + metaData = getConnection().getMetaData(); + } catch (SQLException e) { throw new RuntimeException("Getting database metadata", e); } } return metaData; } - + protected String getDatabaseStructure(String catalog, String schema) { - ResultSet schemaRs = null; - ResultSet catalogRs = null; - String nl = System.getProperty("line.separator"); - StringBuffer sb = new StringBuffer(nl); - // Let's give the user some feedback. The exception - // is probably related to incorrect schema configuration. - sb.append("Configured schema:").append(schema).append(nl); - sb.append("Configured catalog:").append(catalog ).append(nl); - - try { - schemaRs = getMetaData().getSchemas(); - sb.append("Available schemas:").append(nl); - while (schemaRs.next() ) { - sb.append(" ").append(schemaRs.getString("TABLE_SCHEM") ).append(nl); - } - } - catch (SQLException e2) { - log.warn("Could not get schemas", e2); - sb.append(" ").append(nl); - } - finally { - try { - schemaRs.close(); - } - catch (Exception ignore) { - } - } - - try { - catalogRs = getMetaData().getCatalogs(); - sb.append("Available catalogs:").append(nl); - while (catalogRs.next() ) { - sb.append(" ").append(catalogRs.getString("TABLE_CAT") ).append(nl); - } - } - catch (SQLException e2) { - log.warn("Could not get catalogs", e2); - sb.append(" ").append(nl); - } - finally { - try { - catalogRs.close(); - } - catch (Exception ignore) { - } - } - return sb.toString(); - } + ResultSet schemaRs = null; + ResultSet catalogRs = null; + String nl = System.lineSeparator(); + StringBuilder sb = new StringBuilder(nl); + // Let's give the user some feedback. The exception + // is probably related to incorrect schema configuration. + sb.append("Configured schema:").append(schema).append(nl); + sb.append("Configured catalog:").append(catalog ).append(nl); + + try { + schemaRs = getMetaData().getSchemas(); + sb.append("Available schemas:").append(nl); + while (schemaRs.next() ) { + sb.append(" ").append(schemaRs.getString("TABLE_SCHEM") ).append(nl); + } + } + catch (SQLException e2) { + log.warn("Could not get schemas", e2); + sb.append(" ").append(nl); + } + finally { + try { + if (schemaRs != null) { + schemaRs.close(); + } + } + catch (Exception ignore) { + } + } + + try { + catalogRs = getMetaData().getCatalogs(); + sb.append("Available catalogs:").append(nl); + while (catalogRs.next() ) { + sb.append(" ").append(catalogRs.getString("TABLE_CAT") ).append(nl); + } + } + catch (SQLException e2) { + log.warn("Could not get catalogs", e2); + sb.append(" ").append(nl); + } + finally { + try { + if (catalogRs != null) { + catalogRs.close(); + } + } + catch (Exception ignore) { + } + } + return sb.toString(); + } protected Connection getConnection() throws SQLException { if(connection==null) { @@ -136,31 +140,30 @@ protected Connection getConnection() throws SQLException { } return connection; } - + public void close(Iterator iterator) { if(iterator instanceof ResultSetIterator) { ((ResultSetIterator)iterator).close(); } } - + public boolean needQuote(String name) { - + if(name==null) return false; - - // TODO: use jdbc metadata to decide on this. but for now we just handle the most typical cases. + + // TODO: use jdbc metadata to decide on this. but for now we just handle the most typical cases. if(name.indexOf('-')>0) return true; if(name.indexOf(' ')>0) return true; - if(name.indexOf('.')>0) return true; - return false; + return name.indexOf( '.' ) > 0; } - + protected String caseForSearch(String value) throws SQLException { // TODO: handle quoted requests (just strip it ?) if(needQuote(value)) { if ( getMetaData().storesMixedCaseQuotedIdentifiers() ) { return value; - } else if ( getMetaData().storesUpperCaseQuotedIdentifiers() ) { - return toUpperCase( value ); + } else if ( getMetaData().storesUpperCaseQuotedIdentifiers() ) { + return toUpperCase( value ); } else if( getMetaData().storesLowerCaseQuotedIdentifiers() ) { return toLowerCase( value ); } else { @@ -168,23 +171,23 @@ protected String caseForSearch(String value) throws SQLException { } } else if ( getMetaData().storesMixedCaseQuotedIdentifiers() ) { return value; - } else if ( getMetaData().storesUpperCaseIdentifiers() ) { - return toUpperCase( value ); + } else if ( getMetaData().storesUpperCaseIdentifiers() ) { + return toUpperCase( value ); } else if( getMetaData().storesLowerCaseIdentifiers() ) { return toLowerCase( value ); } else { return value; - } + } } private String toUpperCase(String str) { return str==null ? null : str.toUpperCase(); } - + private String toLowerCase(String str) { return str == null ? null : str.toLowerCase(Locale.ENGLISH); } - + public Iterator> getSuggestedPrimaryKeyStrategyName(String catalog, String schema, String table) { Map m = new HashMap(); m.put( "TABLE_CAT", catalog );