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

Add .NET 8 target framework #300

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Add .NET 8 target framework #300

wants to merge 7 commits into from

Conversation

bratchikov
Copy link
Member

@bratchikov bratchikov commented Sep 11, 2024

Summary by CodeRabbit

  • New Features

    • Support for .NET 8.0 framework added across multiple projects.
    • New updateViews parameter introduced for enhanced data model building.
    • Docker Compose configuration for Microsoft SQL Server (MSSQL) added, simplifying database setup for tests.
  • Bug Fixes

    • Resolved issues related to loading objects and details.
  • Documentation

    • Updated README.md with instructions for running tests using MSSQL and PostgreSQL.

Copy link

coderabbitai bot commented Oct 9, 2024

Walkthrough

The changes in this pull request involve updates to several project files to support the .NET 8.0 framework. This includes modifications to the <TargetFrameworks> property across multiple projects and updates to the <ItemGroup> sections to accommodate the new framework. Additionally, the NewPlatform.Flexberry.ORM.ODataService.nuspec file has been updated with a new method parameter and version increment, while the integration tests' README and Docker Compose configuration for MSSQL have been enhanced.

Changes

File Change Summary
NewPlatform.Flexberry.ORM.ODataService.Files/NewPlatform.Flexberry.ORM.ODataService.Files.csproj, NewPlatform.Flexberry.ORM.ODataService/NewPlatform.Flexberry.ORM.ODataService.csproj, NewPlatform.Flexberry.ORM.ODataServiceCore.Common/NewPlatform.Flexberry.ORM.ODataServiceCore.Common.csproj, NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi/NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj, Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/NewPlatform.Flexberry.ORM.ODataService.Tests.csproj Updated <TargetFrameworks> to include net8.0 and modified conditions in <ItemGroup> sections for framework compatibility.
NewPlatform.Flexberry.ORM.ODataService.nuspec Added updateViews parameter in DefaultDataObjectEdmModelBuilder, updated Flexberry ORM version to 7.2.0, added dependency group for .NET8.0, and included new file entries for .NET8.0.
Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/README.md Updated instructions for running tests with MSSQL and modifications for PostgreSQL tests.
Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/docker-compose-mssql.yml Defined new service mssql and volume mssqldb for Docker Compose configuration.

Poem

In the garden of code, we hop and play,
With .NET 8.0, we brighten the day.
New views to update, and tests to run,
A rabbit's delight, oh what fun! 🐇✨
With MSSQL's embrace, our work is complete,
In this world of tech, we dance on our feet!


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Outside diff range and nitpick comments (8)
Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/docker-compose-mssql.yml (2)

1-1: Consider updating to the latest stable Docker Compose version.

The current version '3.2' is relatively old. Consider updating to version '3.8', which is the latest stable version (as of 2021). This would provide access to newer features and improvements in Docker Compose.

Here's the suggested change:

-version: '3.2'
+version: '3.8'
🧰 Tools
🪛 yamllint

[error] 1-1: wrong new line character: expected \n

(new-lines)


1-15: Address linting issues for improved code quality.

The yamllint tool has reported two issues:

  1. Incorrect line endings: The file uses CRLF line endings instead of the expected LF.
  2. Trailing spaces on line 13.

To resolve these issues:

  1. Configure your editor to use LF (Unix-style) line endings for this project.
  2. Remove the trailing spaces from line 13.

Here's a diff to fix the trailing spaces:

 volumes:
   - mssqldb:/var/opt/mssql
-      
+

Consider adding a .editorconfig file to the project root to enforce consistent line endings and trailing space rules across the project.

🧰 Tools
🪛 yamllint

[error] 1-1: wrong new line character: expected \n

(new-lines)


[error] 13-13: trailing spaces

(trailing-spaces)

NewPlatform.Flexberry.ORM.ODataServiceCore.Common/NewPlatform.Flexberry.ORM.ODataServiceCore.Common.csproj (1)

18-18: LGTM: Correctly updated framework reference condition

The condition for including the Microsoft.AspNetCore.App framework reference has been correctly updated to include net8.0.

For future-proofing, consider refactoring this condition to use a more concise syntax. For example:

