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

Feature/changelog with categories #59

Merged
merged 39 commits into from
Jun 22, 2020
Merged

Conversation

tiwalter
Copy link
Collaborator

No description provided.

tiwalter added 16 commits May 10, 2020 18:23
- Implement the FSReleaseArtifactService function ValidateUploadPayload()
- Implement an extension method that validates the DeploymentMetaInfo
- Extend the TestUtils
- Extend the DeploymentMetaInfoMapper to support a mapping of byte arrays
- Write unit tests for the new implementations
- Rename the former "Common" namespaces to "Extensions"
- Add test data for the new unit tests
- Add the ReleaseNotesModel
- Implement a release note parser (Json to ReleaseNotesModel)
- Implement an extension method that converts the release notes dictionary to a proper response dictionary
- Add test release notes
- Use the ReleaseNotesModel in the ReleaseInformationModel & ProductInformationModel
- Implement retrieving the release notes with the ProductInformation
- releaseNotes.txt is now releaseNotes.json
- fill the releaseNotes.json with valid input
- The tests will check the handling of the ReleaseNotesModel
- Implement an extension method that checks the json validity of a byte array
- Add the json validation to the ValidateUploadPayload() function
- Implement a helper function that converts a stream to a byte array
- Add test data
- Write unit tests
…log-with-categories'

- Resolve merge conflicts
@tiwalter tiwalter added this to the Release Server v0.2 milestone May 17, 2020
@tiwalter tiwalter requested a review from f-porter May 17, 2020 22:47
@tiwalter tiwalter linked an issue May 17, 2020 that may be closed by this pull request
tiwalter added 2 commits May 18, 2020 01:22
- The method is now generic
- The check handles now JsonSerializationExceptions
- Add a new test that checks the structure of the release notes
- Add test data
- Added a TODO for a further investigation, why the exception message in the Travis-CI build system is different to the local system.
Copy link
Contributor

@f-porter f-porter left a comment

Choose a reason for hiding this comment

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

I have seen that you implemented a lot of "Mapper" and "ResponseModel" classes to control the way how data is serialized to and deserialized from JSON. Though these are not introduced with this PR I find it vital to address this issue now as these classes are resulting in a lot of maintenance effort.

To control the serialization of an object one usually uses so called custom converters that are registered in the serializer/deserializer settings or passed as list during serialization/deserialization.
This way you have to describe how to process a specific type only once and if you need to change it you can do these in one place.

In our case we want for example control how a DateTime object is serialized. For this purpose we implement a custom converter DateTimeConverter:

public class DateTimeJsonConverter : JsonConverter<DateTime>
{
    public override void WriteJson(JsonWriter writer, DateTime value, JsonSerializer serializer)
    {
        // implementation goes here
    }
    public override DateTime ReadJson(JsonReader reader, Type objectType, DateTime existingValue, bool hasExistingValue, JsonSerializer serializer)
    {
        // implementation goes here
    }
}

To use this converter for every object that is returned by the ReleaseArtifactController we have to register it. This is done in the method ConfigureServices(IServiceCollection services) of class Startup. As you are using the Newtonsoft Json package instead of the new System.Text.Json which is the new default library for ASP.NET Core 3 you have to explicitly add it, too:

services
    .AddControllers()
    .AddNewtonsoftJson(options =>
    {
        options.SerializerSettings.Converters.Add(new DateTimeJsonConverter());
    });

This custom converter is now used for every serialization and deserialization by the ASP.NET Core MVC framework (for deserializing action parameters and serializing action responses).

For deserializing an object from a file like in the method ParseDeploymentMetaInfo() of DeploymentMetaInfoMapper you either need to pass the converter as parameter to the method JsonConvert.DeserializeObject() or pass JsonSerializerSettings that contain this converter. I would prefer the latter one as this enables you to centrally define these JsonSerializerSettings, reuse them and reduce the effort to adapt these settings.

…ture/changelog-with-categories'

- Resolve merge conflicts
- Use a StreamReader for the release note validation
- Adjust unit tests
@tiwalter tiwalter requested a review from f-porter May 31, 2020 10:42
tiwalter added 12 commits May 31, 2020 13:13
- Remove the DeploymentMetaInfoMapper incl. unit tests
- Rename properties of the ProductInformation
- Use the generic json deserializer
- Rename the PlatformsResponse to PlatformsList
- Remove the PlatformsMapper incl. unit tests
- Eliminate the ProductInformationResponses incl mapper & unit tests
- Eliminate the ReleaseInformationResponse incl. mapper & unit tests
- Separate the ChangeSet class
- Eliminate the ZipArchiveMapper
- Implement a ProductVersionConverter
- The FsReleaseArtifactService & FsReleaseArtifactRepository are now handling ProductVersions instead of strings
- Remove obsolete mappers
- Adjust unit tests
- Adjust swagger examples
- Adjust the controller
- Write a TestUtils function CreateTestZipFile()
- Delete the test ZIP files and create them programmatically in the unit tests
… & new unit tests added

- Make FromJsonFile() static
- Write unit tests that check the failing deserialization of DeploymentMetaInfo
…ture/changelog-with-categories'

- Resolve merge conflicts
@f-porter f-porter merged commit dda4f5a into master Jun 22, 2020
@f-porter f-porter deleted the feature/changelog-with-categories branch June 22, 2020 15:20
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.

Changelog with categories, affected platforms and internationalization
2 participants