Skip to content

Commit

Permalink
Merge pull request #1759 from schemacrawler/issue1750
Browse files Browse the repository at this point in the history
Fix #1750 - avoid storing lints in catalog
  • Loading branch information
sualeh authored Nov 24, 2024
2 parents 2e3ac61 + eeade31 commit 2bf6e86
Show file tree
Hide file tree
Showing 39 changed files with 1,555 additions and 723 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,16 @@

package schemacrawler.crawl;

import static java.util.Objects.requireNonNull;
import static schemacrawler.crawl.RetrieverUtility.lookupOrCreateColumn;
import static us.fatehi.utility.Utility.isBlank;
import static us.fatehi.utility.Utility.requireNotBlank;

import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;

import static java.util.Objects.requireNonNull;
import static us.fatehi.utility.Utility.isBlank;
import static us.fatehi.utility.Utility.requireNotBlank;
import schemacrawler.schema.Catalog;
import schemacrawler.schema.Column;
import schemacrawler.schema.ColumnReference;
Expand All @@ -49,9 +47,10 @@
import schemacrawler.schema.Table;
import schemacrawler.schema.TableReference;
import schemacrawler.schema.WeakAssociation;
import us.fatehi.utility.Builder;
import us.fatehi.utility.string.StringFormat;

public final class WeakAssociationBuilder {
public final class WeakAssociationBuilder implements Builder<TableReference> {

public static final class WeakAssociationColumn {

Expand All @@ -68,8 +67,8 @@ public WeakAssociationColumn(final Column column) {

public WeakAssociationColumn(final Schema schema, final String table, final String column) {
this.schema = requireNonNull(schema, "No schema provided");
this.tableName = requireNotBlank(table, "No table name provided");
this.columnName = requireNotBlank(column, "No column name provided");
tableName = requireNotBlank(table, "No table name provided");
columnName = requireNotBlank(column, "No column name provided");
}

public String getColumnName() {
Expand Down Expand Up @@ -146,8 +145,13 @@ public WeakAssociationBuilder addColumnReference(
return this;
}

public void build() {
findOrCreate(null);
@Override
public TableReference build() {
final Optional<TableReference> optionalTableReference = findOrCreate(null);
if (optionalTableReference.isPresent()) {
return optionalTableReference.get();
}
return null;
}

public WeakAssociationBuilder clear() {
Expand Down Expand Up @@ -205,18 +209,16 @@ public Optional<TableReference> findOrCreate(final String name) {
lookupMatchingWeakAssociation(weakAssociation);
if (optionalMatchingWeakAssociation.isPresent()) {
return Optional.of(optionalMatchingWeakAssociation.get());
} else {

// Add weak association to tables if no matching foreign key is found
if (referencedTable instanceof MutableTable) {
((MutableTable) referencedTable).addWeakAssociation(weakAssociation);
}
if (dependentTable instanceof MutableTable) {
((MutableTable) dependentTable).addWeakAssociation(weakAssociation);
}

return Optional.of(weakAssociation);
}
// Add weak association to tables if no matching foreign key is found
if (referencedTable instanceof MutableTable) {
((MutableTable) referencedTable).addWeakAssociation(weakAssociation);
}
if (dependentTable instanceof MutableTable) {
((MutableTable) dependentTable).addWeakAssociation(weakAssociation);
}

return Optional.of(weakAssociation);
}

private Optional<ForeignKey> lookupMatchingForeignKey(final WeakAssociation weakAssociation) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@

package schemacrawler.schemacrawler;

import us.fatehi.utility.Builder;

/**
* Convoluted interface to allow for subclasses builders, while maintaining a fluent interface.
*
Expand All @@ -37,9 +39,15 @@
* "https://stackoverflow.com/questions/17164375/subclassing-a-java-builder-class">Subclassing a
* Java Builder class</a>
*/
public interface OptionsBuilder<B extends OptionsBuilder<B, O>, O extends Options> {
public interface OptionsBuilder<B extends OptionsBuilder<B, O>, O extends Options>
extends Builder<O> {

OptionsBuilder<B, O> fromOptions(O options);

O toOptions();

@Override
default O build() {
return toOptions();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,20 @@ Database [database]
------------------------------------------------------------------------

[lint, medium]
column with same name but different data types STATE [CHARACTER, VARCHAR]
column with same name but different data types DATA [CLOB, PUBLIC.BOOKS.VALID_STRING]
column with same name but different data types FIRSTNAME [VARCHAR, PUBLIC.BOOKS.NAME_TYPE]
column with same name but different data types LASTNAME [VARCHAR, PUBLIC.BOOKS.NAME_TYPE]
column with same name but different data types DATA [CLOB, PUBLIC.BOOKS.VALID_STRING]
column with same name but different data types STATE [CHARACTER, VARCHAR]



PUBLIC.BOOKS.AUTHORS [table]
------------------------------------------------------------------------

[lint, medium]
incrementing columns PUBLIC.BOOKS.AUTHORS.ADDRESS1, PUBLIC.BOOKS.AUTHORS.ADDRESS2
incrementing columns PUBLIC.BOOKS.AUTHORS.ADDRESS1, PUBLIC.BOOKS.AUTHORS.ADDRESS2
[lint, low]
should have remarks ID, FIRSTNAME, LASTNAME, ADDRESS1, ADDRESS2, CITY, STATE, POSTALCODE, COUNTRY
should have remarks ID, FIRSTNAME, LASTNAME, ADDRESS1, ADDRESS2, CITY, STATE, POSTALCODE, COUNTRY



Expand All @@ -39,7 +37,7 @@ PUBLIC.BOOKS.BOOKAUTHORS [table]
------------------------------------------------------------------------

[lint, high]
redundant index PUBLIC.BOOKS.BOOKAUTHORS.SYS_FK_10120
redundant index PUBLIC.BOOKS.BOOKAUTHORS.SYS_FK_10118
no primary key
primary key may not be a surrogate
[lint, low]
Expand Down Expand Up @@ -77,8 +75,8 @@ PUBLIC.BOOKS."Celebrity Updates" [table]
------------------------------------------------------------------------

[lint, high]
redundant index PUBLIC.BOOKS."Celebrity Updates".SYS_FK_10130
redundant index PUBLIC.BOOKS."Celebrity Updates"."PK Celebrity Updates"
redundant index PUBLIC.BOOKS."Celebrity Updates".SYS_FK_10128
[lint, medium]
no non-nullable data columns
spaces in name, or reserved word
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@
import schemacrawler.tools.lint.LintReport;
import schemacrawler.tools.lint.Linters;
import schemacrawler.tools.lint.config.LinterConfigs;
import schemacrawler.tools.lint.formatter.LintReportBuilder;
import schemacrawler.tools.lint.formatter.LintReportJsonBuilder;
import schemacrawler.tools.lint.formatter.LintReportGenerator;
import schemacrawler.tools.lint.formatter.LintReportJsonGenerator;
import schemacrawler.tools.lint.formatter.LintReportTextFormatter;
import schemacrawler.tools.lint.formatter.LintReportYamlBuilder;
import schemacrawler.tools.lint.formatter.LintReportYamlGenerator;
import us.fatehi.utility.string.ObjectToStringFormat;
import us.fatehi.utility.string.StringFormat;

Expand Down Expand Up @@ -72,9 +72,8 @@ public void execute() {
linters.lint(catalog, connection);

// Produce the lint report
final LintReport lintReport =
new LintReport(
outputOptions.getTitle(), catalog.getCrawlInfo(), linters.getCollector().getLints());
final LintReport lintReport = linters.getLintReport();
lintReport.setTitle(outputOptions.getTitle());

// Write out the lint report
LOGGER.log(Level.INFO, "Generating lint report");
Expand Down Expand Up @@ -111,17 +110,17 @@ private void dispatch(final Linters linters) {
lintDispatch.dispatch();
}

private LintReportBuilder getLintReportBuilder() {
private LintReportGenerator getLintReportBuilder() {
final LintReportOutputFormat outputFormat =
LintReportOutputFormat.fromFormat(outputOptions.getOutputFormatValue());

final LintReportBuilder lintReportBuilder;
final LintReportGenerator lintReportBuilder;
switch (outputFormat) {
case json:
lintReportBuilder = new LintReportJsonBuilder(outputOptions);
lintReportBuilder = new LintReportJsonGenerator(outputOptions);
break;
case yaml:
lintReportBuilder = new LintReportYamlBuilder(outputOptions);
lintReportBuilder = new LintReportYamlGenerator(outputOptions);
break;
default:
lintReportBuilder =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@

package schemacrawler.tools.command.lint.options;

import static us.fatehi.utility.Utility.isBlank;
import static us.fatehi.utility.Utility.trimToEmpty;

import schemacrawler.tools.lint.LintDispatch;
import schemacrawler.tools.options.Config;
import schemacrawler.tools.text.options.BaseTextOptionsBuilder;
Expand All @@ -50,10 +48,6 @@ public static LintOptionsBuilder builder() {
return new LintOptionsBuilder();
}

public static LintOptionsBuilder builder(final LintOptions options) {
return new LintOptionsBuilder().fromOptions(options);
}

LintDispatch lintDispatch;
String linterConfigs;
boolean runAllLinters;
Expand All @@ -63,7 +57,7 @@ private LintOptionsBuilder() {
linterConfigs = "";
lintDispatch = LintDispatch.none;
runAllLinters = true;
config = new Config();
config = toConfig();
}

@Override
Expand Down Expand Up @@ -160,12 +154,4 @@ public LintOptionsBuilder withLinterConfigs(final String linterConfigs) {
this.linterConfigs = trimToEmpty(linterConfigs);
return this;
}

/** With property. */
public LintOptionsBuilder withProperty(final String name, final String value) {
if (!isBlank(name)) {
this.config.put(name, value);
}
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
import schemacrawler.inclusionrule.InclusionRule;
import schemacrawler.schema.Catalog;
import schemacrawler.schema.Column;
import schemacrawler.schema.CrawlInfo;
import schemacrawler.schema.Table;
import schemacrawler.tools.lint.config.LinterConfig;
import us.fatehi.utility.property.PropertyName;
Expand All @@ -69,6 +68,33 @@ protected BaseLinter(final PropertyName linterName, final LintCollector lintColl
columnInclusionRule = new IncludeAll();
}

@Override
public final void configure(final LinterConfig linterConfig) {
super.configure(linterConfig);
if (linterConfig != null) {
tableInclusionRule = linterConfig.getTableInclusionRule();
columnInclusionRule = linterConfig.getColumnInclusionRule();
}
}

@Override
public final void lint(final Catalog catalog, final Connection connection) {
this.catalog = requireNonNull(catalog, "No catalog provided");

start(connection);
for (final Table table : catalog.getTables()) {
if (includeTable(table)) {
lint(table, connection);
} else {
LOGGER.log(
Level.FINE,
new StringFormat("Excluding table <%s> for lint <%s>", table, getLinterId()));
}
}
end(connection);
this.catalog = null;
}

protected final void addCatalogLint(final String message) {
addLint(LintObjectType.catalog, catalog, message, null);
}
Expand Down Expand Up @@ -106,22 +132,6 @@ protected final List<Column> getColumns(final Table table) {
return columns;
}

protected final CrawlInfo getCrawlInfo() {
return catalog.getCrawlInfo();
}

protected final TableTypesFilter getTableTypesFilter() {
return tableTypesFilter;
}

protected final boolean includeColumn(final Column column) {
return column != null && columnInclusionRule.test(column.getFullName());
}

protected final boolean includeTable(final Table table) {
return table != null && tableInclusionRule.test(table.getFullName());
}

protected abstract void lint(Table table, Connection connection);

protected final void setTableTypesFilter(final TableTypesFilter tableTypesFilter) {
Expand All @@ -136,30 +146,13 @@ protected void start(final Connection connection) {
// Default implementation - NO-OP
}

@Override
public final void configure(final LinterConfig linterConfig) {
super.configure(linterConfig);
if (linterConfig != null) {
tableInclusionRule = linterConfig.getTableInclusionRule();
columnInclusionRule = linterConfig.getColumnInclusionRule();
}
private final boolean includeColumn(final Column column) {
return column != null && columnInclusionRule.test(column.getFullName());
}

@Override
public final void lint(final Catalog catalog, final Connection connection) {
this.catalog = requireNonNull(catalog, "No catalog provided");

start(connection);
for (final Table table : catalog.getTables()) {
if (tableInclusionRule.test(table.getFullName()) && tableTypesFilter.test(table)) {
lint(table, connection);
} else {
LOGGER.log(
Level.FINE,
new StringFormat("Excluding table <%s> for lint <%s>", table, getLinterId()));
}
}
end(connection);
this.catalog = null;
private final boolean includeTable(final Table table) {
return table != null
&& tableInclusionRule.test(table.getFullName())
&& tableTypesFilter.test(table);
}
}
Loading

0 comments on commit 2bf6e86

Please sign in to comment.