Hi all, [update] After completing the above suggestions, we needed to handle the case of "if a user changes the tenant domain showed in the query param to an invalid domain and visits the store directly"
To address this issue after an offline discussion we added an error code to the REST API [1], This error code will be returned when a REST API call is made with an invalid domain in the x-wso2-tenant header. After checking for this error code in [2] [3] [4] , we set the tenant domain in the context as INVALID, This case will be checked in [5]. If this case is satisfied tenant listing page will be shown to the user again PR for this feature can be found in [6] [1] - https://github.com/wso2/carbon-apimgt/blob/8c139b28baef0543fc6b6bebf19a860c67f77e33/components/apimgt/org.wso2.carbon.apimgt.api/src/main/java/org/wso2/carbon/apimgt/api/ExceptionCodes.java#L282 [2] - https://github.com/wso2/carbon-apimgt/blob/8c139b28baef0543fc6b6bebf19a860c67f77e33/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/components/Apis/Listing/ApiTableView.jsx#L170 [3] - https://github.com/wso2/carbon-apimgt/blob/8c139b28baef0543fc6b6bebf19a860c67f77e33/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/components/Apis/Details/index.jsx#L232 [4] - https://github.com/wso2/carbon-apimgt/blob/8c139b28baef0543fc6b6bebf19a860c67f77e33/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/components/LandingPage/ApisWithTag.jsx#L69 [5] - https://github.com/wso2/carbon-apimgt/blob/8c139b28baef0543fc6b6bebf19a860c67f77e33/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/ProtectedApp.jsx#L214 [6] - https://github.com/wso2/carbon-apimgt/pull/7000 On Fri, Sep 6, 2019 at 10:15 PM Tanya Madurapperuma <[email protected]> wrote: > > > On Fri, Sep 6, 2019 at 4:45 PM Dushan Silva <[email protected]> wrote: > >> Hi Kasun, >> >> >>> - We already have a query string parser/formatter library *qs >>> >>> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/package.json#L42> >>> *shall >>> we use that library in here[1 >>> >>> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52> >>> ][2 >>> >>> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56>] >>> too ? is there any limitations with that library ? >>> >>> no limitation, since query-string was already available in our package I >> used it, we can use qs too. I think both can be used here. >> > There is a minor advantage of using query-string instead of qs where we > can parse the location.search prop directly into query string without > removing the ? > *const parsed = queryString.parse(location.search);* > > But for qs we either have to replace the ? with ' ' like we have done in > [1] or specifically mention to ignore the ? like *var prefixed = > qs.parse('?a=b&c=d', { ignoreQueryPrefix: true });* > > [1] > https://github.com/wso2/carbon-apimgt/blob/master/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/ProtectedApp.jsx#L152 > > Thanks, > Tanya > >> >> Shall we move the BrowserRouter.jsx file to >>> */store-new/source/src/app/components/Base/* directory, Since this file >>> is a React component, it should come under components directory and there >>> we could move it to Base components directory? >> >> Shall we add the props validation in the original BrowserRouter >>> <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25>[4] >>> implementation? >> >> >> +1, Will do that >> >> >> >> On Fri, Sep 6, 2019 at 11:16 AM Kasun Thennakoon <[email protected]> >> wrote: >> >>> Hi Dushan, >>> >>> First, great work on implementing the multi-tenancy support to Store >>> SPA, This improvement will allow users to directly access a tenant >>> store,share tenant store URL etc. >>> I have a few suggestions on the implementation. >>> >>> - We already have a query string parser/formatter library *qs >>> >>> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/package.json#L42> >>> *shall >>> we use that library in here[1 >>> >>> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52> >>> ][2 >>> >>> <https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56>] >>> too ? is there any limitations with that library ? >>> - Shall we move the BrowserRouter.jsx file to >>> */store-new/source/src/app/components/Base/* directory, Since this >>> file is a React component, it should come under components directory and >>> there we could move it to Base components directory? >>> - Shall we add the props validation in the original BrowserRouter >>> >>> <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25>[4] >>> implementation? >>> - And also I think we now need to keep an eye on the original >>> (react-router-dom) BrowserRouter.js >>> >>> <https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25> >>> implementation, >>> To absorb their changes to our component. >>> >>> >>> [1]: >>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L52 >>> [2]: >>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L56 >>> [3]: >>> https://github.com/dushansilva/carbon-apimgt/tree/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/components/Base >>> [4]: >>> https://github.com/ReactTraining/react-router/blob/master/packages/react-router-dom/modules/BrowserRouter.js#L19-L25 >>> >>> Thanks >>> ~KasunTe >>> >>> On Thu, Sep 5, 2019 at 3:29 PM Dushan Silva <[email protected]> wrote: >>> >>>> Hi all, >>>> I have been working on $Subject. While working on it we came across the >>>> following requirement. >>>> >>>> In the previous store implementation the tenant domain is passed on the >>>> url as a query param and when either the page is refresh or if the url is >>>> directly accessed with the query param tenant specific details are loaded. >>>> However in the current react based single page implementation all the >>>> routing is handle via react router. Here if we initially plan on keeping >>>> the tenant domain in the react context api as a global setting. Once the >>>> app is loaded routing can be handled easily by taking the tenant domain >>>> from the context api. >>>> >>>> The problem is when the app is refreshed or if a url is visited >>>> directly, at this occasion the context api is empty and the user will >>>> always be redirect to the tenant listing. To handle this issue after >>>> considering some input from the react community [1] , we came up with the >>>> solution of writing an interceptor for the react router. This would require >>>> us to extend the currently react router implementation and its history >>>> implementation to plugin our custom interceptor. An advantage of the >>>> interceptor approach is that this saves the developer the hassle handling >>>> both tenant and normal cases each and every route path already added and >>>> paths to be added in the future. The flow will be as follows. >>>> >>>> When the store is initially loaded react router should be initialized >>>> [2]. Rather than using the default Router component by react router we will >>>> be using our custom component BrowserRouter [3]. In this we place the >>>> interceptor to be invoked when we are navigating around (push or replace) >>>> [4]. The interceptor [5] will take the tenant domain from the context and >>>> append it to the path. So in each route (when the user navigates to >>>> different pages this will be appended as a query param. >>>> >>>> To retrieve the tenant specific data from the REST API , we will be >>>> getting the tenant domain from the query param and setting it to the >>>> x-wso2-tenant header [6]. >>>> >>>> Any thoughts and feedback on this much appreciated. >>>> >>>> >>>> [1] - >>>> https://stackoverflow.com/questions/57757263/how-to-write-an-interceptor-for-react-router/57757921#57757921 >>>> [2] - >>>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/App.jsx#L96 >>>> [3] - >>>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L15 >>>> [4] - >>>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L28 >>>> [5] - >>>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/CustomRouter/BrowserRouter.jsx#L43 >>>> [6] - >>>> https://github.com/dushansilva/carbon-apimgt/blob/5603333b774c10b74c2709c6bf5b9c6ac20ccca6/features/apimgt/org.wso2.carbon.apimgt.store.feature/src/main/resources/store-new/source/src/app/data/APIClient.jsx#L163 >>>> -- >>>> Best Regards >>>> Dushan Silva >>>> Software Engineer >>>> >>>> *WSO2, Inc. * >>>> >>>> lean . enterprise . middleware >>>> Mob: +94 774 979042 >>>> >>> >>> >>> -- >>> *Kasun Thennakoon* | Senior Software Engineer | WSO2 Inc. >>> (m) +94 711661919 | (w) +94 11 214 5345 | (e) [email protected] >>> GET INTEGRATION AGILE >>> Integration Agility for Digitally Driven Business >>> >> >> >> -- >> Best Regards >> Dushan Silva >> Software Engineer >> >> *WSO2, Inc. * >> >> lean . enterprise . middleware >> Mob: +94 774 979042 >> _______________________________________________ >> Architecture mailing list >> [email protected] >> https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture >> > > > -- > *Tanya Madurapperuma* | Technical Lead | WSO2 Inc. > (m) +94718184439 | (e) [email protected] > > <http://wso2.com/signature> > > > _______________________________________________ > Architecture mailing list > [email protected] > https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture > -- Best Regards Dushan Silva Software Engineer *WSO2, Inc. * lean . enterprise . middleware Mob: +94 774 979042
_______________________________________________ Architecture mailing list [email protected] https://mail.wso2.org/cgi-bin/mailman/listinfo/architecture
