Project:
| View Issue Details[ Jump to Notes ] | [ Issue History ] [ Print ] | |||||||
| ID | ||||||||
| 0032681 | ||||||||
| Type | Category | Severity | Reproducibility | Date Submitted | Last Update | |||
| feature request | [Retail Modules] Web POS | minor | have not tried | 2016-04-15 17:32 | 2016-05-18 16:38 | |||
| Reporter | mtaal | View Status | public | |||||
| Assigned To | mtaal | |||||||
| Priority | normal | Resolution | fixed | Fixed in Version | RR16Q3 | |||
| Status | closed | Fix in branch | Fixed in SCM revision | 490a2e8132e2 | ||||
| Projection | none | ETA | none | Target Version | RR16Q3 | |||
| OS | Any | Database | Any | Java version | ||||
| OS Version | Database version | Ant version | ||||||
| Product Version | SCM revision | |||||||
| Review Assigned To | AugustoMauch | |||||||
| Regression level | ||||||||
| Regression date | ||||||||
| Regression introduced in release | ||||||||
| Regression introduced by commit | ||||||||
| Triggers an Emergency Pack | No | |||||||
| Summary | 0032681: Provide a webservice which returns the offline/online status of a store | |||||||
| Description | The webservice should be available on the central and store server. On the store server it can return its current status from the mobile server table. From the central server it should check the mobile server status table and check how old the last update there is. If it is not too old (configurable preference) then the status from the table should be returned. If too old then the store server should be called with the same webservice to retrieve the current online status. The webservice should be documented in the store server wiki pages. | |||||||
| Steps To Reproduce | See description | |||||||
| Tags | No tags attached. | |||||||
| Attached Files | ||||||||
 Relationships [ Relation Graph ]
[ Dependency Graph ]
|
|
 Relationships |
