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 V4 compatibility with V3 API #347

Merged
merged 18 commits into from
Aug 15, 2023

Conversation

nzdev
Copy link
Contributor

@nzdev nzdev commented Jul 29, 2023

Builds on #346
Merges V3.x into V4.
Fixed compatibility issues with v3.

@nzdev nzdev changed the title [Draft] Fix compatibility with V3 API Fix compatibility with V3 API Jul 29, 2023
@nzdev nzdev marked this pull request as ready for review July 29, 2023 12:55
@nikcio nikcio mentioned this pull request Jul 29, 2023
5 tasks
@nzdev nzdev changed the title Fix compatibility with V3 API Fix V4 compatibility with V3 API Jul 30, 2023
@andrewmckaskill
Copy link

@nzdev I tried this out today but ran into a breaking conflict.
Examine.Lucene.Directories.DirectoryFactoryBase introduced a new abstract method, CreateTaxonomyDirectory

The existing subclasses won't load anymore and cause an error (for Umbraco it's ConfigurationEnabledDirectoryFactory and LuceneRAMDirectoryFactory):

Could not load all types from "Umbraco.Examine.Lucene, Version=10.6.1.0, Culture=neutral, PublicKeyToken=null" due to LoaderExceptions, skipping:
. System.TypeLoadException on Umbraco.Cms.Infrastructure.Examine.ConfigurationEnabledDirectoryFactory: Method 'CreateTaxonomyDirectory' in type 'Umbraco.Cms.Infrastructure.Examine.ConfigurationEnabledDirectoryFactory' from assembly 'Umbraco.Examine.Lucene, Version=10.6.1.0, Culture=neutral, PublicKeyToken=null' does not have an implementation.
. System.TypeLoadException on Umbraco.Cms.Infrastructure.Examine.LuceneRAMDirectoryFactory: Method 'CreateTaxonomyDirectory' in type 'Umbraco.Cms.Infrastructure.Examine.LuceneRAMDirectoryFactory' from assembly 'Umbraco.Examine.Lucene, Version=10.6.1.0, Culture=neutral, PublicKeyToken=null' does not have an implementation.


@nzdev
Copy link
Contributor Author

nzdev commented Aug 13, 2023

@andrewmckaskill Does that error only appear if the UseTaxonomyIndex option is on? If so, I think implementing a version of ConfigurationEnabledDirectoryFactory which implements the CreateTaxonomyDirectory() method is ok. See how ImageSharp v2/v3 was managed in Umbraco. If not, then we would need to move around the directory sync code as well as the directory code.

@andrewmckaskill
Copy link

@nzdev no it appears always. It's because they have directly extended the base class, which now has a new abstract method that has no implementation.

One solution might be to make the method virtual instead of abstract, and to have the default implementation throw a NotImplemetedException.

If the method is only used when Taxonomy is turned on then the default Umbraco implementation won't hit it.

@nzdev
Copy link
Contributor Author

nzdev commented Aug 13, 2023

Done

@Shazwazza
Copy link
Owner

I think this looks good, will merge

@Shazwazza Shazwazza merged commit b1dad2f into Shazwazza:release/4.0 Aug 15, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants