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

Multilple configs export #3088

Merged

Conversation

Anilople
Copy link
Contributor

@Anilople Anilople commented May 20, 2020

What's the purpose of this PR

Enhance the PR #1767

Which issue(s) this PR fixes:

issues

#1760
#1681

Brief changelog

Export multiple configs to a single .zip file.

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.

@codecov-commenter
Copy link

codecov-commenter commented May 20, 2020

Codecov Report

Merging #3088 into master will decrease coverage by 0.26%.
The diff coverage is 25.68%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3088      +/-   ##
============================================
- Coverage     51.55%   51.28%   -0.27%     
- Complexity     2255     2275      +20     
============================================
  Files           433      439       +6     
  Lines         13447    13622     +175     
  Branches       1378     1385       +7     
============================================
+ Hits           6932     6986      +54     
- Misses         6036     6150     +114     
- Partials        479      486       +7     
Impacted Files Coverage Δ Complexity Δ
...ip/framework/apollo/portal/entity/bo/ConfigBO.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ramework/apollo/portal/util/ConfigToFileUtils.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...framework/apollo/portal/util/NamespaceBOUtils.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...rk/apollo/portal/service/ConfigsExportService.java 12.30% <12.30%> (ø) 2.00 <2.00> (?)
...rk/apollo/portal/service/ConfigsImportService.java 15.62% <15.62%> (ø) 1.00 <1.00> (?)
...llo/portal/controller/ConfigsExportController.java 21.73% <23.07%> (+13.57%) 2.00 <2.00> (+1.00)
...llo/portal/controller/ConfigsImportController.java 37.50% <37.50%> (ø) 1.00 <1.00> (?)
.../framework/apollo/portal/util/ConfigFileUtils.java 64.58% <64.58%> (ø) 13.00 <13.00> (?)
...mework/apollo/portal/service/NamespaceService.java 67.30% <75.00%> (+0.20%) 27.00 <2.00> (ø)
...ervice/service/ReleaseMessageServiceWithCache.java 85.88% <0.00%> (-1.18%) 24.00% <0.00%> (-1.00%)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0341e5c...25fd38d. Read the comment docs.

@nobodyiam
Copy link
Member

@Anilople That's all right, we may just focus on the config export function in this pull request

@Anilople Anilople changed the title Multilple configs import and export Multilple configs export Jul 5, 2020
this.format = null;
}

public ConfigBO(Env env, String ownerName, String appId, String clusterName,
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this constructor is not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change the constructor's logic. com.ctrip.framework.apollo.portal.entity.bo.ConfigBO#ConfigBO(com.ctrip.framework.apollo.portal.environment.Env, java.lang.String, java.lang.String, java.lang.String, com.ctrip.framework.apollo.portal.entity.bo.NamespaceBO) use it now.

  public ConfigBO(Env env, String ownerName, String appId, String clusterName, NamespaceBO namespaceBO) {
    this(env, ownerName, appId, clusterName,
        namespaceBO.getBaseInfo().getNamespaceName(),
        NamespaceBOUtils.convert2configFileContent(namespaceBO),
        ConfigFileFormat.fromString(namespaceBO.getFormat())
    );
  }

apollo-portal/src/main/resources/static/i18n/en.json Outdated Show resolved Hide resolved
apollo-portal/src/main/resources/static/i18n/en.json Outdated Show resolved Hide resolved
@@ -34,18 +34,20 @@
<li value="zh-CN"><a href="javascript:void(0)" ng-click="changeLanguage('zh-CN')">简体中文</a></li>
</ul>
</li>
<li class="dropdown" ng-if="hasRootPermission">
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest adding another drop down menu e.g. System Tools and leave the Admin Tools only for Admin Users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Add other menu in position 1 ?

How to name new menu better? tool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use different menu name when user is admin or non-admin.

For admin user

image

For non-admin user

image

How about this implementation?

Copy link
Member

Choose a reason for hiding this comment

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

This looks good

* @param outputStream receive zip file output stream
* @throws IOException
*/
private static void writeAsZipOutputStream(Stream<ConfigBO> configBOStream, OutputStream outputStream)
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessarily marked as static?

Copy link
Contributor Author

@Anilople Anilople Jul 24, 2020

Choose a reason for hiding this comment

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

The method write Stream<ConfigBO>'s content to an OutputStream.

Everytime the method is invoked, those parameters are different with before, and not use field in object,

so I think use static for this method is better.

* @param configBO a namespace represent
* @return zip file output stream same as parameter zipOutputStream
*/
private static ZipOutputStream write2ZipOutputStream(
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessarily marked as static?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same reson as above.

apollo-portal/src/main/resources/static/i18n/zh-CN.json Outdated Show resolved Hide resolved
apollo-portal/src/main/resources/static/i18n/en.json Outdated Show resolved Hide resolved
@Anilople Anilople requested a review from nobodyiam July 24, 2020 15:18
Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

OK

@nobodyiam nobodyiam merged commit f8f3706 into apolloconfig:master Jul 25, 2020
@Anilople Anilople added the area/portal apollo-portal label Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/portal apollo-portal enhancement feature Categorizes issue as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants