change: remove ID of consumer #1745

nic-chen · 2021-04-11T07:42:27Z

Please answer these questions before submitting a pull request, or your PR will get closed.

Why submit this pull request?

Bugfix
New feature provided
Improve performance
Backport patches

APISIX has removed the consumer id in version 2.5, see: apache/apisix#3868.

Dashboard also needs to remove the consumer id before it can be used with APISIX 2.5 and above.

What changes will this PR take into?

Sync schema from APISIX 2.5 version
Remove the id field from the consumer's struct
Modify related test cases

Related issues

close #1742

Checklist:

Did you explain what problem does this PR solve? Or what new features have been added?
Have you added corresponding test cases?
Have you modified the corresponding document?
Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

# Conflicts: # api/test/docker/docker-compose.yaml

netlify · 2021-04-11T11:44:13Z

Deploy preview for apisix-dashboard ready!

Built with commit 15d1ce5

https://deploy-preview-1745--apisix-dashboard.netlify.app

starsz

It seems we don't modify E2E test.
Do we need?

starsz · 2021-04-11T14:16:36Z

api/test/e2e/route_with_plugin_jwt_test.go

+	// sleep for process log
+	time.Sleep(1500 * time.Millisecond)
+	logContent := ReadAPISIXErrorLog(t)
+	fmt.Println("logContent:", logContent)


This is debugging info?

yes, removed.

starsz · 2021-04-11T14:17:31Z

api/test/docker/apisix_config.yaml

@@ -22,7 +22,7 @@ etcd:
    - "http://172.16.238.10:2379"
    - "http://172.16.238.11:2379"
    - "http://172.16.238.12:2379"
-  resync_delay: 0.1  # sync data from etcd quickly for e2e test
+  resync_delay: 1  # sync data from etcd quickly for e2e test


Do we need to modify this?

sure. APISIX allow only int for this configuration now.

starsz · 2021-04-11T14:20:10Z

api/internal/handler/consumer/consumer_test.go

 				assert.True(t, args.Bool(2))
 			}).Return(tc.giveRet, tc.giveErr)

+			mStore.On("Get", mock.Anything).Run(func(args mock.Arguments) {
+			}).Return(nil, nil)


We should test if the Get returns a consumer.

codecov-io · 2021-04-11T14:37:00Z

Codecov Report

Merging #1745 (389f93e) into master (8b6080d) will decrease coverage by 20.10%.
The diff coverage is 60.00%.

@@             Coverage Diff             @@
##           master    #1745       +/-   ##
===========================================
- Coverage   72.45%   52.34%   -20.11%     
===========================================
  Files         132       38       -94     
  Lines        5721     2661     -3060     
  Branches      649        0      -649     
===========================================
- Hits         4145     1393     -2752     
+ Misses       1334     1080      -254     
+ Partials      242      188       -54

Flag	Coverage Δ
backend-e2e-test	`?`
backend-e2e-test-ginkgo	`?`
backend-unit-test	`52.34% <60.00%> (+0.05%)`	⬆️
frontend-e2e-test	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
api/internal/core/entity/entity.go	`0.00% <ø> (-100.00%)`	⬇️
api/internal/handler/consumer/consumer.go	`66.66% <60.00%> (-24.83%)`	⬇️
api/internal/utils/version.go	`0.00% <0.00%> (-100.00%)`	⬇️
api/internal/filter/request_id.go	`0.00% <0.00%> (-100.00%)`	⬇️
api/internal/core/store/storehub.go	`0.00% <0.00%> (-74.77%)`	⬇️
api/internal/filter/cors.go	`0.00% <0.00%> (-66.67%)`	⬇️
api/internal/filter/schema.go	`0.00% <0.00%> (-55.47%)`	⬇️
api/internal/utils/consts/api_error.go	`0.00% <0.00%> (-50.00%)`	⬇️
api/internal/handler/data_loader/route_import.go	`27.41% <0.00%> (-37.50%)`	⬇️
... and 120 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 8b6080d...389f93e. Read the comment docs.

nic-chen · 2021-04-12T01:36:29Z

@juzhiyuan @LiteSun
FE CI failed, please help to check, thanks.

juzhiyuan · 2021-04-12T02:07:52Z

@nic-chen Please take a look #1752, once it's merged, please resync your branch. Thanks!

nic-chen · 2021-04-12T07:08:13Z

@nic-chen Please take a look #1752, once it's merged, please resync your branch. Thanks!

After synchronizing the code, it still failed. Does FE need to make responsive changes?

juzhiyuan · 2021-04-12T07:33:03Z

This test failed due to raw data editor 🤔 It's the first time that failed, if it still failed after rerun, then need @LiteSun's help.

juzhiyuan · 2021-04-12T07:35:38Z

test-rawDataEditor.spec.js.mp4

@nic-chen I see what happened, the test failed due to Network error, does this PR changes some fields or?

juzhiyuan · 2021-04-12T07:36:09Z

see 00:34

nic-chen · 2021-04-12T07:42:40Z

does this PR changes some fields or?

remove ID of consumer. @juzhiyuan

nic-chen · 2021-04-15T07:14:51Z

FE CI still failed, please have a look when you have time. thanks. @LiteSun

juzhiyuan · 2021-04-15T08:58:23Z

cecb6cf

Wait for this PR cecb6cf or take the above change :)

juzhiyuan · 2021-04-16T21:52:19Z

@LiteSun You may need to take some changes with FE & API.

codecov-commenter · 2021-04-17T05:42:41Z

Codecov Report

