Skip to content

[Datasets] Task/rdmp 33 dataset update post load#2191

Draft
JFriel wants to merge 3 commits intotask/RDMP-33-dataset-integration-interfacefrom
task/RDMP-33-dataset-update-post-load
Draft

[Datasets] Task/rdmp 33 dataset update post load#2191
JFriel wants to merge 3 commits intotask/RDMP-33-dataset-integration-interfacefrom
task/RDMP-33-dataset-update-post-load

Conversation

@JFriel
Copy link
Collaborator

@JFriel JFriel commented May 2, 2025

Proposed Change

Summarise your proposed changes here, including any notes for reviewers.

Type of change

What types of changes does your code introduce? Tick all that apply.

  • Bugfix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation-Only Update
  • Other (if none of the other choices apply)

Checklist

By opening this PR, I confirm that I have:

  • Ensured that the PR branch is in sync with the target branch (i.e. it is automatically merge-able)
  • Created or updated any tests if relevant
  • Have validated this change against the Test Plan
  • Requested a review by one of the repository maintainers
  • Have written new documentation or updated existing documentation to detail any new or updated functionality and how to use it
  • Have added an entry into the changelog

Assert.DoesNotThrow(cmd.Execute);
var founddataset = GetMockActivator().RepositoryLocator.CatalogueRepository.GetAllObjects<Core.Curation.Data.Dataset>().First();
var linkCmd = new ExecuteCommandLinkCatalogueToDataset(GetMockActivator(), new Catalogue(GetMockActivator().RepositoryLocator.CatalogueRepository,"catalogue"), null, false);
var founddataset = GetMockActivator().RepositoryLocator.CatalogueRepository.GetAllObjects<Core.Curation.Data.Datasets.Dataset>().First();

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning test

This assignment to
founddataset
is useless, since its value is never read.

Copilot Autofix

AI 9 months ago

To fix the issue, the assignment to founddataset on line 90 should be removed entirely, as it serves no purpose in the method. This will eliminate the "useless assignment" and make the code cleaner and more maintainable. No additional changes are required, as the removal does not affect the logic or functionality of the test.


Suggested changeset 1
Rdmp.Core.Tests/CommandExecution/ExecuteCommandLinkCatalogueToDatasetTests.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Rdmp.Core.Tests/CommandExecution/ExecuteCommandLinkCatalogueToDatasetTests.cs b/Rdmp.Core.Tests/CommandExecution/ExecuteCommandLinkCatalogueToDatasetTests.cs
--- a/Rdmp.Core.Tests/CommandExecution/ExecuteCommandLinkCatalogueToDatasetTests.cs
+++ b/Rdmp.Core.Tests/CommandExecution/ExecuteCommandLinkCatalogueToDatasetTests.cs
@@ -89,3 +89,2 @@
         Assert.DoesNotThrow(cmd.Execute);
-        var founddataset = GetMockActivator().RepositoryLocator.CatalogueRepository.GetAllObjects<Core.Curation.Data.Datasets.Dataset>().First();
         var linkCmd = new ExecuteCommandLinkCatalogueToDataset(GetMockActivator(), new Catalogue(GetMockActivator().RepositoryLocator.CatalogueRepository, "catalogue"), null, false);