<ItemGroup Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'netcoreapp3.1'))">
  <FrameworkReference Include="Microsoft.AspNetCore.App" />
</ItemGroup>

This approach would automatically include the framework reference for all compatible versions without needing to update the condition for each new .NET version.

Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/README.md (2)

3-16: Approve changes with minor suggestions for improvement

The changes in this section look good, especially the addition of the -d flag to the Docker command. However, there are a few minor grammatical improvements that could be made:

  1. Line 3: Consider changing "Tests run Postgres" to "Running Tests with PostgreSQL" for clarity.
  2. Line 5: Add a comma after "Before start test run" for better readability.
  3. Line 10: Change "if it not installed yet" to "if it's not installed yet" for correct grammar.
  4. Line 12: Consider rephrasing to "Then start the Docker container with PostgreSQL using this command from the root folder of this repository:"

Would you like me to propose these changes in a diff format?

🧰 Tools
🪛 LanguageTool

[typographical] ~5-~5: Consider adding a comma here.
Context: ...ts run Postgres

Before start test run please fill connection string `ConnectionStrin...

(PLEASE_COMMA)


[grammar] ~10-~10: Did you mean “did not install”?
Context: ...tall Docker if it not installed yet and start Docker.

Then start Doc...

(PRP_NOT_VB)


[uncategorized] ~12-~12: You might be missing the article “the” here.
Context: ...ntainer with PostgreSQL by command from root folder this repository:

docke...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

</blockquote></details>

</details>

---

`28-50`: **Approve addition of MSSQL section with minor suggestions**

The addition of the MSSQL section is excellent, providing clear instructions for users who need to run tests with Microsoft SQL Server. The structure is consistent with the PostgreSQL section, which aids readability. Here are a few suggestions for improvement:

1. Line 28: Consider changing "Test run MSSQL" to "Running Tests with Microsoft SQL Server (MSSQL)" for clarity.
2. Line 30: Add a comma after "Before start test run" for better readability.
3. Line 36: Change "if it not installed yet" to "if it's not installed yet" for correct grammar.
4. Line 38: Consider rephrasing to "Then start the Docker container with Microsoft SQL Server using this command from the root folder of this repository:"



Would you like me to propose these changes in a diff format?

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 LanguageTool</summary><blockquote>

[typographical] ~30-~30: Consider adding a comma here.
Context: ... Test run MSSQL

Before start test run please fill connection string `ConnectionStrin...

(PLEASE_COMMA)

---