❗ No coverage uploaded for pull request base (master@f1b8f0c). Click here to learn what that means.
The diff coverage is 52.63%.

@@            Coverage Diff            @@
##             master    #1745   +/-   ##
=========================================
  Coverage          ?   71.69%           
=========================================
  Files             ?      172           
  Lines             ?     6049           
  Branches          ?      699           
=========================================
  Hits              ?     4337           
  Misses            ?     1468           
  Partials          ?      244

Flag	Coverage Δ
backend-e2e-test	`62.04% <27.77%> (?)`
backend-e2e-test-ginkgo	`48.87% <16.66%> (?)`
backend-unit-test	`52.41% <50.00%> (?)`
frontend-e2e-test	`71.95% <100.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
api/internal/core/entity/entity.go	`100.00% <ø> (ø)`
api/internal/core/store/store_mock.go	`0.00% <0.00%> (ø)`
api/internal/handler/consumer/consumer.go	`91.07% <69.23%> (ø)`
web/src/components/Plugin/PluginDetail.tsx	`62.23% <100.00%> (ø)`

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 f1b8f0c...15d1ce5. Read the comment docs.

api/test/docker/apisix_config2.yaml

…bator-apisix-dashboard into remove-consumer-id

johzchen added 8 commits April 11, 2021 14:33

chore: sync json schema from APISIX 2.5

0868120

change: remove ID from consumer

ea5b929

chore: run e2e with APISIX 2.5

8956ea5

fix: backend unit test

213b244

test: debug

0d3a354

fix: CI failed

d1a4661

Merge branch 'master' into remove-consumer-id

f9c1578

# Conflicts: # api/test/docker/docker-compose.yaml

fix: CI failed

3ab1ff2

johzchen added 2 commits April 11, 2021 21:58

test: debug

18ea8fc

fix: remove debug

6882a94

starsz reviewed Apr 11, 2021

View reviewed changes

test: debug

389f93e

fix: test

34e716b

Merge branch 'master' into remove-consumer-id

afe4cbb

johzchen added 2 commits April 14, 2021 10:55

fix: error

da43607

fix: the unit test cannot cover the default sorting function

4fc721e

nic-chen requested review from starsz, imjoey, gxthrj, Jaycean and ShiningRush April 14, 2021 10:33

nic-chen requested a review from tokers April 14, 2021 10:33

tokers approved these changes Apr 14, 2021

View reviewed changes

Merge branch 'master' into remove-consumer-id

10b4ed3

Merge branch 'master' into remove-consumer-id

d724aca

fix: FE CI

cdf2ade

LiteSun and others added 2 commits April 17, 2021 13:47

fix: FE CI

8a23ada

fix review

6d2ab28

starsz reviewed Apr 17, 2021

View reviewed changes

api/test/docker/apisix_config2.yaml Show resolved Hide resolved

johzchen and others added 4 commits April 18, 2021 00:32

fix: invalid schema

91b6bde

feat: disable delete and submit in route step4

7a27942

Merge branch 'remove-consumer-id' of https://github.com/nic-chen/incu…

59fdcec

…bator-apisix-dashboard into remove-consumer-id

fix: FE CI

15d1ce5

gxthrj approved these changes Apr 19, 2021

View reviewed changes

starsz approved these changes Apr 19, 2021

View reviewed changes

nic-chen merged commit 5d9cd3f into apache:master Apr 19, 2021

nic-chen deleted the remove-consumer-id branch April 19, 2021 03:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change: remove ID of consumer #1745

change: remove ID of consumer #1745

nic-chen commented Apr 11, 2021 •

edited

Loading

netlify bot commented Apr 11, 2021 •

edited

Loading

starsz left a comment

starsz Apr 11, 2021

nic-chen Apr 12, 2021

starsz Apr 11, 2021

nic-chen Apr 12, 2021 •

edited

Loading

starsz Apr 11, 2021

codecov-io commented Apr 11, 2021

nic-chen commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

nic-chen commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

nic-chen commented Apr 12, 2021

nic-chen commented Apr 15, 2021

juzhiyuan commented Apr 15, 2021

juzhiyuan commented Apr 16, 2021

codecov-commenter commented Apr 17, 2021 •

edited

Loading

change: remove ID of consumer #1745

change: remove ID of consumer #1745

Conversation

nic-chen commented Apr 11, 2021 • edited Loading

netlify bot commented Apr 11, 2021 • edited Loading

starsz left a comment

Choose a reason for hiding this comment

starsz Apr 11, 2021

Choose a reason for hiding this comment

nic-chen Apr 12, 2021

Choose a reason for hiding this comment

starsz Apr 11, 2021

Choose a reason for hiding this comment

nic-chen Apr 12, 2021 • edited Loading

Choose a reason for hiding this comment

starsz Apr 11, 2021

Choose a reason for hiding this comment

codecov-io commented Apr 11, 2021

Codecov Report

nic-chen commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

nic-chen commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

juzhiyuan commented Apr 12, 2021

nic-chen commented Apr 12, 2021

nic-chen commented Apr 15, 2021

juzhiyuan commented Apr 15, 2021

juzhiyuan commented Apr 16, 2021

codecov-commenter commented Apr 17, 2021 • edited Loading

Codecov Report

nic-chen commented Apr 11, 2021 •

edited

Loading

netlify bot commented Apr 11, 2021 •

edited

Loading

nic-chen Apr 12, 2021 •

edited

Loading

codecov-commenter commented Apr 17, 2021 •

edited

Loading