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

[Environment API Spark]: Compute MergedEnvironmentConfig etag using Objects.hash #5812

Merged
merged 4 commits into from
Feb 8, 2019

Conversation

bdpiprava
Copy link
Contributor

@bdpiprava bdpiprava commented Feb 7, 2019

Issue link: #5807

Summary

Environment coming from config repo is represented using MergedEnvironmentConfig. The EntityHashingService is calculating hashes only for the config entities. AsMergedEnvironmentConfig is not a config entity it fails to detect @ConfigTag annotation and fails to generate md5.

In ruby, we calculate the md5 on generated JSON payload for index calls.

Fix

Calculate md5 using object hash.

  • This is done as Environment coming from config repo represented using
    MergedEnvironmentConfig object and calculating Etag for none config
    the entity is not supported by entity hashing service.

  • Here we have used Objects.hash(...) which generates an identical hash
    for the same valued objects.

  • Sort the EnvironmentConfigs by name to guaranty the order in the response as well as for Etag computation.

- Use `getEnvironments` method instead of `getAllMergedEnvironments` to
  get all environment for index call.

- Use `getEnvironmentConfig` instead of `getMergedEnvironmentforDisplay`
  to find environment by name.
- This is done as Environment coming from config repo represeted using
  MergedEnvironmentConfig object and caculating etag for none config
  entity is not supported by entity  hashing service.

- Here we have used `Objects.hash(...)` which generates identical hash
  for same valued objects.
@bdpiprava bdpiprava merged commit 6c4da91 into gocd:master Feb 8, 2019
@bdpiprava bdpiprava deleted the issue-5807 branch February 8, 2019 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant