Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for DAT-15716 :: Include 'OR REPLACE' instruction for a view when generate-changelog/diff-changelog command are executed #5304

Merged
merged 17 commits into from
Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Global Options
--always-drop-instead-of-replace=PARAM
If true, drop and recreate a view instead of
replacing it.
DEFAULT: false
(defaults file: 'liquibase.
alwaysDropInsteadOfReplace', environment
variable:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import liquibase.Scope
import liquibase.command.CommandScope
import liquibase.command.core.helpers.DbUrlConnectionArgumentsCommandStep
import liquibase.command.util.CommandUtil
import liquibase.exception.CommandExecutionException
import liquibase.extension.testing.testsystem.DatabaseTestSystem
import liquibase.extension.testing.testsystem.TestSystemFactory
import liquibase.extension.testing.testsystem.spock.LiquibaseIntegrationTest
Expand Down Expand Up @@ -113,5 +114,59 @@ create table str4 (
CommandUtil.runDropAll(mysql)

}

def "Ensure generated changelog SQL format contains 'OR REPLACE' instruction for a view when USE_OR_REPLACE_OPTION is set as true"() {
given:
mysql.executeSql("""create table foo(
id numeric not null primary key,
some_json json null)""")
mysql.executeSql("CREATE VIEW fooView AS Select * from foo;")

when:
runGenerateChangelog(mysql, "output.mysql.sql", true)
def outputFile = new File("output.mysql.sql")
def contents = FileUtil.getContents(outputFile)

then:
contents.contains("CREATE OR REPLACE VIEW fooView")

cleanup:
outputFile.delete()
CommandUtil.runDropAll(mysql)
}

def "Ensure generated changelog SQL format does NOT contain 'OR REPLACE' instruction for a view when USE_OR_REPLACE_OPTION is set as false"() {
given:
mysql.executeSql("""create table foo(
id numeric not null primary key,
some_json json null)""")
mysql.executeSql("CREATE VIEW fooView AS Select * from foo;")

when:
runGenerateChangelog(mysql, "output.mysql.sql", false)
def outputFile = new File("output.mysql.sql")
def contents = FileUtil.getContents(outputFile)

then:
!contents.contains("CREATE OR REPLACE VIEW fooView")
contents.contains("CREATE VIEW fooView")

cleanup:
outputFile.delete()
CommandUtil.runDropAll(mysql)
}

static void runGenerateChangelog(DatabaseTestSystem db, String outputFile, boolean useOrReplaceOption) throws CommandExecutionException {
CommandScope commandScope = new CommandScope(GenerateChangelogCommandStep.COMMAND_NAME)
commandScope.addArgumentValue(DbUrlConnectionArgumentsCommandStep.URL_ARG, db.getConnectionUrl())
commandScope.addArgumentValue(DbUrlConnectionArgumentsCommandStep.USERNAME_ARG, db.getUsername())
commandScope.addArgumentValue(DbUrlConnectionArgumentsCommandStep.PASSWORD_ARG, db.getPassword())
commandScope.addArgumentValue(GenerateChangelogCommandStep.OVERWRITE_OUTPUT_FILE_ARG, true)
commandScope.addArgumentValue(GenerateChangelogCommandStep.CHANGELOG_FILE_ARG, outputFile)
commandScope.addArgumentValue(GenerateChangelogCommandStep.USE_OR_REPLACE_OPTION, useOrReplaceOption)
OutputStream outputStream = new ByteArrayOutputStream()
commandScope.setOutput(outputStream)
commandScope.execute()
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import liquibase.command.util.CommandUtil
import liquibase.database.Database
import liquibase.database.DatabaseFactory
import liquibase.diff.compare.CompareControl
import liquibase.exception.CommandExecutionException
import liquibase.extension.testing.testsystem.DatabaseTestSystem
import liquibase.extension.testing.testsystem.TestSystemFactory
import liquibase.extension.testing.testsystem.spock.LiquibaseIntegrationTest
Expand Down Expand Up @@ -171,4 +172,73 @@ COMMENT ON COLUMN $viewName.$columnName IS '$columnComment';
targetDatabase.close()
CommandUtil.runDropAll(postgres)
}

def "Ensure diff-changelog with SQL output format contains 'OR REPLACE' instruction for a view when USE_OR_REPLACE_OPTION is set as true"() {
given:
def outputChangelogFile = String.format("diffChangelogFile-%s-output.postgresql.sql", StringUtil.randomIdentifer(10))
Database refDatabase =
DatabaseFactory.instance.openDatabase(postgres.getConnectionUrl(), postgres.getUsername(), postgres.getPassword(), null, null)
Database targetDatabase =
DatabaseFactory.instance.openDatabase(postgres.getConnectionUrl().replace("lbcat", "lbcat2"), postgres.getUsername(), postgres.getPassword(), null, null)

postgres.executeSql("""create table foo(
id numeric not null primary key,
some_json json null)""")
postgres.executeSql("CREATE VIEW fooview AS Select * from foo;")

when:
runDiffToChangelogWithUseOrReplaceCommandArgument(targetDatabase, refDatabase, outputChangelogFile, true)
def outputFile = new File(outputChangelogFile)
def contents = FileUtil.getContents(outputFile)

then:
contents.contains("CREATE OR REPLACE VIEW \"fooview\"")

cleanup:
outputFile.delete()
refDatabase.close()
targetDatabase.close()
CommandUtil.runDropAll(postgres)
postgres.getConnection().close()
}

def "Ensure diff-changelog with SQL output format does NOT contain 'OR REPLACE' instruction for a view when USE_OR_REPLACE_OPTION is set as false"() {
given:
def outputChangelogFile = String.format("diffChangelogFile-%s-output.postgresql.sql", StringUtil.randomIdentifer(10))
Database refDatabase =
DatabaseFactory.instance.openDatabase(postgres.getConnectionUrl(), postgres.getUsername(), postgres.getPassword(), null, null)
Database targetDatabase =
DatabaseFactory.instance.openDatabase(postgres.getConnectionUrl().replace("lbcat", "lbcat2"), postgres.getUsername(), postgres.getPassword(), null, null)

postgres.executeSql("""create table foo(
id numeric not null primary key,
some_json json null)""")
postgres.executeSql("CREATE VIEW fooview AS Select * from foo;")

when:
runDiffToChangelogWithUseOrReplaceCommandArgument(targetDatabase, refDatabase, outputChangelogFile, false)
def outputFile = new File(outputChangelogFile)
def contents = FileUtil.getContents(outputFile)

then:
!contents.contains("CREATE OR REPLACE VIEW \"fooview\"")
contents.contains("CREATE VIEW \"fooview\"")

cleanup:
outputFile.delete()
refDatabase.close()
targetDatabase.close()
CommandUtil.runDropAll(postgres)
postgres.getConnection().close()
}

static void runDiffToChangelogWithUseOrReplaceCommandArgument(Database targetDatabase, Database referenceDatabase,
String outputFile, boolean useOrReplaceOption) throws CommandExecutionException {
CommandScope commandScope = new CommandScope(DiffChangelogCommandStep.COMMAND_NAME)
commandScope.addArgumentValue(DbUrlConnectionArgumentsCommandStep.DATABASE_ARG, targetDatabase)
commandScope.addArgumentValue(DiffChangelogCommandStep.CHANGELOG_FILE_ARG, outputFile)
commandScope.addArgumentValue(DiffChangelogCommandStep.USE_OR_REPLACE_OPTION, useOrReplaceOption)
commandScope.addArgumentValue(ReferenceDbUrlConnectionCommandStep.REFERENCE_DATABASE_ARG, referenceDatabase)
commandScope.execute()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ Optional Args:
Default: null
schemas (String) Schemas to include in diff
Default: null
useOrReplaceOption (Boolean) If true, will add 'OR REPLACE' option to the create view change object
Default: false
username (String) Username to use to connect to the database
Default: null
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ Optional Args:
OBFUSCATED
schemas (String) Schemas to include in diff
Default: null
useOrReplaceOption (Boolean) If true, will add 'OR REPLACE' option to the create view change object
Default: false
username (String) Username to use to connect to the database
Default: null
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,14 @@ public class LiquibaseDatabaseDiff extends AbstractLiquibaseChangeLogMojo {
@PropertyElement
protected String format;

/**
* Flag to allow adding 'OR REPLACE' option to the create view change object when generating changelog in SQL format
*
* @parameter property="liquibase.useOrReplaceOption" default-value="false"
*/
@PropertyElement
protected boolean useOrReplaceOption;

@Override
public void execute() throws MojoExecutionException, MojoFailureException {
if (this.referenceServer != null) {
Expand Down Expand Up @@ -262,6 +270,9 @@ protected void performLiquibaseTask(Liquibase liquibase) throws LiquibaseExcepti
try {
DiffOutputControl diffOutputControl = new DiffOutputControl(diffIncludeCatalog, diffIncludeSchema, diffIncludeTablespace, null).addIncludedSchema(new CatalogAndSchema(referenceDefaultCatalogName, referenceDefaultSchemaName));
diffOutputControl.setObjectChangeFilter(objectChangeFilter);
if(useOrReplaceOption) {
diffOutputControl.setReplaceIfExistsSet(true);
}
CommandLineUtils.doDiffToChangeLog(diffChangeLogFile, referenceDatabase, db, changeSetAuthor, diffOutputControl,
objectChangeFilter, StringUtil.trimToNull(diffTypes), schemaComparisons);
if (new File(diffChangeLogFile).exists()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import liquibase.GlobalConfiguration;
import liquibase.Liquibase;
import liquibase.Scope;
import liquibase.command.core.GenerateChangelogCommandStep;
import liquibase.database.Database;
import liquibase.diff.compare.CompareControl;
import liquibase.diff.output.DiffOutputControl;
Expand Down Expand Up @@ -114,6 +115,14 @@ public class LiquibaseGenerateChangeLogMojo extends
@PropertyElement
protected boolean overwriteOutputFile;

/**
* Flag to allow adding 'OR REPLACE' option to the create view change object when generating changelog in SQL format
*
* @parameter property="liquibase.useOrReplaceOption" default-value="false"
*/
@PropertyElement
protected boolean useOrReplaceOption;

@Override
protected void performLiquibaseTask(Liquibase liquibase)
throws LiquibaseException {
Expand Down Expand Up @@ -141,6 +150,9 @@ protected void performLiquibaseTask(Liquibase liquibase)
if (diffIncludeObjects != null) {
diffOutputControl.setObjectChangeFilter(new StandardObjectChangeFilter(StandardObjectChangeFilter.FilterType.INCLUDE, diffIncludeObjects));
}
if(useOrReplaceOption) {
diffOutputControl.setReplaceIfExistsSet(true);
}

//
// Set the global configuration option based on presence of the dataOutputDirectory
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package liquibase;

import liquibase.command.CommandArgumentDefinition;
import liquibase.configuration.AutoloadedConfigurations;
import liquibase.configuration.ConfigurationDefinition;
import liquibase.ui.UIServiceEnum;
Expand Down Expand Up @@ -239,8 +240,9 @@ public class GlobalConfiguration implements AutoloadedConfigurations {
.setDescription("Complete list of Location(s) to search for files such as changelog files in. Multiple paths can be specified by separating them with commas.")
.build();

ALWAYS_DROP_INSTEAD_OF_REPLACE = builder.define("alwaysDropInsteadOfReplace", Boolean.class)
ALWAYS_DROP_INSTEAD_OF_REPLACE = builder.define("alwaysDropInsteadOfReplace", Boolean.class)
.setDescription("If true, drop and recreate a view instead of replacing it.")
.setDefaultValue(false)
.setValueHandler(ValueHandlerUtil::booleanValueHandler)
.build();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package liquibase.change;

/**
* ReplaceIfExists interface will control whether an implementation change object class will set the replaceIfExists flag that basically will tell
* generate-changelog/diffToChangelog commands the given change object needs to generate the SQL for replace the stored logic if it already exists
*/
public interface ReplaceIfExists {

void setReplaceIfExists(Boolean flag);

}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import java.util.Map;

@DatabaseChange(name = "createProcedure", description = "Defines a stored procedure.", priority = ChangeMetaData.PRIORITY_DEFAULT)
public class CreateProcedureChange extends AbstractChange implements DbmsTargetedChange {
public class CreateProcedureChange extends AbstractChange implements DbmsTargetedChange, ReplaceIfExists {
private String comments;
private String catalogName;
private String schemaName;
Expand Down Expand Up @@ -175,6 +175,7 @@ public Boolean getReplaceIfExists() {
return replaceIfExists;
}

@Override
public void setReplaceIfExists(Boolean replaceIfExists) {
this.replaceIfExists = replaceIfExists;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
* Creates a new view.
*/
@DatabaseChange(name = "createView", description = "Create a new database view", priority = ChangeMetaData.PRIORITY_DEFAULT)
public class CreateViewChange extends AbstractChange {
public class CreateViewChange extends AbstractChange implements ReplaceIfExists {

private String catalogName;
private String schemaName;
Expand Down Expand Up @@ -96,6 +96,7 @@ public Boolean getReplaceIfExists() {
return replaceIfExists;
}

@Override
public void setReplaceIfExists(Boolean replaceIfExists) {
this.replaceIfExists = replaceIfExists;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package liquibase.command.core;

import liquibase.GlobalConfiguration;
import liquibase.Scope;
import liquibase.command.*;
import liquibase.command.core.helpers.DiffOutputControlCommandStep;
Expand All @@ -14,6 +15,7 @@
import liquibase.logging.mdc.MdcObject;
import liquibase.logging.mdc.MdcValue;
import liquibase.util.StringUtil;
import liquibase.util.ValueHandlerUtil;

import java.io.PrintStream;
import java.util.Arrays;
Expand All @@ -28,6 +30,7 @@ public class DiffChangelogCommandStep extends AbstractCommandStep {
public static final CommandArgumentDefinition<String> CHANGELOG_FILE_ARG;
public static final CommandArgumentDefinition<String> CONTEXTS_ARG;
public static final CommandArgumentDefinition<String> LABEL_FILTER_ARG;
public static final CommandArgumentDefinition<Boolean> USE_OR_REPLACE_OPTION;

static {
final CommandBuilder builder = new CommandBuilder(COMMAND_NAME);
Expand All @@ -43,6 +46,11 @@ public class DiffChangelogCommandStep extends AbstractCommandStep {
.addAlias("contexts")
.description("Changeset contexts to generate")
.build();
USE_OR_REPLACE_OPTION = builder.argument("useOrReplaceOption", Boolean.class)
.description("If true, will add 'OR REPLACE' option to the create view change object")
.defaultValue(false)
.setValueHandler(ValueHandlerUtil::booleanValueHandler)
.build();
Comment on lines +49 to +53
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't necessary for this PR, but instead of repeating the argument, you can make a new class that extends AbstractArgumentCommandStep, and then specify that as a requiresDependency here and in every other class which requires this argument. This helps to reduce the code duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an abstract class with some common behavior that GenerateChangelogCommandStep and DiffChangelogCommandStep both use, I was thinking once that PR is merged to move this new argument there, and then in these two command classes do this:

builder.addArgument(AbstractChangelogCommandStep. USE_OR_REPLACE_OPTION).build();

What do you think about it?

Thanks,
Daniel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good. I don't think that's been done before, but it sounds simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is! @filipelautert showed me yesterday that was already applied to RollbackCommandStep, so I applied the same to GenerateChangelog and DiffChangelog command step classes.

}

@Override
Expand Down Expand Up @@ -72,6 +80,9 @@ public void run(CommandResultsBuilder resultsBuilder) throws Exception {
CommandScope commandScope = resultsBuilder.getCommandScope();
Database referenceDatabase = (Database) commandScope.getDependency(ReferenceDatabase.class);
DiffOutputControl diffOutputControl = (DiffOutputControl) resultsBuilder.getResult(DiffOutputControlCommandStep.DIFF_OUTPUT_CONTROL.getName());
if(commandScope.getArgumentValue(DiffChangelogCommandStep.USE_OR_REPLACE_OPTION).booleanValue()) {
diffOutputControl.setReplaceIfExistsSet(true);
}
referenceDatabase.setOutputDefaultSchema(diffOutputControl.getIncludeSchema());

InternalSnapshotCommandStep.logUnsupportedDatabase(referenceDatabase, this.getClass());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package liquibase.command.core;

import liquibase.GlobalConfiguration;
import liquibase.Scope;
import liquibase.command.*;
import liquibase.command.core.helpers.DbUrlConnectionArgumentsCommandStep;
Expand All @@ -16,6 +17,7 @@
import liquibase.resource.PathHandlerFactory;
import liquibase.resource.Resource;
import liquibase.util.StringUtil;
import liquibase.util.ValueHandlerUtil;

import java.io.IOException;
import java.io.PrintStream;
Expand Down Expand Up @@ -49,6 +51,8 @@ public class GenerateChangelogCommandStep extends AbstractCommandStep {
public static final CommandArgumentDefinition<String> REFERENCE_LIQUIBASE_SCHEMA_NAME_ARG;
public static final CommandArgumentDefinition<String> REFERENCE_LIQUIBASE_CATALOG_NAME_ARG;

public static final CommandArgumentDefinition<Boolean> USE_OR_REPLACE_OPTION;

static {
final CommandBuilder builder = new CommandBuilder(COMMAND_NAME);
CHANGELOG_FILE_ARG = builder.argument(CommonArgumentNames.CHANGELOG_FILE, String.class)
Expand Down Expand Up @@ -88,6 +92,11 @@ public class GenerateChangelogCommandStep extends AbstractCommandStep {
.hidden().build();
REFERENCE_LIQUIBASE_CATALOG_NAME_ARG = builder.argument("referenceLiquibaseCatalogName", String.class)
.hidden().build();
USE_OR_REPLACE_OPTION = builder.argument("useOrReplaceOption", Boolean.class)
.description("If true, will add 'OR REPLACE' option to the create view change object")
.defaultValue(false)
.setValueHandler(ValueHandlerUtil::booleanValueHandler)
.build();
}

@Override
Expand Down Expand Up @@ -115,6 +124,10 @@ public void run(CommandResultsBuilder resultsBuilder) throws Exception {
diffOutputControl.setDataDir(commandScope.getArgumentValue(DATA_OUTPUT_DIR_ARG));
referenceDatabase.setOutputDefaultSchema(diffOutputControl.getIncludeSchema());

if(commandScope.getArgumentValue(GenerateChangelogCommandStep.USE_OR_REPLACE_OPTION).booleanValue()) {
diffOutputControl.setReplaceIfExistsSet(true);
}

InternalSnapshotCommandStep.logUnsupportedDatabase(referenceDatabase, this.getClass());

DiffResult diffResult = (DiffResult) resultsBuilder.getResult(DiffCommandStep.DIFF_RESULT.getName());
Expand Down