|
Notes |
|
|
(0085765) mtaal (manager) 2016-04-20 09:23 |
Docs: http://wiki.openbravo.com/wiki/Store_Server_Webservices#Retrieve_Store_Server_Status [^] |
|
(0085809) hgbot (developer) 2016-04-21 08:12 |
Repository: erp/pmods/org.openbravo.mobile.core Changeset: 490a2e8132e21f1cfd205e01598ddc2825fd3393 Author: Martin Taal <martin.taal <at> openbravo.com> Date: Wed Apr 20 09:29:26 2016 +0200 URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/490a2e8132e21f1cfd205e01598ddc2825fd3393 [^] Fixes issue 32681: Provide a webservice which returns the offline/online status of a store Adds webservice and other improvements for updating the status --- M src-db/database/sourcedata/AD_REF_LIST.xml M src/org/openbravo/mobile/core/process/JSONProcessSimple.java M src/org/openbravo/mobile/core/servercontroller/MobileServerController.java M src/org/openbravo/mobile/core/servercontroller/MobileServerRequestExecutor.java M src/org/openbravo/mobile/core/servercontroller/MobileServerUtils.java --- |
|
(0085810) hgbot (developer) 2016-04-21 08:12 |
Repository: erp/pmods/org.openbravo.mobile.core Changeset: b1b8b84cfabb57bc4799600cfa13a098be1f2e57 Author: Martin Taal <martin.taal <at> openbravo.com> Date: Wed Apr 20 15:48:48 2016 +0200 URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/b1b8b84cfabb57bc4799600cfa13a098be1f2e57 [^] Related to issue 32681: Provide a webservice which returns the offline/online status of a store Added missing classes --- A src/org/openbravo/mobile/core/servercontroller/MobileServerStatusUpdateHandler.java A src/org/openbravo/mobile/core/servercontroller/RetrieveMobileServerStatus.java --- |
|
(0086449) mtaal (manager) 2016-05-15 09:26 |
Code review comments and reaction, next push will contain changes: >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> I finally did the code review of https://issues.openbravo.com/view.php?id=32681. [^] I have a few comments: 1) Three methods (executeServerRequest, executeCentralRequest, executeRequest) have this comment: * This method performs so-called store server state handling. If the calling server can not be * reached then this server can decide to go offline this depends on the * {@link MobileServerDefinition#isTriggerstate()} value of the called server. But at no point they set the status of the server to false if the request fails, I think. MT>> is done through the tryConnection used by all three of them. I adapted the comment to reflect this. 2) There is a lot of duplicated code in the executeServerRequest and executeCentralRequest methods, it could make sense to refactor them. MT>> indeed! I refactored this 3) The getCentralServer method adds this (MobileServerDefinition.PROPERTY_MOBILESERVERKEY + "!='" + thisServerDef.getMobileServerKey()) to the where clause, that results in the method returning null if called from the central server. MT>> Yes is on purpose, I added this to the comment. The reason is that this method is used to call the central server. If a server starts doing requests to itself then we will get infinite loops which are really hard to detect. I added this as a comment. 4) Shouldn't that method be placed in the MobileServerController class? That class has similar methods, like getThisServerDefinition. The same applies for the getMobileServer method. MT>> yes changed it 5) Wrong log here. MT>> Changed 6) Regarding the getMobileServerForStore method. Why returning only the first store server that applies? Isn't it a little arbitrary? Also I did not see that the method was used anywhere in the changesets related with the issue. MT>> removed it 7) Removing the getRetriesPreference is an API change (I think those are not checked automatically in retail). MT>> it was introduced in this release so not yet in official api, so can be removed without breaking api 8) Duplicated code in StatusUpdateRunnable.run(). It would be better if that method called getMobileServer. Maybe it would make sense to cache the mobile server definitions if the method is called often enough. MT>> yes changed to call getMobileServer 9) Just out of curiosity, what locking problems happened in this code? Not saying that is wrong, only that I would not have been able to detect them myself. MT>> The status is updated for each request, there can be many simultaneous requests, and they will update a server def at the same time in maybe somewhat longer transactions. So there is a risk that multiple threads will update the mobile server table. |
|
(0086453) hgbot (developer) 2016-05-16 00:55 |
Repository: erp/pmods/org.openbravo.mobile.core Changeset: dc7dba5a385b425f3d6fe2741beedd3e85ed2c03 Author: Martin Taal <martin.taal <at> openbravo.com> Date: Sun May 15 09:32:20 2016 +0200 URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/dc7dba5a385b425f3d6fe2741beedd3e85ed2c03 [^] Related to issue 32681: Provide a webservice which returns the offline/online status of a store Changes for code review MobileCoreLoginUtilsServlet.java: do preference reading in utils class MobileServerController.java: several new utility methods used by webservice call MobileServerRequestExecutor.java: updated comments to reflect inner working of methods, refactor to prevent code duplication MobileServerStatusUpdateHandler.java: use utility method to prevent code duplication MobileServerUtils.java: move server-related methods to MobileServerController class --- M src/org/openbravo/mobile/core/login/MobileCoreLoginUtilsServlet.java M src/org/openbravo/mobile/core/servercontroller/MobileServerController.java M src/org/openbravo/mobile/core/servercontroller/MobileServerRequestExecutor.java M src/org/openbravo/mobile/core/servercontroller/MobileServerStatusUpdateHandler.java M src/org/openbravo/mobile/core/servercontroller/MobileServerUtils.java --- |
|
(0086555) AugustoMauch (manager) 2016-05-18 16:38 |
Code reviewed and verified in [1]@dc7dba5a385b [1] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core [^] |
|
Notes |
|
Issue History |
|||
| Date Modified | Username | Field | Change |
| 2016-04-15 17:32 | mtaal | New Issue | |
| 2016-04-15 17:32 | mtaal | Assigned To | => mtaal |
| 2016-04-15 17:32 | mtaal | Triggers an Emergency Pack | => No |
| 2016-04-20 09:23 | mtaal | Note Added: 0085765 | |
| 2016-04-21 08:12 | hgbot | Checkin | |
| 2016-04-21 08:12 | hgbot | Note Added: 0085809 | |
| 2016-04-21 08:12 | hgbot | Status | new => resolved |
| 2016-04-21 08:12 | hgbot | Resolution | open => fixed |
| 2016-04-21 08:12 | hgbot | Fixed in SCM revision | => http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/490a2e8132e21f1cfd205e01598ddc2825fd3393 [^] |
| 2016-04-21 08:12 | hgbot | Checkin | |
| 2016-04-21 08:12 | hgbot | Note Added: 0085810 | |
| 2016-05-03 14:52 | mtaal | Review Assigned To | => AugustoMauch |
| 2016-05-15 09:26 | mtaal | Note Added: 0086449 | |
| 2016-05-16 00:55 | hgbot | Checkin | |
| 2016-05-16 00:55 | hgbot | Note Added: 0086453 | |
| 2016-05-18 16:38 | AugustoMauch | Note Added: 0086555 | |
| 2016-05-18 16:38 | AugustoMauch | Status | resolved => closed |
| 2016-05-18 16:38 | AugustoMauch | Fixed in Version | => RR16Q3 |
|
Issue History |
| Copyright © 2000 - 2009 MantisBT Group |
 |