Anonymous | Login
Project:
RSS
  
News | My View | View Issues | Roadmap | Summary

View Issue Details[ Jump to Notes ] [ Issue History ] [ Print ]
ID
0033101
TypeCategorySeverityReproducibilityDate SubmittedLast Update
feature request[Retail Modules] Web POSminorhave not tried2016-06-01 12:162016-06-29 17:36
ReportermtaalView Statuspublic 
Assigned Tomtaal 
PrioritynormalResolutionfixedFixed in Version
StatusclosedFix in branchFixed in SCM revisioncaa052599230
ProjectionnoneETAnoneTarget VersionRR16Q3
OSAnyDatabaseAnyJava version
OS VersionDatabase versionAnt version
Product VersionSCM revision 
Review Assigned ToAugustoMauch
Regression level
Regression date
Regression introduced in release
Regression introduced by commit
Triggers an Emergency PackNo
Summary

0033101: Several improvements to Multi-Server: all prefs as milli-seconds, separate thread for online transition

DescriptionThis issue combines several improvements in the multi-server behavior:
- use request retry time out also on server
- wait short time between retrying connection
- do transition to online in separate thread
- don't cache server definition, but cache the server definition id
- make sure all multi-server time prefs are in milli-seconds and have sensible defaults
Steps To ReproduceCheck multi-server code
TagsNo tags attached.
Attached Files

- Relationships [ Relation Graph ] [ Dependency Graph ]

-  Notes
(0087015)
hgbot (developer)
2016-06-06 08:52

 Repository: erp/pmods/org.openbravo.mobile.core
Changeset: 6a6f980e374af7fabf739bda3998edcf0682135d
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Mon Jun 06 08:51:09 2016 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d [^]

Fixes issue 33101: Several improvements to Multi-Server: all prefs as milli-seconds, separate thread for online transition
AD_REF_LIST.xml --> change description of prefs to show that milliseconds should be entered
MobileMainServerCheck.java --> use milliseconds, use request interval pref, prevent connection leak
MobileServerController.java --> make sure that also in admin mode a server can be found, cache this server id, not the server instance, move transition to online logic to separate thread
MobileServerRequestExecutor.java --> wait 50ms between retries, re-use code, solve issue that status was not recomputed
ServerStateBackground.java --> if store server only check main server, if main server only check store servers

---
M src-db/database/sourcedata/AD_REF_LIST.xml
M src/org/openbravo/mobile/core/servercontroller/MobileMainServerCheck.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/RetrieveMobileServerStatus.java
M src/org/openbravo/mobile/core/servercontroller/ServerStateBackground.java
---
(0087257)
AugustoMauch (manager)
2016-06-14 16:49

 Code review:

* [1] Use constant attributed instead of hardcoded numbers
* [2] Comment is wrong, it says it defaults back to 30 seconds, but it actually is defaulting to 60. Better not even specifying the default in the comment as that might change, leaving the comment outdated
* If possible refactor the run method, it is currently 140+ lines long which makes it difficult to understand
* [3] This comment seems outdated too


[1] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d#l2.29 [^]
[2] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d#l2.43 [^]
[3] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/file/6a6f980e374a/src/org/openbravo/mobile/core/servercontroller/MobileMainServerCheck.java#l25 [^]
(0087259)
AugustoMauch (manager)
2016-06-14 17:21

 More:
* [4] Wrong comment, it returns an ID not a MobileServerDefinition
* [5] getThisServerDefinition (returns a MobileServerDefinition) uses getThisServerDefinitionId (obtains a MobileServerDefinition and returns its id). Doing an OBQuery to obtain a MobileServerDefinition, then return its id and then using a OBDal.getInstance.get() to obtain the MobileServerDefinition seems overcomplex. The getThisServerDefinitionId is only used by getThisServerDefinition
* [6] Consider refactoring TransitionToOnline.run to make it more readable. Add a comment to clarify when do the handlers need to be retried.
* [7] Add Javadoc to TryConnection
* Overload the TryConnection method with a signature that does not require entering the number of previous attempts.
* Add Javadoc to handleOtherServerStatus
* [8] After add part to the where clause (store servers notify central servers of their statuses), is the previous queryPart [9] also needed?

[4] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d#l3.124 [^]
[5] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d#l3.106 [^]
[6] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d#l3.271 [^]
[7] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/file/6a6f980e374a/src/org/openbravo/mobile/core/servercontroller/MobileServerRequestExecutor.java#l113 [^]
[8] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d#l6.18 [^]
[9] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/file/6a6f980e374a/src/org/openbravo/mobile/core/servercontroller/ServerStateBackground.java#l59 [^]
(0087324)
hgbot (developer)
2016-06-16 08:37

 Repository: erp/pmods/org.openbravo.mobile.core
Changeset: caa052599230fd23f9b798c2b17c08ae8df02249
Author: Martin Taal <martin.taal <at> openbravo.com>
Date: Thu Jun 16 08:36:30 2016 +0200
URL: http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/caa052599230fd23f9b798c2b17c08ae8df02249 [^]

Fixes issue 33101: Several improvements to Multi-Server: all prefs as milli-seconds, separate thread for online transition
Improvements after code review

MobileMainServerCheck:
- improved class javadoc
- moved values to constants
- offline check should not check own server, added this in the whereclause in the main loop
- moved code from main loop to separate method to improve readability

MobileServerController:
- updated javadoc
- transition to online code: moved code to separate method to improve readability

MobileServerRequestExecutor:
- implemented tryConnection without retries parameter
- updated javadoc
- solved error in whereclause (forgotten quote)

MultiServerJSONProcess:
- log the exception

---
M src/org/openbravo/mobile/core/servercontroller/MobileMainServerCheck.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/MultiServerJSONProcess.java
---
(0088086)
AugustoMauch (manager)
2016-06-29 17:36

 Code reviewed and verified in [1]@caa052599230

[1] https://code.openbravo.com/erp/pmods/org.openbravo.mobile.core [^]

- Issue History
Date Modified Username Field Change
2016-06-01 12:16 mtaal New Issue
2016-06-01 12:16 mtaal Assigned To => mtaal
2016-06-01 12:16 mtaal Triggers an Emergency Pack => No
2016-06-06 08:52 hgbot Checkin
2016-06-06 08:52 hgbot Note Added: 0087015
2016-06-06 08:52 hgbot Status new => resolved
2016-06-06 08:52 hgbot Resolution open => fixed
2016-06-06 08:52 hgbot Fixed in SCM revision => http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d [^]
2016-06-13 16:51 mtaal Review Assigned To => AugustoMauch
2016-06-14 16:49 AugustoMauch Note Added: 0087257
2016-06-14 17:21 AugustoMauch Note Added: 0087259
2016-06-14 17:21 AugustoMauch Status resolved => new
2016-06-14 17:21 AugustoMauch Resolution fixed => open
2016-06-16 08:37 hgbot Checkin
2016-06-16 08:37 hgbot Note Added: 0087324
2016-06-16 08:37 hgbot Status new => resolved
2016-06-16 08:37 hgbot Resolution open => fixed
2016-06-16 08:37 hgbot Fixed in SCM revision http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/6a6f980e374af7fabf739bda3998edcf0682135d [^] => http://code.openbravo.com/erp/pmods/org.openbravo.mobile.core/rev/caa052599230fd23f9b798c2b17c08ae8df02249 [^]
2016-06-29 17:36 AugustoMauch Note Added: 0088086
2016-06-29 17:36 AugustoMauch Status resolved => closed

Copyright © 2000 - 2009 MantisBT Group
Powered by Mantis Bugtracker