[grammar] ~36-~36: Did you mean “did not install”?
Context: ...tall [Docker](https://docker.com) if it not installed yet and start Docker.

Then start Doc...

(PRP_NOT_VB)

---

[uncategorized] ~38-~38: Possible missing preposition found.
Context: ... SQL Server by command from root folder this repository:

```sh
docker-compose -f...

(AI_HYDRA_LEO_MISSING_IN)

</blockquote></details>

</details>

</blockquote></details>
<details>
<summary>NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi/NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj (1)</summary><blockquote>

Line range hint `1-39`: **Summary: Successful addition of .NET 8 target framework**

The changes to add .NET 8 as a target framework have been implemented correctly. Both the `TargetFrameworks` property and the `ItemGroup` condition have been updated appropriately. These changes align well with the PR objective.

To ensure a smooth transition to .NET 8 support, consider the following final checks:

1. Run the provided verification scripts to confirm package and framework reference compatibility.
2. Test the project build and runtime behavior targeting .NET 8 to catch any potential issues not visible in the project file.
3. Update any related documentation or README files to reflect the new .NET 8 support.
4. If this project is part of a larger solution, ensure that other projects are updated as necessary to maintain compatibility.

</blockquote></details>
<details>
<summary>NewPlatform.Flexberry.ORM.ODataService.Files/NewPlatform.Flexberry.ORM.ODataService.Files.csproj (1)</summary><blockquote>

`43-45`: **LGTM: Updated framework reference condition for .NET 8.0**

The addition of `net8.0` to the condition for including the `Microsoft.AspNetCore.App` framework reference is correct and necessary for .NET 8.0 support. This ensures that the required ASP.NET Core dependencies are included when targeting .NET 8.0.



Consider refactoring the condition for improved readability:

```diff
-  <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' Or '$(TargetFramework)' == 'net6.0'  Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0'">
+  <ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' Or '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0'">

This removes the extra space before Or '$(TargetFramework)' == 'net7.0', making the condition more consistent and easier to read.

NewPlatform.Flexberry.ORM.ODataService/NewPlatform.Flexberry.ORM.ODataService.csproj (1)

18-21: Approved, but consider addressing the TODO comment.

The addition of 'net8.0' to the PropertyGroup condition is correct and consistent with the TargetFrameworks update. However, the TODO comment suggests that this might be a temporary solution.

Consider creating a separate task to address this TODO and implement a more permanent solution if needed.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between d68e938 and 5d2c75a.

📒 Files selected for processing (9)
  • NewPlatform.Flexberry.ORM.ODataService.Files/NewPlatform.Flexberry.ORM.ODataService.Files.csproj (2 hunks)
  • NewPlatform.Flexberry.ORM.ODataService.nuspec (2 hunks)
  • NewPlatform.Flexberry.ORM.ODataService/NewPlatform.Flexberry.ORM.ODataService.csproj (3 hunks)
  • NewPlatform.Flexberry.ORM.ODataServiceCore.Common/NewPlatform.Flexberry.ORM.ODataServiceCore.Common.csproj (2 hunks)
  • NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi/NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj (2 hunks)
  • README.md (1 hunks)
  • Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/NewPlatform.Flexberry.ORM.ODataService.Tests.csproj (3 hunks)
  • Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/README.md (1 hunks)
  • Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/docker-compose-mssql.yml (1 hunks)
🧰 Additional context used
🪛 LanguageTool
Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/README.md

[typographical] ~5-~5: Consider adding a comma here.
Context: ...ts run Postgres

Before start test run please fill connection string `ConnectionStrin...

(PLEASE_COMMA)


[grammar] ~10-~10: Did you mean “did not install”?
Context: ...tall Docker if it not installed yet and start Docker.

Then start Doc...

(PRP_NOT_VB)


[uncategorized] ~12-~12: You might be missing the article “the” here.
Context: ...ntainer with PostgreSQL by command from root folder this repository:

docke...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

---

[typographical] ~30-~30: Consider adding a comma here.
Context: ... Test run MSSQL

Before start test run please fill connection string `ConnectionStrin...

(PLEASE_COMMA)

---

[grammar] ~36-~36: Did you mean “did not install”?
Context: ...tall [Docker](https://docker.com) if it not installed yet and start Docker.

Then start Doc...

(PRP_NOT_VB)

---

[uncategorized] ~38-~38: Possible missing preposition found.
Context: ... SQL Server by command from root folder this repository:

```sh
docker-compose -f...

(AI_HYDRA_LEO_MISSING_IN)

</blockquote></details>

</blockquote></details>
<details>
<summary>🪛 yamllint</summary><blockquote>

<details>
<summary>Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/docker-compose-mssql.yml</summary><blockquote>

[error] 1-1: wrong new line character: expected \n

(new-lines)

---

[error] 13-13: trailing spaces

(trailing-spaces)

</blockquote></details>

</blockquote></details>

</details>

<details>
<summary>🔇 Additional comments (11)</summary><blockquote>

<details>
<summary>Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/docker-compose-mssql.yml (1)</summary><blockquote>

`14-15`: **LGTM: Volumes section is correctly configured.**

The volume 'mssqldb' is properly defined, following Docker Compose best practices for data persistence.

</blockquote></details>
<details>
<summary>NewPlatform.Flexberry.ORM.ODataServiceCore.Common/NewPlatform.Flexberry.ORM.ODataServiceCore.Common.csproj (2)</summary><blockquote>

`4-4`: **LGTM: Successfully added .NET 8 support**

The addition of `net8.0` to the `TargetFrameworks` property correctly implements support for .NET 8, which aligns with the PR objective.

---

Line range hint `1-34`: **Verify package compatibility with .NET 8**

While the changes to add .NET 8 support look good, it's important to ensure that all package references are compatible with .NET 8. 

Please run the following script to check the latest versions of the packages used in this project:



Review the output and update any packages if necessary to ensure full compatibility with .NET 8.

</blockquote></details>
<details>
<summary>NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi/NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj (1)</summary><blockquote>

`18-18`: **LGTM: Updated ItemGroup condition for .NET 8**

The update to the `ItemGroup` condition to include `net8.0` is correct and necessary. This ensures that the `Microsoft.AspNetCore.App` framework reference is included when targeting .NET 8, maintaining consistency across all supported frameworks.


To verify the framework references across all target frameworks, please run the following script:

```shell
#!/bin/bash
# Description: Verify framework references for all target frameworks

# Test: Check framework references for each target framework
rg --type xml '<FrameworkReference.*Include="Microsoft.AspNetCore.App"' NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj

# Test: Verify conditions for framework references
rg --type xml '<ItemGroup.*Condition.*TargetFramework' NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj

These commands will help ensure that the framework references are correctly configured for all target frameworks, including .NET 8.

NewPlatform.Flexberry.ORM.ODataService.Files/NewPlatform.Flexberry.ORM.ODataService.Files.csproj (2)

4-4: LGTM: Addition of .NET 8.0 target framework

The addition of net8.0 to the TargetFrameworks property is correct and aligns with the PR objective. This change allows the project to be built and run on .NET 8.0 while maintaining backward compatibility with other frameworks.


Line range hint 1-46: Verify compatibility with all target frameworks

The changes successfully add .NET 8.0 support to the project. However, it's important to ensure that the project builds and runs correctly on all target frameworks, including the newly added .NET 8.0.

Please run the following script to verify the project's compatibility:

This script will attempt to restore and build the project for each target framework. Review the output to ensure there are no build errors, especially for .NET 8.0.

NewPlatform.Flexberry.ORM.ODataService/NewPlatform.Flexberry.ORM.ODataService.csproj (3)

4-4: LGTM: .NET 8.0 target framework added successfully.

The addition of 'net8.0' to the TargetFrameworks property is correct and aligns with the PR objective. The order of frameworks is logical, and backward compatibility is maintained.


Line range hint 1-62: Summary: .NET 8.0 support successfully added

The changes to add .NET 8.0 support to the project file are consistent and well-implemented. All necessary sections have been updated to include the new target framework. However, please ensure to:

  1. Address the TODO comment in the PropertyGroup condition if needed.
  2. Verify the compatibility of the NewPlatform.Flexberry.AspNetCore.OData package with .NET 8.0.
  3. Confirm that the referenced NewPlatform.Flexberry.ORM.ODataServiceCore.Common project also supports .NET 8.0.

Overall, good job on expanding the project's compatibility!


57-59: LGTM, but verify .NET 8.0 support in the referenced project.

The addition of 'net8.0' to the ItemGroup condition for the NewPlatform.Flexberry.ORM.ODataServiceCore.Common project reference is correct and consistent with the TargetFrameworks update.

Please ensure that the NewPlatform.Flexberry.ORM.ODataServiceCore.Common project also supports .NET 8.0. You can verify this by checking its project file:

#!/bin/bash
# Description: Check if the NewPlatform.Flexberry.ORM.ODataServiceCore.Common project supports .NET 8.0

# Test: Search for .NET 8.0 support in the project file
grep -n "net8.0" ../NewPlatform.Flexberry.ORM.ODataServiceCore.Common/NewPlatform.Flexberry.ORM.ODataServiceCore.Common.csproj

# Note: If 'net8.0' is not found in the TargetFrameworks, consider updating that project as well.
Tests/NewPlatform.Flexberry.ORM.ODataService.Tests/NewPlatform.Flexberry.ORM.ODataService.Tests.csproj (2)

Line range hint 1-145: Summary of changes and recommendations.

  1. The addition of .NET 8.0 to the target frameworks aligns with the PR objective.
  2. The EnableUnsafeBinaryFormatterSerialization property raises security concerns and needs justification.
  3. The condition for including the Microsoft.AspNetCore.Mvc.Testing package needs to be corrected to remove the duplicate .NET 7.0 check.

Please address the identified issues. Once resolved, the changes will successfully add .NET 8 support to the project.


4-4: LGTM: Addition of .NET 8.0 target framework.

The addition of .NET 8.0 to the target frameworks aligns with the PR objective. This allows the project to be built and run on the latest .NET version.

Please ensure that all dependencies are compatible with .NET 8.0 and that the project builds and runs correctly on this new target framework. Consider running the following command to verify:

Comment on lines +4 to +12
mssql:
image: mcr.microsoft.com/mssql/server:2022-latest
environment:
- ACCEPT_EULA=Y
- MSSQL_SA_PASSWORD=p@ssw0rd
ports:
- 1433:1433
volumes:
- mssqldb:/var/opt/mssql
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve security and stability of the MSSQL service configuration.

The current configuration has two areas for improvement:

  1. The SA password is hardcoded, which is a security risk.
  2. The latest tag is used for the image, which might lead to unexpected changes in future builds.

Consider the following changes:

  1. Use an environment variable or Docker secret for the SA password:
 environment:
   - ACCEPT_EULA=Y
-  - MSSQL_SA_PASSWORD=p@ssw0rd
+  - MSSQL_SA_PASSWORD=${MSSQL_SA_PASSWORD}
  1. Use a specific version tag for the MSSQL image:
-image: mcr.microsoft.com/mssql/server:2022-latest
+image: mcr.microsoft.com/mssql/server:2022-CU10-ubuntu-20.04

Remember to add instructions in the project documentation on how to set the MSSQL_SA_PASSWORD environment variable securely.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mssql:
image: mcr.microsoft.com/mssql/server:2022-latest
environment:
- ACCEPT_EULA=Y
- MSSQL_SA_PASSWORD=p@ssw0rd
ports:
- 1433:1433
volumes:
- mssqldb:/var/opt/mssql
mssql:
image: mcr.microsoft.com/mssql/server:2022-CU10-ubuntu-20.04
environment:
- ACCEPT_EULA=Y
- MSSQL_SA_PASSWORD=${MSSQL_SA_PASSWORD}
ports:
- 1433:1433
volumes:
- mssqldb:/var/opt/mssql

Comment on lines +1 to +52
# Integration Tests

## Tests run Postgres

Before start test run please fill connection string `ConnectionStringPostgres` in `App.config` like this:

```xml
<add name="ConnectionStringPostgres" connectionString="SERVER=localhost;User ID=postgres;Password=p@ssw0rd;Port=5432;" />
```
Install [Docker](https://docker.com) if it not installed yet and start Docker.

Then start Docker container with PostgreSQL by command from root folder this repository:

```sh
docker-compose up -d
```

Tests ready to run. Do it now.

When the database is no longer needed for tests, run the command:

```sh
docker-compose down
```

Also perform undo in App.config before commit changes.

## Test run MSSQL

Before start test run please fill connection string `ConnectionStringMssql` in `App.config` like this:

```xml
<add name="ConnectionStringMssql" connectionString="SERVER=localhost;User ID=sa;Password=p@ssw0rd;" />
```

Install [Docker](https://docker.com) if it not installed yet and start Docker.

Then start Docker container with Microsoft SQL Server by command from root folder this repository:

```sh
docker-compose -f docker-compose-mssql.yml up -d
```

Tests ready to run. Do it now.

When the database is no longer needed for tests, run the command:

```sh
docker-compose down
```

Also perform undo in App.config before commit changes.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance README structure for improved navigation

The addition of the MSSQL section has made this README more comprehensive, which is great. To further improve its usability, consider the following suggestions:

  1. Add a brief introduction at the beginning of the file explaining the purpose of these integration tests.
  2. Include a table of contents with links to each section for easy navigation.
  3. Consider adding a "Prerequisites" section that lists common requirements (e.g., Docker installation) to avoid repetition in each database-specific section.
  4. If applicable, add a section on how to switch between databases or run tests for both databases.

Here's a suggested structure:

# Integration Tests

## Table of Contents
- [Introduction](#introduction)
- [Prerequisites](#prerequisites)
- [Running Tests with PostgreSQL](#running-tests-with-postgresql)
- [Running Tests with Microsoft SQL Server (MSSQL)](#running-tests-with-microsoft-sql-server-mssql)
- [Switching Between Databases](#switching-between-databases)

## Introduction
[Brief explanation of the integration tests and their purpose]

## Prerequisites
- Docker installed and running
- [Any other common requirements]

[Existing sections with minor improvements]

Would you like me to provide a more detailed example of this improved structure?

🧰 Tools
🪛 LanguageTool

[typographical] ~5-~5: Consider adding a comma here.
Context: ...ts run Postgres

Before start test run please fill connection string `ConnectionStrin...

(PLEASE_COMMA)


[grammar] ~10-~10: Did you mean “did not install”?
Context: ...tall Docker if it not installed yet and start Docker.

Then start Doc...

(PRP_NOT_VB)


[uncategorized] ~12-~12: You might be missing the article “the” here.
Context: ...ntainer with PostgreSQL by command from root folder this repository:

docke...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

---

[typographical] ~30-~30: Consider adding a comma here.
Context: ... Test run MSSQL

Before start test run please fill connection string `ConnectionStrin...

(PLEASE_COMMA)

---

[grammar] ~36-~36: Did you mean “did not install”?
Context: ...tall [Docker](https://docker.com) if it not installed yet and start Docker.

Then start Doc...

(PRP_NOT_VB)

---

[uncategorized] ~38-~38: Possible missing preposition found.
Context: ... SQL Server by command from root folder this repository:

```sh
docker-compose -f...

(AI_HYDRA_LEO_MISSING_IN)

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit -->

@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFrameworks>netcoreapp3.1;netstandard2.0;net6.0;net7.0</TargetFrameworks>
<TargetFrameworks>netcoreapp3.1;netstandard2.0;net6.0;net7.0;net8.0</TargetFrameworks>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Issues Found: Package references may not be compatible with .NET 8

The .csproj file does not contain package references conditioned for net8.0, and the following packages are using outdated versions:

  • Microsoft.AspNetCore.Mvc.Core Version="2.2.5"
  • Microsoft.AspNetCore.Mvc.Formatters.Json Version="2.2.0"
  • NewPlatform.Flexberry.LockService Version="3.0.0"

Please verify that these packages are compatible with .NET 8 or update them accordingly.

🔗 Analysis chain

LGTM: Addition of .NET 8 target framework

The addition of net8.0 to the TargetFrameworks property is correct and aligns with the PR objective. This change allows the project to target .NET 8, which is the latest LTS release of .NET, while maintaining compatibility with previous versions.

To ensure all dependencies are compatible with .NET 8, please run the following script:

If the first command returns no results and the second command shows package versions that are compatible with .NET 8, no further action is needed. Otherwise, consider updating the packages or adding .NET 8 specific references as necessary.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify package references for .NET 8 compatibility

# Test: Check if there are any package references specific to .NET 8
rg --type xml '<PackageReference.*Condition.*net8.0' NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj

# Test: List all package references to review their versions
rg --type xml '<PackageReference' NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj

Length of output: 438


Script:

#!/bin/bash
# Description: Locate the NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj file within the repository

# Search for the .csproj file by name
fd 'NewPlatform\.Flexberry\.ORM\.ODataServiceCore\.WebApi\.csproj' --type f

Length of output: 184


Script:

#!/bin/bash
# Description: Verify package references for .NET 8 compatibility using the correct .csproj file path

# Correct path to the .csproj file
CSPROJ_PATH="NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi/NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.csproj"

# Test: Check if there are any package references specific to .NET 8
rg --type xml '<PackageReference.*Condition.*net8.0' "$CSPROJ_PATH"

# Test: List all package references to review their versions
rg --type xml '<PackageReference' "$CSPROJ_PATH"

Length of output: 683

Comment on lines +49 to 51
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' Or '$(TargetFramework)' == 'netcoreapp3.1' Or '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0'">
<PackageReference Include="NewPlatform.Flexberry.AspNetCore.OData" Version="7.6.2" />
</ItemGroup>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

.NET 8.0 Compatibility Issue Identified

  • NewPlatform.Flexberry.ORM.ODataServiceCore/NewPlatform.Flexberry.ORM.ODataServiceCore.csproj does not support .NET 8.0.
🔗 Analysis chain

LGTM, but verify package compatibility with .NET 8.0.

The addition of 'net8.0' to the ItemGroup condition is correct and consistent with the TargetFrameworks update.

Please verify that the NewPlatform.Flexberry.AspNetCore.OData package (version 7.6.2) is compatible with .NET 8.0. You can check this by running the following script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the NewPlatform.Flexberry.AspNetCore.OData package is compatible with .NET 8.0

# Test: Search for the package information
dotnet list package NewPlatform.Flexberry.AspNetCore.OData

# Note: If the package is not found or doesn't support .NET 8.0, consider updating to a newer version that does.

Length of output: 269


Script:

#!/bin/bash
# Description: Find csproj files that reference NewPlatform.Flexberry.AspNetCore.OData package and verify .NET 8.0 compatibility

# Find all csproj files that include the specific PackageReference
csproj_files=$(fd -e csproj | xargs grep -l '<PackageReference Include="NewPlatform.Flexberry.AspNetCore.OData"')

# Check each csproj file for .NET 8.0 in TargetFramework or TargetFrameworks
for file in $csproj_files; do
  echo "Checking $file for .NET 8.0 compatibility..."
  if grep -q '<TargetFramework>.*net8\.0<\/TargetFramework>' "$file" || grep -q '<TargetFrameworks>.*net8\.0' "$file"; then
    echo ".NET 8.0 is supported in $file"
  else
    echo ".NET 8.0 is NOT supported in $file"
  fi
done

Length of output: 1916

@@ -40,7 +40,7 @@

### Доработка

Исправление ошибок приветствуется, технические детали можно выяснить в [чате](https://gitter.im/Flexberry/PlatformDevelopment) или непосредственно в описании Issue.
Исправление ошибок приветствуется, технические детали можно выяснить в [чате](https://t.me/flexberry) или непосредственно в описании Issue.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remaining Gitter Reference Found

A Gitter chat link is still present in README.md:

Please update this reference to align with the project's current communication channels.

🔗 Analysis chain

LGTM! Consider updating other Gitter references.

The update of the chat link from Gitter to Telegram is appropriate and consistent with the project's communication strategy. The surrounding text has been adjusted accordingly.

To ensure consistency throughout the document, please run the following script to check for any remaining references to Gitter:

If any results are returned, consider updating those references as well to maintain consistency.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to Gitter in the README.md file

# Test: Search for 'gitter' (case-insensitive) in README.md
rg -i 'gitter' README.md

Length of output: 88

@@ -12,6 +12,7 @@
<CodeAnalysisRuleSet>..\..\Flexberry.ruleset</CodeAnalysisRuleSet>
<AssemblyVersion>1.0.0.0</AssemblyVersion>
<FileVersion>1.0.0.0</FileVersion>
<EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security Concern: Enabling unsafe BinaryFormatter serialization.

The addition of <EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization> raises security concerns. The BinaryFormatter is considered unsafe and has been deprecated due to security vulnerabilities.

Please provide justification for enabling this feature. If it's necessary for backwards compatibility, consider adding a comment explaining the reason and any plans to remove it in the future. If possible, it's recommended to find alternative serialization methods that are more secure.

@@ -57,7 +58,7 @@
<PackageReference Include="Unity.Microsoft.DependencyInjection" Version="5.11.5" />
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' ">
<ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0' ">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix condition: Remove duplicate .NET 7.0 check and add .NET 8.0.

The inclusion of .NET 8.0 in the condition is correct. However, there's a duplication in the condition for .NET 7.0.

Please update the condition to remove the duplicate check for .NET 7.0 and include .NET 8.0:

-  <ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0' ">
+  <ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0' ">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0' ">
<ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' Or '$(TargetFramework)' == 'net7.0' Or '$(TargetFramework)' == 'net8.0' ">

Comment on lines +63 to +67
<group targetFramework=".NET8.0">
<dependency id="NewPlatform.Flexberry.AspNetCore.OData" version="7.6.2" />
<dependency id="NewPlatform.Flexberry.LockService" version="3.0.0" />
<dependency id="NewPlatform.Flexberry.ORM" version="7.2.0" />
<dependency id="Newtonsoft.Json" version="13.0.1" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use Correct Target Framework Moniker (TFM) for .NET 8

The targetFramework attribute should use the standard Target Framework Moniker. For .NET 8, the correct TFM is net8.0, not .NET8.0.

Apply this change to correct the TFM:

-          <group targetFramework=".NET8.0">
+          <group targetFramework="net8.0">

Also, consider reviewing other <group> elements to ensure all targetFramework attributes use the correct TFM notation for consistency and compliance with NuGet conventions.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<group targetFramework=".NET8.0">
<dependency id="NewPlatform.Flexberry.AspNetCore.OData" version="7.6.2" />
<dependency id="NewPlatform.Flexberry.LockService" version="3.0.0" />
<dependency id="NewPlatform.Flexberry.ORM" version="7.2.0" />
<dependency id="Newtonsoft.Json" version="13.0.1" />
<group targetFramework="net8.0">
<dependency id="NewPlatform.Flexberry.AspNetCore.OData" version="7.6.2" />
<dependency id="NewPlatform.Flexberry.LockService" version="3.0.0" />
<dependency id="NewPlatform.Flexberry.ORM" version="7.2.0" />
<dependency id="Newtonsoft.Json" version="13.0.1" />

Comment on lines +121 to +128
<file src="NewPlatform.Flexberry.ORM.ODataService\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataService.dll" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataService.dll" />
<file src="NewPlatform.Flexberry.ORM.ODataService\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataService.xml" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataService.xml" />
<file src="NewPlatform.Flexberry.ORM.ODataService.Files\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataService.Files.dll" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataService.Files.dll" />
<file src="NewPlatform.Flexberry.ORM.ODataService.Files\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataService.Files.xml" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataService.Files.xml" />
<file src="NewPlatform.Flexberry.ORM.ODataServiceCore.Common\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.Common.dll" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.Common.dll" />
<file src="NewPlatform.Flexberry.ORM.ODataServiceCore.Common\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.Common.xml" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.Common.xml" />
<file src="NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.dll" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.dll" />
<file src="NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi\bin\$configuration$\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.xml" target="lib\net8.0\NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.xml" />
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Action Required: Verification Failed for .NET 8.0 Files

The verification script reported that all specified net8.0 build output files are missing. Please ensure that the build process correctly generates these files in the expected directories.

🔗 Analysis chain

Confirm File Entries for .NET 8.0

The added <file> entries for the net8.0 target framework appear correctly specified and consistent with existing entries for other frameworks.

Run the following script to verify that all necessary files for net8.0 exist in the expected directories:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify existence of net8.0 build outputs.

# Expected Result: All listed files should be present.

declare -a files=(
  "NewPlatform.Flexberry.ORM.ODataService/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataService.dll"
  "NewPlatform.Flexberry.ORM.ODataService/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataService.xml"
  "NewPlatform.Flexberry.ORM.ODataService.Files/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataService.Files.dll"
  "NewPlatform.Flexberry.ORM.ODataService.Files/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataService.Files.xml"
  "NewPlatform.Flexberry.ORM.ODataServiceCore.Common/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataServiceCore.Common.dll"
  "NewPlatform.Flexberry.ORM.ODataServiceCore.Common/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataServiceCore.Common.xml"
  "NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.dll"
  "NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi/bin/\$configuration\$/net8.0/NewPlatform.Flexberry.ORM.ODataServiceCore.WebApi.xml"
)

for file in "${files[@]}"; do
  if [ ! -f "$file" ]; then
    echo "Missing file: $file"
  else
    echo "Found file: $file"
  fi
done

Length of output: 4652

Copy link

sonarcloud bot commented Oct 9, 2024

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.

2 participants