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.

Sign up for GitHub

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

Jump to bottom

Use 'instanceandevents' endpoint when updating process state, so that process and events are updated in a single transaction #937

Merged
merged 3 commits into from
Dec 11, 2024
Merged

Use 'instanceandevents' endpoint when updating process state, so that process and events are updated in a single transaction #937

merged 3 commits into from
Dec 11, 2024

Conversation

martinothamar
Copy link
Contributor

@martinothamar martinothamar commented Nov 28, 2024 •
edited
Loading

Description

Uses the new endpoint which commits new process state and instance events in one transaction.

The related tests were a little funny. Test names said they were testing more than they actually were, it seems like most of the variation comes from the AltinnTaskType thingy.

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

github-advanced-security[bot]
github-advanced-security bot found potential problems Nov 28, 2024
View reviewed changes
src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs
}
else
{
_logger.LogError($"Unable to update instance process with instance id {instance.Id}");

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.
This log entry depends on a
user-provided value
Loading
.

Copilot Autofix AI 3 months ago

To fix the problem, we need to sanitize the instance.Id before logging it. Since the log entries are plain text, we should remove any new line characters from the instance.Id to prevent log forging. This can be done using the String.Replace method to replace new line characters with an empty string.

Suggested changeset 1
src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.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/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs
--- a/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs
+++ b/src/Altinn.App.Core/Infrastructure/Clients/Storage/InstanceClient.cs
@@ -192,3 +192,3 @@
         {
-            _logger.LogError($"Unable to update instance process with instance id {instance.Id}");
+            _logger.LogError($"Unable to update instance process with instance id {instance.Id.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "")}");
             throw await PlatformHttpException.CreateAsync(response);
@@ -218,3 +218,3 @@
         _logger.LogError(
-            $"Unable to create instance {response.StatusCode} - {await response.Content.ReadAsStringAsync()}"
+            $"Unable to create instance {response.StatusCode} - {await response.Content.ReadAsStringAsync().Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "")}"
         );
EOF
@@ -192,3 +192,3 @@
{
_logger.LogError($"Unable to update instance process with instance id {instance.Id}");
_logger.LogError($"Unable to update instance process with instance id {instance.Id.Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "")}");
throw await PlatformHttpException.CreateAsync(response);
@@ -218,3 +218,3 @@
_logger.LogError(
$"Unable to create instance {response.StatusCode} - {await response.Content.ReadAsStringAsync()}"
$"Unable to create instance {response.StatusCode} - {await response.Content.ReadAsStringAsync().Replace(Environment.NewLine, "").Replace("\n", "").Replace("\r", "")}"
);
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
@martinothamar martinothamar force-pushed the chore/use-instanceandevents-endpoint branch from d2dc7c6 to b42909b Compare December 6, 2024 09:30
@martinothamar
Copy link
Contributor Author

/publish

1 similar comment
@martinothamar
Copy link
Contributor Author

/publish

@martinothamar martinothamar force-pushed the chore/use-instanceandevents-endpoint branch from d922cd0 to b42909b Compare December 6, 2024 09:49
@martinothamar
Copy link
Contributor Author

/publish

2 similar comments
@martinothamar
Copy link
Contributor Author

/publish

@martinothamar
Copy link
Contributor Author

/publish

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 6, 2024

Published 8.5.0-rc11.use-instanceandevents-endpoint.b42909b521d1fdb1548f4f3a74ec1103751f34d9

@martinothamar
Copy link
Contributor Author

/publish

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 6, 2024 

Published PR packages:
* [Altinn.App.Api 8.5.0-rc11.use-instanceandevents-endpoint.d6f619d9](https://www.nuget.org/packages/Altinn.App.Api.Experimental/8.5.0-rc11.use-instanceandevents-endpoint.d6f619d9)
* [Altinn.App.Core 8.5.0-rc11.use-instanceandevents-endpoint.d6f619d9](https://www.nuget.org/packages/Altinn.App.Core.Experimental/8.5.0-rc11.use-instanceandevents-endpoint.d6f619d9)

@martinothamar
Copy link
Contributor Author

/publish

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 6, 2024

Published PR packages:

@martinothamar
Copy link
Contributor Author

/publish

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 6, 2024

Published PR packages:

@martinothamar martinothamar force-pushed the chore/use-instanceandevents-endpoint branch from daccd8b to 5bc79a4 Compare December 9, 2024 08:57
@martinothamar
Copy link
Contributor Author

/publish

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 9, 2024

Published PR packages:

@martinothamar martinothamar added the feature Label Pull requests with new features. Used when generation releasenotes label Dec 11, 2024
@martinothamar martinothamar changed the title Use 'instanceandevents' endpoint when updating process state Use 'instanceandevents' endpoint when updating process state, so that process and events are updated in a single transaction Dec 11, 2024
@martinothamar martinothamar force-pushed the chore/use-instanceandevents-endpoint branch from 4f37aa8 to de9fe3f Compare December 11, 2024 08:33
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
10.5% Coverage on New Code (required ≥ 65%)
16.67% Condition Coverage on New Code (required ≥ 65%)

See analysis details on SonarQube Cloud

@martinothamar martinothamar merged commit 80c12b5 into main Dec 11, 2024
10 of 12 checks passed
@martinothamar martinothamar deleted the chore/use-instanceandevents-endpoint branch December 11, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjorntore bjorntore bjorntore approved these changes

Assignees
No one assigned
Labels
feature Label Pull requests with new features. Used when generation releasenotes kind/chore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@martinothamar @bjorntore