Fix outputcache FileProvider invalid datetime culture format

[thienvc]

Fix outputcache FileProvider invalid datetime culture format

[bdukes]

Can you clarify what issue you're seeing, how to reproduce and validate that this fixes the issue? It looks like there are maybe two or three different fixes in this PR, is that right?

[thienvc]

Can you clarify what issue you're seeing, how to reproduce and validate that this fixes the issue? It looks like there are maybe two or three different fixes in this PR, is that right?

DateTime.UtcNow.Add(duration).ToString() sometimes return localized date style like "2021-11-21 9:44:90 CH".
When the PureCache Scheduler reads the Expiry Date from the .attrib.resources file which cannot be parsed because the scheduler runs NeuturalCulture.
So it must be clear InvariantCulture ToString(CultureInfo.InvariantCulture)
See more similar situation in ModuleCache FileProvider:

Dnn.Platform/DNN Platform/Library/Services/ModuleCache/FileProvider.cs

Line 125 in 1808727

File.WriteAllLines(attribFile, new[] { DateTime.UtcNow.Add(duration).ToString(CultureInfo.InvariantCulture) });

[thienvc]

In addition, during debugging, I encountered an annoying situation when both the cache files of the Module and Page were in the same Cache\Pages folder, which was very difficult to distinguish. I've put together a PR that separates the Module's Cache folder into a separate folder called Cache\Modules, hoping for approval.
cacheFolder = string.Concat(homeDirectoryMapPath, "Cache\\Modules\\");

Dnn.Platform/DNN Platform/Library/Services/ModuleCache/FileProvider.cs

Line 210 in 1808727

cacheFolder = string.Concat(homeDirectoryMapPath, "Cache\\Modules\\");

[thienvc]

It looks like there are maybe two or three different fixes in this PR, is that right?

No, just because PR1 I forgot to delete some meaningless variables I added for debugging so I don't know how to recover so I push PR2 again.

Variables I added but forgot to remove:

Dnn.Platform/DNN Platform/Library/Services/OutputCache/Providers/FileProvider.cs

Lines 175 to 180 in 1808727

	var currentCulture = CultureInfo.CurrentCulture.DateTimeFormat;
	var currentCultureX = CultureInfo.DefaultThreadCurrentCulture.DateTimeFormat;
	var datetimeUtc = DateTime.UtcNow.Add(duration).ToString(CultureInfo.InvariantCulture);
	var datetime1 = DateTime.UtcNow.Add(duration).ToString();
	var datetime2 = DateTime.UtcNow.Add(duration).ToString();

[bdukes]

Looks good to me, thanks!

[thienvc]

I've built and tested on the actual running environment, it runs very well, it's faster
In addition, in the Database cache mechanism, there is also a small error that when inserting content into the OutputCache table, the PrimaryKey is not checked, leading to the Dublicate Primary Key error.
I don't know if I should pull PR to fix this error? This error does not cause a system crash, but Log Viewer is reported to be full
SQL here, Always insert data without checking PrimaryKey:

Dnn.Platform/DNN Platform/Website/Providers/DataProviders/SqlDataProvider/08.00.00.25.SqlDataProvider

Lines 49 to 61 in 9f83285

	BEGIN
	INSERT INTO {databaseOwner}{objectQualifier}OutputCache
	( ItemId,
	CacheKey,
	Data,
	Expiration
	) VALUES
	( @ItemId,
	@CacheKey,
	@Data,
	@Expiration
	)
	END

I fixed on my production site

BEGIN

	IF NOT EXISTS (SELECT CacheKey from dbo.OutputCache where CacheKey = @CacheKey)
		begin
		    INSERT INTO dbo.OutputCache
				(ItemId, 
				CacheKey, 
				Data, 
				Expiration
				) VALUES 
				(@ItemId, 
				@CacheKey, 
				@Data, 
				@Expiration
				)
		end
	ELSE UPDATE dbo.OutputCache Set Data = @Data, Expiration = @Expiration Where CacheKey = @CacheKey
END

@bdukes I do not know should pull this PR? and where to fix?

[bdukes]

Yes, a PR for that would be welcome. You'd need to introduce a new SQL file, 09.10.03.SqlDataProvider with the code to update that stored procedure.

Also, I've updated this PR to target the develop branch (the master branch is only updated when new versions are released). Can you rebase your changes on develop? It may be easier to start a new branch from develop just for this PR and re-submit from that branch.

Thanks!

[sleupold]

I suggest using a SQL merge statement instead for better performance
MERGE INTO {databaseOwner}[{objectQualifier}outputCache] T
USING (SELECT @itemid as ItemID, @Cachekey as CacheKey, ...) S on T.cacheKey = S.CacheKey
WHEN NOT EXIST THEN Insert (ItemID, CacheKey, ...) VALUES S.ItemID, S.CacheKey, ...)

[mitchelsellers]

If adding the SQL fix I believe that the structure outlined by @thienvc is appropriate, per the following guidance from Microsoft.

Performance Tip: The conditional behavior described for the MERGE statement works best when the two tables have a complex mixture of matching characteristics. For example, inserting a row if it doesn't exist, or updating a row if it matches. When simply updating one table based on the rows of another table, improve the performance and scalability with basic INSERT, UPDATE, and DELETE statements. For example:

This was obtained from the official MERGE Documentation

[thienvc]

Yes, a PR for that would be welcome. You'd need to introduce a new SQL file, 09.10.03.SqlDataProvider with the code to update that stored procedure.

Also, I've updated this PR to target the develop branch (the master branch is only updated when new versions are released). Can you rebase your changes on develop? It may be easier to start a new branch from develop just for this PR and re-submit from that branch.

Thanks!

I'm not very good with Github but let me try

[thienvc]

I suggest using a SQL merge statement instead for better performance MERGE INTO {databaseOwner}[{objectQualifier}outputCache] T USING (SELECT @itemid as ItemID, @Cachekey as CacheKey, ...) S on T.cacheKey = S.CacheKey WHEN NOT EXIST THEN Insert (ItemID, CacheKey, ...) VALUES S.ItemID, S.CacheKey, ...)

If adding the SQL fix I believe that the structure outlined by @thienvc is appropriate, per the following guidance from Microsoft.

Performance Tip: The conditional behavior described for the MERGE statement works best when the two tables have a complex mixture of matching characteristics. For example, inserting a row if it doesn't exist, or updating a row if it matches. When simply updating one table based on the rows of another table, improve the performance and scalability with basic INSERT, UPDATE, and DELETE statements. For example:

This was obtained from the official MERGE Documentation

Thanks @sleupold @mitchelsellers I haven't used Merge, so not sure about performance, someone who has tested Profiler and has more experience can give me some advice

[bdukes]

@mitchelsellers I think that's talking about syncing between two tables, where the source of the data being inserted into the table is another query. I don't think that applies here. The first example in that documentation matches what we're trying to do here. It also reduces the possibility for a concurrency race to use MERGE instead of multiple statements.

[thienvc]

Oh, after 2 years it seems this error is back

[valadas]

@thienvc I would recommend creating a new issue with steps to reproduce referencing this PR.