EOF
@@ -89,3 +89,2 @@
Assert.DoesNotThrow(cmd.Execute);
var founddataset = GetMockActivator().RepositoryLocator.CatalogueRepository.GetAllObjects<Core.Curation.Data.Datasets.Dataset>().First();
var linkCmd = new ExecuteCommandLinkCatalogueToDataset(GetMockActivator(), new Catalogue(GetMockActivator().RepositoryLocator.CatalogueRepository, "catalogue"), null, false);
Copilot is powered by AI and may make mistakes. Always verify output.
base.SaveToDatabase();
foreach (var dataset in CatalogueRepository.GetAllObjectsWhere<CatalogueDatasetLinkage>("Catalogue_ID", this.ID).Where(cdl => cdl.Autoupdate).Select(cld => cld.Dataset))
{
var provider = CatalogueRepository.GetObjectByID<DatasetProviderConfiguration>((int)dataset.Provider_ID);

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
provider
is useless, since its value is never read.

Copilot Autofix

AI 9 months ago

To fix the issue, the assignment to the provider variable on line 1163 should be removed entirely, as it is not used anywhere in the code. This will eliminate the unnecessary assignment and make the code cleaner. No additional changes are required, as the subsequent assignment to providerConfiguration already retrieves the necessary object.


Suggested changeset 1
Rdmp.Core/Curation/Data/Catalogue.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Rdmp.Core/Curation/Data/Catalogue.cs b/Rdmp.Core/Curation/Data/Catalogue.cs
--- a/Rdmp.Core/Curation/Data/Catalogue.cs
+++ b/Rdmp.Core/Curation/Data/Catalogue.cs
@@ -1162,3 +1162,2 @@
         {
-            var provider = CatalogueRepository.GetObjectByID<DatasetProviderConfiguration>((int)dataset.Provider_ID);
             var providerConfiguration = CatalogueRepository.GetObjectByID<DatasetProviderConfiguration>((int)dataset.Provider_ID);
EOF
@@ -1162,3 +1162,2 @@
{
var provider = CatalogueRepository.GetObjectByID<DatasetProviderConfiguration>((int)dataset.Provider_ID);
var providerConfiguration = CatalogueRepository.GetObjectByID<DatasetProviderConfiguration>((int)dataset.Provider_ID);
Copilot is powered by AI and may make mistakes. Always verify output.
{
private int _catalogueID;
private int _datasetID;
private ICatalogueRepository _repository;

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_repository' can be 'readonly'.

Copilot Autofix

AI 9 months ago

To fix the issue, the readonly modifier should be added to the _repository field declaration. This change ensures that _repository can only be assigned during its declaration or within the constructors of the CatalogueDatasetLinkage class. No other changes are required, as the field is already only assigned in the constructors.


Suggested changeset 1
Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs b/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs
--- a/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs
+++ b/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs
@@ -20,3 +20,3 @@
         private int _datasetID;
-        private ICatalogueRepository _repository;
+        private readonly ICatalogueRepository _repository;
 
EOF
@@ -20,3 +20,3 @@
private int _datasetID;
private ICatalogueRepository _repository;
private readonly ICatalogueRepository _repository;

Copilot is powered by AI and may make mistakes. Always verify output.
{
{"Catalogue_ID", catalogue.ID },
{ "Dataset_ID", dataset.ID},
{"Autoupdate", autoupdate==true?1:0 }

Check notice

Code scanning / CodeQL

Unnecessarily complex Boolean expression Note

The expression 'A == true' can be simplified to 'A'.

Copilot Autofix

AI 9 months ago

To fix the issue, we will simplify the expression autoupdate == true to just autoupdate. This change will make the code more concise and easier to read while maintaining the same functionality. The modification will be applied to line 50 in the constructor of the CatalogueDatasetLinkage class.

Suggested changeset 1
Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs b/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs
--- a/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs
+++ b/Rdmp.Core/Curation/Data/Datasets/CatalogueDatasetLinkage.cs
@@ -49,3 +49,3 @@
                 { "Dataset_ID", dataset.ID},
-                {"Autoupdate", autoupdate==true?1:0 }
+                {"Autoupdate", autoupdate ? 1 : 0 }
             });
EOF
@@ -49,3 +49,3 @@
{ "Dataset_ID", dataset.ID},
{"Autoupdate", autoupdate==true?1:0 }
{"Autoupdate", autoupdate ? 1 : 0 }
});
Copilot is powered by AI and may make mistakes. Always verify output.
qb.AddCustomLine($"{SpecialFieldNames.DataLoadRunID} = {dataLoadID}", FAnsi.Discovery.QuerySyntax.QueryComponent.WHERE);
var sql = qb.SQL;

var dt = new DataTable();

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'DataTable' is created but not disposed.
Comment on lines 37 to 44
if (_activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).Any())
{
dataset = _activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).First();
}
else
{
dataset = provider.AddExistingDatasetWithReturn(null, tbID.Text);
}

Check notice

Code scanning / CodeQL

Missed ternary opportunity Note

Both branches of this 'if' statement write to the same variable - consider using '?' to express intent better.

Copilot Autofix

AI 9 months ago

To fix the issue, we will replace the if statement on line 37 with a single line using the ternary operator. This will assign the value to the dataset variable based on the condition in a more concise and readable manner. The functionality of the code will remain unchanged.


Suggested changeset 1
Rdmp.UI/SubComponents/LinkDatasetDialog.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Rdmp.UI/SubComponents/LinkDatasetDialog.cs b/Rdmp.UI/SubComponents/LinkDatasetDialog.cs
--- a/Rdmp.UI/SubComponents/LinkDatasetDialog.cs
+++ b/Rdmp.UI/SubComponents/LinkDatasetDialog.cs
@@ -36,10 +36,5 @@
             var fetchedDataset = provider.FetchDatasetByID(int.Parse(tbID.Text));//todo is it always ints?
-            if (_activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).Any())
-            {
-                dataset = _activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).First();
-            }
-            else
-            {
-                dataset = provider.AddExistingDatasetWithReturn(null, tbID.Text);
-            }
+            dataset = _activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).Any()
+                ? _activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).First()
+                : provider.AddExistingDatasetWithReturn(null, tbID.Text);
             if (!_activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<CatalogueDatasetLinkage>("Dataset_ID", dataset.ID).Where(l => l.Catalogue.ID == _catalogue.ID).Any())
EOF
@@ -36,10 +36,5 @@
var fetchedDataset = provider.FetchDatasetByID(int.Parse(tbID.Text));//todo is it always ints?
if (_activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).Any())
{
dataset = _activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).First();
}
else
{
dataset = provider.AddExistingDatasetWithReturn(null, tbID.Text);
}
dataset = _activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).Any()
? _activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<Dataset>("Url", fetchedDataset.Url).First()
: provider.AddExistingDatasetWithReturn(null, tbID.Text);
if (!_activator.RepositoryLocator.CatalogueRepository.GetAllObjectsWhere<CatalogueDatasetLinkage>("Dataset_ID", dataset.ID).Where(l => l.Catalogue.ID == _catalogue.ID).Any())
Copilot is powered by AI and may make mistakes. Always verify output.

if(typeof(T) == typeof(CatalogueDatasetLinkage))
{
return (T)(object)new CatalogueDatasetLinkage(repository.CatalogueRepository, WhenIHaveA<Catalogue>(repository), WhenIHaveA<Dataset>(repository));

Check warning

Code scanning / CodeQL

Useless upcast Warning

There is no need to upcast from
CatalogueDatasetLinkage
to
Object
- the conversion can be done implicitly.

Copilot Autofix

AI 9 months ago

To fix the issue, we will remove the explicit cast to (object) on line 611. The new CatalogueDatasetLinkage(...) expression can be directly cast to (T) without the intermediate cast to (object). This change will simplify the code while maintaining its functionality.


Suggested changeset 1
Tests.Common/UnitTests.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Tests.Common/UnitTests.cs b/Tests.Common/UnitTests.cs
--- a/Tests.Common/UnitTests.cs
+++ b/Tests.Common/UnitTests.cs
@@ -610,3 +610,3 @@
         {
-            return (T)(object)new CatalogueDatasetLinkage(repository.CatalogueRepository, WhenIHaveA<Catalogue>(repository), WhenIHaveA<Dataset>(repository));
+            return (T)new CatalogueDatasetLinkage(repository.CatalogueRepository, WhenIHaveA<Catalogue>(repository), WhenIHaveA<Dataset>(repository));
         }
EOF
@@ -610,3 +610,3 @@
{
return (T)(object)new CatalogueDatasetLinkage(repository.CatalogueRepository, WhenIHaveA<Catalogue>(repository), WhenIHaveA<Dataset>(repository));
return (T)new CatalogueDatasetLinkage(repository.CatalogueRepository, WhenIHaveA<Catalogue>(repository), WhenIHaveA<Dataset>(repository));
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
if(typeof(T) == typeof(Dataset))
{
return (T)(object)new Dataset(repository.CatalogueRepository, "Dataset");

Check warning

Code scanning / CodeQL

Useless upcast Warning

There is no need to upcast from
Dataset
to
Object
- the conversion can be done implicitly.

Copilot Autofix

AI 9 months ago

To fix the issue, we will remove the explicit cast (object) from the expression new Dataset(repository.CatalogueRepository, "Dataset"). This will allow the implicit conversion to object to occur naturally, as all types in C# derive from object. The functionality of the code will remain unchanged.


Suggested changeset 1
Tests.Common/UnitTests.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Tests.Common/UnitTests.cs b/Tests.Common/UnitTests.cs
--- a/Tests.Common/UnitTests.cs
+++ b/Tests.Common/UnitTests.cs
@@ -614,3 +614,3 @@
         {
-            return (T)(object)new Dataset(repository.CatalogueRepository, "Dataset");
+            return (T)new Dataset(repository.CatalogueRepository, "Dataset");
         }
EOF
@@ -614,3 +614,3 @@
{
return (T)(object)new Dataset(repository.CatalogueRepository, "Dataset");
return (T)new Dataset(repository.CatalogueRepository, "Dataset");
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
if (typeof(T) == typeof(PluginDataset))
{
return (T)(object)new PluginDataset(repository.CatalogueRepository, "Plugin Dataset");

Check warning

Code scanning / CodeQL

Useless upcast Warning

There is no need to upcast from
PluginDataset
to
Object
- the conversion can be done implicitly.

Copilot Autofix

AI 9 months ago

To fix the issue, we will remove the explicit cast (object) from the expression new PluginDataset(repository.CatalogueRepository, "Plugin Dataset"). This will eliminate the unnecessary upcast while preserving the functionality of the code. The return statement will still cast the result to type T, which is the intended behavior.


Suggested changeset 1
Tests.Common/UnitTests.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Tests.Common/UnitTests.cs b/Tests.Common/UnitTests.cs
--- a/Tests.Common/UnitTests.cs
+++ b/Tests.Common/UnitTests.cs
@@ -618,3 +618,3 @@
         {
-            return (T)(object)new PluginDataset(repository.CatalogueRepository, "Plugin Dataset");
+            return (T)new PluginDataset(repository.CatalogueRepository, "Plugin Dataset");
         }
EOF
@@ -618,3 +618,3 @@
{
return (T)(object)new PluginDataset(repository.CatalogueRepository, "Plugin Dataset");
return (T)new PluginDataset(repository.CatalogueRepository, "Plugin Dataset");
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
if (typeof(T) == typeof(DatasetProviderConfiguration))
{
return (T)(object)new DatasetProviderConfiguration(repository.CatalogueRepository, "","","",WhenIHaveA<DataAccessCredentials>(repository).ID,"");

Check warning

Code scanning / CodeQL

Useless upcast Warning

There is no need to upcast from
DatasetProviderConfiguration
to
Object
- the conversion can be done implicitly.

Copilot Autofix

AI 9 months ago

To fix the issue, we will remove the explicit cast to (object) on line 623. The DatasetProviderConfiguration object can be directly cast to the generic type T without the intermediate cast to object. This change will simplify the code without altering its functionality.


Suggested changeset 1
Tests.Common/UnitTests.cs

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Tests.Common/UnitTests.cs b/Tests.Common/UnitTests.cs
--- a/Tests.Common/UnitTests.cs
+++ b/Tests.Common/UnitTests.cs
@@ -622,3 +622,3 @@
         {
-            return (T)(object)new DatasetProviderConfiguration(repository.CatalogueRepository, "","","",WhenIHaveA<DataAccessCredentials>(repository).ID,"");
+            return (T)new DatasetProviderConfiguration(repository.CatalogueRepository, "","","",WhenIHaveA<DataAccessCredentials>(repository).ID,"");
         }
EOF
@@ -622,3 +622,3 @@
{
return (T)(object)new DatasetProviderConfiguration(repository.CatalogueRepository, "","","",WhenIHaveA<DataAccessCredentials>(repository).ID,"");
return (T)new DatasetProviderConfiguration(repository.CatalogueRepository, "","","",WhenIHaveA<DataAccessCredentials>(repository).ID,"");
}
Copilot is powered by AI and may make mistakes. Always verify output.
@JFriel JFriel changed the base branch from develop to task/RDMP-33-dataset-integration-interface May 6, 2025 08:47
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.

1 participant