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

Default implementation of handleInvalidEmptyPreconditionCase added in AbtractFormattedChangeLogParser to avoid breaking extensions #5660

Merged
merged 3 commits into from Mar 5, 2024

Conversation

MalloD12
Copy link
Contributor

@MalloD12 MalloD12 commented Mar 5, 2024

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

Recently with the merge of this PR the team has noticed this is breaking extensions, so just move the code implementation of handleInvalidEmptyPreconditionCase to the abstract class which should fix this issue if no extension overrides this method.

Things to be aware of

Things to worry about

Additional Context

…attedSqlChangeLogParser.

- Added default handleInvalidEmptyPreconditionCase implementation in AbstractFormattedChangeLogParser.
@@ -610,6 +608,23 @@
return resourceAccessor.getExisting(physicalChangeLogLocation).openInputStream();
}

protected void handleInvalidEmptyPreconditionCase(ChangeLogParameters changeLogParameters, ChangeSet changeSet, Matcher preconditionMatcher) throws ChangeLogParseException {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'changeLogParameters' is never used.
@@ -610,6 +608,23 @@
return resourceAccessor.getExisting(physicalChangeLogLocation).openInputStream();
}

protected void handleInvalidEmptyPreconditionCase(ChangeLogParameters changeLogParameters, ChangeSet changeSet, Matcher preconditionMatcher) throws ChangeLogParseException {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'changeSet' is never used.
@vitaliimak
Copy link
Contributor

Thanks, @MalloD12 for such a quick fix!
It should work, but, I'm wondering if it is possible to leave the abstract class without SQL-specific implementation. The commercial MongoDB extension does not use sql-check, table-exists, and so on.
Is it possible to write something like this?

protected void handleInvalidEmptyPreconditionCase(ChangeLogParameters changeLogParameters, ChangeSet changeSet, Matcher preconditionMatcher) throws ChangeLogParseException {
    throw new NotImplementedException();
}

And left SQL-specific code in the FormattedSqlChangeLogParser class.

@MalloD12
Copy link
Contributor Author

MalloD12 commented Mar 5, 2024

protected void handleInvalidEmptyPreconditionCase(ChangeLogParameters changeLogParameters, ChangeSet changeSet, Matcher preconditionMatcher) throws ChangeLogParseException {
throw new NotImplementedException();
}

Ohh, I see. I get it now. Sure, I can do that @vitaliimak.

…mpty.

- Added back handleInvalidEmptyPreconditionCase specific implemenation in FormattedSqlChangeLogParser class.
@MalloD12 MalloD12 changed the title Moved handleInvalidEmptyPreconditionCase to AbtractFormattedChangeLogParser to avoid breaking extensions Added default implementation of handleInvalidEmptyPreconditionCase in AbtractFormattedChangeLogParser to avoid breaking extensions Mar 5, 2024
@MalloD12 MalloD12 changed the title Added default implementation of handleInvalidEmptyPreconditionCase in AbtractFormattedChangeLogParser to avoid breaking extensions Default implementation of handleInvalidEmptyPreconditionCase added in AbtractFormattedChangeLogParser to avoid breaking extensions Mar 5, 2024
@filipelautert filipelautert added this to the 1NEXT milestone Mar 5, 2024
@filipelautert filipelautert merged commit e8a62e8 into master Mar 5, 2024
31 of 33 checks passed
@filipelautert filipelautert deleted the fix-for-PR-5456 branch March 5, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants