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

Jtikekar/reverting nullables #353

Merged
merged 50 commits into from
Nov 6, 2024
Merged

Conversation

juileetikekar
Copy link
Contributor

@juileetikekar juileetikekar commented Sep 26, 2024

Description

Upgrading the AAS Repo APIs to BugFix Version 3.0.2 and fixing the broken Get APIs.

Motivation and Context

Due to refactoring lot of APIs have stopped working. Thus, fixing the API step by step also upgrading the APIs to the latest BugFix Version.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (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)
  • This change requires a documentation update

How Has This Been Tested?

The APIs have been tested manually and with the aas-test-engine.

Screenshots (if appropriate):

Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works


[XmlIgnore]
[JsonIgnore]
private DateTime[] _timeStamp = new DateTime[2];

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_timeStamp' can be 'readonly'.
else if(typeof(IMetadataDTO).IsAssignableFrom(context.ObjectType))
{
JsonNode? json = MetadataJsonSerializer.ToJsonObject((IMetadataDTO)context.Object);
var writer = new Utf8JsonWriter(response.Body);

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'Utf8JsonWriter' is created but not disposed.
Comment on lines +249 to +253
foreach (var item in pagedResult.result)
{
var json = MetadataJsonSerializer.ToJsonObject(item);
jsonArray.Add(json);
}

Check notice

Code scanning / CodeQL

Missed opportunity to use Select Note

This foreach loop immediately
maps its iteration variable to another variable
- consider mapping the sequence explicitly using '.Select(...)'.
pagingMetadata["cursor"] = cursor;
}
jsonNode["paging_metadata"] = pagingMetadata;
var writer = new Utf8JsonWriter(response.Body);

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'Utf8JsonWriter' is created but not disposed.
{
jsonNode = JsonSerializer.SerializeToNode(pagedResult);
}
var writer = new Utf8JsonWriter(response.Body);

Check warning

Code scanning / CodeQL

Missing Dispose call on local IDisposable Warning

Disposable 'Utf8JsonWriter' is created but not disposed.
var valueNode = JsonSerializer.Deserialize<JsonNode>(valueString);
valueArray.Add(valueNode);
}
var elementNode = ToJsonObject(element, true) as JsonNode;

Check warning

Code scanning / CodeQL

Cast to same type Warning

This cast is redundant because the expression already has type JsonNode.
return valueObject;
}

if (isParentSML)

Check warning

Code scanning / CodeQL

Constant condition Warning

Condition always evaluates to 'false'.
var context = new LevelExtentModifierContext(level, extent);
return that.Select(source => source != null ? _transformer.Transform(source, context) : null).ToList();
private readonly IAppLogger<LevelExtentModifierService> _logger;
LevelExtentTransformer _transformer;

Check notice

Code scanning / CodeQL

Missed 'readonly' opportunity Note

Field '_transformer' can be 'readonly'.
@@ -214,7 +214,7 @@
[SwaggerResponse(statusCode: 404, type: typeof(Result), description: "Not Found")]
[SwaggerResponse(statusCode: 500, type: typeof(Result), description: "Internal Server Error")]
[SwaggerResponse(statusCode: 0, type: typeof(Result), description: "Default error handling for unmentioned status codes")]
public virtual IActionResult GetAllSubmodelDescriptorsThroughSuperpath([FromRoute] [Required] string aasIdentifier,
public virtual IActionResult GetAllSubmodelDescriptorsThroughSuperpath([FromRoute] [Required] string? aasIdentifier,

Check failure

Code scanning / CodeQL

Missing function level access control High

This action is missing an authorization check.

Copilot Autofix AI 4 months ago

To fix the issue, we need to add an authorization check to the GetAllSubmodelDescriptorsThroughSuperpath method. The best way to do this in an ASP.NET Core MVC application is to use the [Authorize] attribute, which ensures that only authenticated users can access the method. If specific roles are required, we can specify them within the attribute.

  • Add the [Authorize] attribute to the GetAllSubmodelDescriptorsThroughSuperpath method.
  • Ensure that the necessary imports for authorization are present.
Suggested changeset 1
src/IO.Swagger.Registry.Lib.V3/Controllers/AssetAdministrationShellRegistryAPIApi.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/src/IO.Swagger.Registry.Lib.V3/Controllers/AssetAdministrationShellRegistryAPIApi.cs b/src/IO.Swagger.Registry.Lib.V3/Controllers/AssetAdministrationShellRegistryAPIApi.cs
--- a/src/IO.Swagger.Registry.Lib.V3/Controllers/AssetAdministrationShellRegistryAPIApi.cs
+++ b/src/IO.Swagger.Registry.Lib.V3/Controllers/AssetAdministrationShellRegistryAPIApi.cs
@@ -216,2 +216,3 @@
     [SwaggerResponse(statusCode: 0, type: typeof(Result), description: "Default error handling for unmentioned status codes")]
+    [Authorize]
     public virtual IActionResult GetAllSubmodelDescriptorsThroughSuperpath([FromRoute] [Required] string? aasIdentifier,
EOF
@@ -216,2 +216,3 @@
[SwaggerResponse(statusCode: 0, type: typeof(Result), description: "Default error handling for unmentioned status codes")]
[Authorize]
public virtual IActionResult GetAllSubmodelDescriptorsThroughSuperpath([FromRoute] [Required] string? aasIdentifier,
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ juileetikekar
❌ martafullen
You have signed the CLA already but the status is still pending? Let us recheck it.

@martafullen
Copy link
Contributor

Added copyright headers.
Note that this branch still contains OPCFoundation dependencies (and so does main), see PR #352

Change in incorrect level and extent handling
var aasList = new List<IAssetAdministrationShell>();
foreach (var assetId in assetIds)
{
aasList = output.Where(a => a.AssetInformation.SpecificAssetIds.ContainsSpecificAssetId(assetId)).ToList();
var result = new List<IAssetAdministrationShell>();

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
result
is useless, since its value is never read.
Comment on lines +226 to +233
if (assetId.Name.Equals("globalAssetId", StringComparison.OrdinalIgnoreCase))
{
result = output.Where(a => a.AssetInformation.GlobalAssetId!.Equals(assetId.Value)).ToList();
}
else
{
result = output.Where(a => a.AssetInformation.SpecificAssetIds!.ContainsSpecificAssetId(assetId)).ToList();
}

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.
Comment on lines +98 to +112
(
InoutputArguments == other.InoutputArguments ||
InoutputArguments != null &&
InoutputArguments.Equals(other.InoutputArguments)
) &&
(
InputArguments == other.InputArguments ||
InputArguments != null &&
InputArguments.Equals(other.InputArguments)
) &&
(
ClientTimeoutDuration == other.ClientTimeoutDuration ||
ClientTimeoutDuration != null &&
ClientTimeoutDuration.Equals(other.ClientTimeoutDuration)
);

Check notice

Code scanning / CodeQL

Complex condition Note

Complex condition: too many logical operations in this expression.
Comment on lines +125 to +126
if (InoutputArguments != null)
hashCode = hashCode * 59 + InoutputArguments.GetHashCode();

Check warning

Code scanning / CodeQL

Misleading indentation Warning

Missing braces? Inspect indentation of
the control flow successor
.
Comment on lines +127 to +128
if (InputArguments != null)
hashCode = hashCode * 59 + InputArguments.GetHashCode();

Check warning

Code scanning / CodeQL

Misleading indentation Warning

Missing braces? Inspect indentation of
the control flow successor
.
Comment on lines +111 to +112
if (InoutputArguments != null)
hashCode = hashCode * 59 + InoutputArguments.GetHashCode();

Check warning

Code scanning / CodeQL

Misleading indentation Warning

Missing braces? Inspect indentation of
the control flow successor
.
Comment on lines +35 to +38
catch (Exception e)
{
throw new InvalidSerializationModifierException("extent");
}

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
{
result = Enum.Parse<ExtentEnum>(extent, true);
}
catch (Exception e)

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
e
is useless, since its value is never read.
Comment on lines +53 to +56
catch(Exception e)
{
throw new InvalidSerializationModifierException("level");
}

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
{
result = Enum.Parse<LevelEnum>(level, true);
}
catch(Exception e)

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
e
is useless, since its value is never read.
[SwaggerResponse(statusCode: 403, type: typeof(Result), description: "Forbidden")]
public virtual IActionResult GetDescription()
{
var output = new ServiceDescription();

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

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

/// <inheritdoc cref="IServiceDescription.ToString" />
public override string ToString()
{
var sb = new StringBuilder();
sb.Append("class ServiceDescription {\n");
sb.Append(" Profiles: ").Append(Profiles).Append('\n');
sb.Append(" Profiles: ").Append(profiles).Append('\n');

Check warning

Code scanning / CodeQL

Use of default ToString() Warning

Default 'ToString()':
List
inherits 'ToString()' from 'Object', and so is not suitable for printing.
var reqIsCaseOf = _jsonQueryDeserializer.DeserializeReference("isCaseOf", isCaseOf);
var reqDataSpecificationRef = _jsonQueryDeserializer.DeserializeReference("dataSpecificationRef", dataSpecificationRef);

var cdList = new List<IConceptDescription>();

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
cdList
is useless, since its value is never read.
outputEnv.AssetAdministrationShells.AddRange(aasList);
}

//Fetch Submodels for the requested submodelIds
var submodelList = _submodelService.GetAllSubmodels();
//Using is null or empty, as the query parameter in controll currently receives empty list (not null, but count = 0)
if (submodelIds.IsNullOrEmpty())
if (!submodelIds.IsNullOrEmpty())

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
submodelIds
may be null at this access because the parameter has a null default value.
Variable
submodelIds
may be null at this access because the parameter has a null default value.
.Where(foundSubmodel => foundSubmodel.Any()))


if((bool)includeCD)

Check warning

Code scanning / CodeQL

Dereferenced variable may be null Warning

Variable
includeCD
may be null at this access because it has a nullable type.
@juileetikekar juileetikekar merged commit d1cffea into main Nov 6, 2024
6 of 7 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.

3 participants