EN-7804: api and components versioning #96

bogdan-rosianu · 2020-10-12T17:02:26Z

Testing procedure:

start proxy as usual
for all proxy clients, append a /v1.0 in the URL. Example: http://127.0.0.1:8080 => http://127.0.0.1:8080/v1.0
everything should work the same

…mponents

# Conflicts: # api/api.go # cmd/proxy/main.go # process/baseProcessor.go # process/mock/processorStub.go

api/groups/baseAccountsGroup.go

api/groups/baseTransactionGroup.go

api/groups/baseVmValuesGroup.go

iulianpascalau · 2020-10-30T10:02:59Z

api/groups/v1_2/interface.go

+import "github.com/ElrondNetwork/elrond-proxy-go/data"
+
+// AccountsFacadeHandlerV1_2 interface defines methods that can be used from facade context variable
+type AccountsFacadeHandlerV1_2 interface {


Why do we need to have a single facade implementation that will satisfy all the interfaces?
The question arose from seeing the function GetShardIDForAddressV1_2.
We can have multiple facade implementations that will be consumed by various endpoint versions. Having all functions in a single monstrous facade implementation will impact struct size (which should be kept as small as possible)

Do you mean that we should drop the facade entity that holds all the processors and calls the appropriate methods of them?

facade/versions/elrondProxyFacadeV1_0.go

versions/factory/versionedFacadeCreator.go

iulianpascalau · 2020-10-30T10:25:51Z

versions/interface.go

@@ -0,0 +1,8 @@
+package versions
+
+//// VersionManagerHandler defines the actions that a version manager implementation has to do


remove or uncomment

versions/versionManager.go

api/groups/v1_2/accountsGroupV1_2.go

iulianpascalau

Minor stuff remained. Code looks way better. GJ! 👍

iulianpascalau · 2020-11-11T09:20:45Z

api/apiHandler.go

-func NewCommonApiHandler() *commonApiHandler {
-	return &commonApiHandler{
-		groups: initBaseGroups(),
+func NewApiHandler(facade data.FacadeHandler) (*commonApiHandler, error) {


missing comment

iulianpascalau · 2020-11-11T09:30:50Z

api/groups/baseAccountsGroup.go

@@ -58,8 +63,8 @@ func getTransactions(c *gin.Context) ([]data.DatabaseTransaction, int, error) {

 // GetAccount returns an accountResponse containing information
 // about the account correlated with provided address
-func GetAccount(c *gin.Context) {
-	account, status, err := getAccount(c)
+func (ag *accountsGroup) GetAccount(c *gin.Context) {


Wondering if we still need these methods exported.

made them unexported

iulianpascalau · 2020-11-11T09:33:20Z

api/groups/baseVmValuesGroup.go

 	}
+
+	baseRoutesHandlers := map[string]*data.EndpointHandlerData{
+		"/hex":    {Handler: vvg.getHex, Method: http.MethodPost},


unexported methods 👍

iulianpascalau · 2020-11-11T09:36:34Z

cmd/proxy/main.go

 }

 func createVersionManager(
 	cfg *config.Config,
 	ecConf *erdConfig.EconomicsConfig,
 	exCfg *erdConfig.ExternalConfig,
 	pemFileLocation string,
+	isRosettaOn bool,


isRosettaEnabled (On can mean something else :D )

renamed to isRosettaModeEnabled

iulianpascalau · 2020-11-11T09:38:47Z

facade/versions/elrondProxyFacadeV1_2.go

+	*facade.ElrondProxyFacade
+}
+
+func (epf *ElrondProxyFacadeV1_2) GetShardIDForAddressV1_2(address string, additionalField int) (uint32, error) {


iulianpascalau · 2020-11-11T09:40:03Z

process/v1_0/transactionProcessorV1_0.go

+	"github.com/ElrondNetwork/elrond-proxy-go/process"
+)
+
+type TransactionProcessorV1_0 struct {


Some elements without comments remained in this file

iulianpascalau · 2020-11-11T09:41:42Z

versions/versionManager.go

 	}

-	vm.mutVersions.Lock()
+	vm.Lock()


andreibancioiu · 2020-11-10T10:07:22Z

api/api.go

@@ -35,7 +27,10 @@ func CreateServer(elrondProxyFacade ElrondProxyHandler, port int) (*http.Server,
 		return nil, err
 	}

-	registerRoutes(ws, elrondProxyFacade)
+	err = registerRoutes(ws, versionManager)


Optionally, reorder the definitions of registerRoutes, registerValidators, with respect to the order of their usage.

Now I see (older code) - why do we need skValidator? CC: @iulianpascalau?

AFAIK, skValidator is used in the authentication & authorization process when accessing a REST API endpoint.

reordered. it's not used now, but can remain as a placeholder for further authorized requests

andreibancioiu · 2020-11-10T10:09:55Z

api/api.go

-	blockAtlasRoutes := ws.Group("/block-atlas")
-	blockAtlasRoutes.Use(WithElrondProxyFacade(elrondProxyFacade))
-	blockatlas.Routes(blockAtlasRoutes)
+	for version, versionData := range versionsMap {


andreibancioiu · 2020-11-10T10:13:13Z

api/api.go

-	blockAtlasRoutes := ws.Group("/block-atlas")
-	blockAtlasRoutes.Use(WithElrondProxyFacade(elrondProxyFacade))
-	blockatlas.Routes(blockAtlasRoutes)
+	for version, versionData := range versionsMap {


Optionally, we can rename VersionData (the struct, the variables) to VersionBundle or VersionController. But In this sense, perhaps also rename VersionManager to ApiVersioningRegistry or VersionsRegistry.

I think the current naming is better because VersionData only holds the specific components for each version and it doesn't have any functions.
However, renamed VersionManager to VersionsRegistry

api/api.go

api/apiHandler.go

api/groups/baseBlockAtlasGroup.go

api/groups/baseGroup.go

api/groups/interface.go

process/v1_0/transactionProcessorV1_0.go

iulianpascalau · 2020-11-16T13:32:37Z

api/api.go

@@ -86,11 +85,3 @@ func skValidator(
 ) bool {
 	return true
 }
-
-// WithElrondProxyFacade middleware will set up an ElrondFacade object in the gin context
-func WithElrondProxyFacade(elrondProxyFacade ElrondProxyHandler, version string) gin.HandlerFunc {


iulianpascalau · 2020-11-24T13:25:33Z

api/groups/v_next/accountsGroupV_next.go

+	})
+}
+
+// GetShardForAccountV_next is an example of an updated endpoint in the version v_next


iulianpascalau · 2020-11-24T13:26:10Z

process/v_next/doc.go

@@ -1,2 +1,2 @@
 // Package v1.2 represents the processors needed for the version v1.2 of the API


maybe update the comment?

# Conflicts: # api/address/interface.go # api/groups/baseAccountsGroup.go # api/groups/baseTransactionGroup.go # api/transaction/interface.go

andreibancioiu · 2020-12-07T09:20:33Z

README.md

+- `/v1.0/hyperblock/by-hash/:hash`    (GET) --> returns a hyperblock by hash, with transactions included
+
+# V_next


andreibancioiu · 2020-12-07T09:21:24Z

Docker/Dockerfile

@@ -5,12 +5,13 @@ COPY . .
 # Proxy
 WORKDIR /elrond/cmd/proxy
 RUN go build
+# $(ls /go/pkg/mod/github.com/!elrond!network/ | grep arwen-wasm-vm | sed 's/.* //' | head -1) this command is required  to be able to extract the full name of the directory that starts with 'arwen-wasm-wm'


Not sure why Github picked these as PR changes.

andreibancioiu · 2020-12-07T09:21:37Z

api/errors/errors.go

@@ -14,12 +14,21 @@ var ErrGetValueForKey = errors.New("get value for key error")
 // ErrComputeShardForAddress signals an error in computing the shard ID for a given address
 var ErrComputeShardForAddress = errors.New("compute shard ID for address error")

+// ErrGetESDTTokenData signals an error in fetching an ESDT token data
+var ErrGetESDTTokenData = errors.New("cannot get ESDT token data")


Not sure why Github picked these as PR changes.

andreibancioiu · 2020-12-07T09:22:41Z

data/transaction.go

-	Status                            transaction.TxStatus `json:"status,omitempty"`
-	HyperblockNonce                   uint64               `json:"hyperblockNonce,omitempty"`
-	HyperblockHash                    string               `json:"hyperblockHash,omitempty"`
+	Type                              string                                `json:"type"`


Not sure why Github picked these as PR changes.

andreibancioiu · 2020-12-07T09:23:37Z

go.mod

@@ -3,10 +3,9 @@ module github.com/ElrondNetwork/elrond-proxy-go
 go 1.12

 require (
-	github.com/ElrondNetwork/elrond-go v1.1.2-0.20201016080150-915a6ef433de
+	github.com/ElrondNetwork/elrond-go v1.1.6-0.20201126175543-3e5a9696c46f


v1.1.10+?

andreibancioiu · 2020-12-07T09:23:56Z

process/accountProcessor.go

@@ -100,6 +100,67 @@ func (ap *AccountProcessor) GetValueForKey(address string, key string) (string,
 	return "", ErrSendingRequest
 }

+// GetESDTTokenData returns the token data for a token with the given name
+func (ap *AccountProcessor) GetESDTTokenData(address string, key string) (*data.GenericAPIResponse, error) {


Not sure why Github picked these as PR changes.

andreibancioiu · 2020-12-07T09:24:44Z

process/transactionProcessor.go

@@ -455,19 +465,42 @@ func (tp *TransactionProcessor) getTxFromObservers(txHash string, reqType reques
 	return nil, errors.ErrTransactionNotFound
 }

-func (tp *TransactionProcessor) getTxWithSenderAddr(txHash, sender string) (*data.FullTransaction, error) {
+func (tp *TransactionProcessor) alterTxWithScResultsFromSourceIfNeeded(txHash string, tx *data.FullTransaction, withResults bool) *data.FullTransaction {


Not sure why Github picked these as PR changes.

# Conflicts: # README.md

bogdan-rosianu added 6 commits October 12, 2020 19:59

EN-7804: api and components versioning

aded703

EN-7804: removed unused files

cb38182

EN-7804: extracted routes registration functions and groups to new co…

7e0651a

…mponents

EN-7804: versioned facades

bb37515

EN-7804: refactoring & moved common structs & interfaces to data package

0e08d2a

changes + example when adding a breaking changes endpoint

f4973f9

andreibancioiu assigned bogdan-rosianu Oct 26, 2020

andreibancioiu requested review from iulianpascalau and andreibancioiu October 26, 2020 17:32

bogdan-rosianu added 2 commits October 30, 2020 11:40

Merge branch 'development' into EN-7804-versioning

e5ee60e

# Conflicts: # api/api.go # cmd/proxy/main.go # process/baseProcessor.go # process/mock/processorStub.go

fix after merge

1ea7f2a

iulianpascalau requested changes Oct 30, 2020

View reviewed changes

bogdan-rosianu and others added 3 commits November 9, 2020 17:24

fix after review

fe75d05

fix linter issues

cfd4a52

Merge branch 'development' into EN-7804-versioning

de13776

iulianpascalau reviewed Nov 11, 2020

View reviewed changes

fix after review, moved tests, removed unused api packages

3b34a4c

bogdan-rosianu changed the title ~~[WIP] EN-7804: api and components versioning~~ EN-7804: api and components versioning Nov 11, 2020

bogdan-rosianu marked this pull request as ready for review November 11, 2020 14:57

andreibancioiu reviewed Nov 13, 2020

View reviewed changes

fix after review

f8b4331

iulianpascalau previously approved these changes Nov 16, 2020

View reviewed changes

Merge branch 'development' into EN-7804-versioning

cebf512

andreibancioiu previously approved these changes Nov 16, 2020

View reviewed changes

moved everything to v1.0 and created v_next for examples

8e2ad5e

bogdan-rosianu dismissed stale reviews from andreibancioiu and iulianpascalau via 8e2ad5e November 23, 2020 14:26

iulianpascalau reviewed Nov 24, 2020

View reviewed changes

bogdan-rosianu added 2 commits November 24, 2020 18:29

fix after review

0105744

Merge branch 'development' into EN-7804-versioning

78327a2

# Conflicts: # api/address/interface.go # api/groups/baseAccountsGroup.go # api/groups/baseTransactionGroup.go # api/transaction/interface.go

fix after merge

00feec2

iulianpascalau previously approved these changes Dec 7, 2020

View reviewed changes

andreibancioiu previously approved these changes Dec 7, 2020

View reviewed changes

Merge branch 'development' into EN-7804-versioning

e3702a0

# Conflicts: # README.md

bogdan-rosianu dismissed stale reviews from andreibancioiu and iulianpascalau via e3702a0 December 11, 2020 12:09

andreibancioiu previously approved these changes Dec 11, 2020

View reviewed changes

added default version

ab37581

bogdan-rosianu dismissed andreibancioiu’s stale review via ab37581 December 15, 2020 09:25

andreibancioiu approved these changes Dec 15, 2020

View reviewed changes

iulianpascalau approved these changes Dec 15, 2020

View reviewed changes

LucianMincu approved these changes Dec 16, 2020

View reviewed changes

LucianMincu merged commit 5eda4f5 into development Dec 16, 2020

andreibancioiu deleted the EN-7804-versioning branch December 16, 2020 15:34

andreibancioiu mentioned this pull request Dec 17, 2020

Merge Development into Main #116

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EN-7804: api and components versioning #96

EN-7804: api and components versioning #96

bogdan-rosianu commented Oct 12, 2020 •

edited

iulianpascalau Oct 30, 2020

bogdan-rosianu Nov 9, 2020

iulianpascalau Oct 30, 2020

iulianpascalau left a comment

iulianpascalau Nov 11, 2020

bogdan-rosianu Nov 11, 2020

iulianpascalau Nov 11, 2020

bogdan-rosianu Nov 11, 2020

iulianpascalau Nov 11, 2020

bogdan-rosianu Nov 11, 2020

iulianpascalau Nov 11, 2020

bogdan-rosianu Nov 11, 2020

iulianpascalau Nov 11, 2020

iulianpascalau Nov 11, 2020

iulianpascalau Nov 11, 2020

andreibancioiu Nov 10, 2020

andreibancioiu Nov 10, 2020

iulianpascalau Nov 16, 2020

bogdan-rosianu Nov 16, 2020

andreibancioiu Nov 10, 2020

andreibancioiu Nov 10, 2020

bogdan-rosianu Nov 16, 2020

iulianpascalau Nov 16, 2020

iulianpascalau Nov 24, 2020

iulianpascalau Nov 24, 2020

bogdan-rosianu Nov 24, 2020

andreibancioiu Dec 7, 2020

andreibancioiu Dec 7, 2020

andreibancioiu Dec 7, 2020

andreibancioiu Dec 7, 2020

andreibancioiu Dec 7, 2020

andreibancioiu Dec 7, 2020

andreibancioiu Dec 7, 2020

		@@ -0,0 +1,8 @@
		package versions

		//// VersionManagerHandler defines the actions that a version manager implementation has to do

		@@ -1,2 +1,2 @@
		// Package v1.2 represents the processors needed for the version v1.2 of the API

		- `/v1.0/hyperblock/by-hash/:hash` (GET) --> returns a hyperblock by hash, with transactions included

		# V_next

EN-7804: api and components versioning #96

EN-7804: api and components versioning #96

Conversation

bogdan-rosianu commented Oct 12, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iulianpascalau left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdan-rosianu commented Oct 12, 2020 •

edited