View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0029822 | Community | OCCT:Application Framework | public | 2018-05-29 03:49 | 2020-12-26 14:52 |
| Reporter | Vico Liang | Assigned To | bugmaster | ||
| Priority | normal | Severity | feature | ||
| Status | closed | Resolution | fixed | ||
| Target Version | 7.6.0 | Fixed in Version | 7.6.0 | ||
| Summary | 0029822: Application Framework - Make TDocStd_Document extensible | ||||
| Description | Document creation mechanism is sealed inside the OCAF framework. There is no way or convenient way to subclass TDocStd_Document to cooperate with OCAF. When design application, it's unavoidable to extend class TDocStd_Document. There are several places create document: void TDocStd_Application::NewDocument(const TCollection_ExtendedString& format,Handle(TDocStd_Document)& aDoc) Handle(CDM_Document) BinLDrivers_DocumentRetrievalDriver::CreateDocument() Handle(CDM_Document) XmlLDrivers_DocumentRetrievalDriver::CreateDocument() It would be more convenient if the NewDocument method is center controlled inside TDocStd_Application, so we can override the document creation method in one place. | ||||
| Steps To Reproduce | Dear MPV, could you review the changes, please: http://vm-jenkins-test-12.nnov.opencascade.com:8080/view/CR29822_2-master-VRO/view/COMPARE/ A new method NewDocument() is added (by your advice). It solves all warnings of old compilers. OCCT: CR29822_2 Products: nothing | ||||
| Tags | No tags attached. | ||||
| Test case number | Not required | ||||
|
|
We don't expect or advise the developer extends TDocStd_Document. It is a data container that provides standard (and generic) methods for working with data, suitable for any application. If you want to implement application-specific interfaces to the document, you simply may create a new interface-class that whould keep the TDocStd_Document as a field. One of the advantages here is that this higher-level interface does not provide an access low-level methods of Document to the user of this object. |
|
|
>We don't expect or advise the developer extends TDocStd_Document. I don't think we should limit the user to do so. It's no any problem for user to extend TDocStd_Document as long as there is single entrance to create user defined Document. Another problem is that there are several entrance to create document, it's hard to maintain the consistence. I suggest to remove the these two document creation methods: Handle(CDM_Document) BinLDrivers_DocumentRetrievalDriver::CreateDocument(); Handle(CDM_Document) XmlLDrivers_DocumentRetrievalDriver::CreateDocument(); And add a virtual method as below: Handle(CDM_Document) TDocStd_Application::NewDocument(); With this minor changes, will make the interface simpler and maintainable and also user can extend document class by just override this method in application. |
|
|
CreateDocument are virtual methods. If they are removed, we lose possibility for custom schema (thar inherits retriveal drivers) to create document in a specific way. Actually, this is not as simple as you wrote because of dependencies and current usage of this mechanism. I would ask you to prepare an OCCT patch that is good for your application and then we will decide is this approach usefull in OCCT as a standard behavior. |
|
|
> CreateDocument are virtual methods. If they are removed, > we lose possibility for >custom schema (thar inherits retriveal drivers) > to create document in a specific way. It will cause mismatching if the document type created in retrieval drivers is different from the document type in TDocStd_Application::NewDocument() method. For an application, it should make sure that all document created are the same type including the retrieval driver. That's to say, the storage driver and the retrieval driver should use the same document type. |
|
|
I searched the whole source code tree there is not a case redefined method CreateDocument() in retrieval drivers, all use class TDocStd_Document. It should not be a problem to upgrade user's program with such changes. |
|
|
"to create document in a specific way" is not the same as creation of new type of the document. There could be new attributes added, some labels structure prepared, etc. The custom way of document creation is out of the OCCT scope this is an application-specific functionality. |
|
|
but you can't imagine that user won't define a new document type. |
|
|
A good mechanism will prevent user making mistake and guide them in the right direction. |
|
|
What is the added value from a new document type? There are different ways to change content of the document (like creation of new kinds of attributes). Normally application-specific data is stored inside using standard atomic types. Here the specific part is the specific combination of these attributes. Attributes are standard, but the meaning for different applications is different. It is like you are asking for inheriting "int" type in C++. You may create a specific-meaning interfaces only by keeing "int" in the field of this interface. |
|
|
I consider more about the document creation mechanism. It's easy to confuse user with several document creation entrances. It would be more simpler and better understanding for developer with single document creation entrance. We can provide a method Init(Handle(TDocStd_Document& theDoc) in retrieval drivers to replace the CreateDocument() method. So with these changes, the document creation should be always the same type. It sometimes make sense to inheriting document type with several other temp field instead of standard attribute. Anyway, Document is a rather big class, user have his own decision to inheriting it by adding more application specific logic inside it base on base class implementation. |
|
|
Branch CR29822 has been created by vro. SHA-1: febc630bcebe2fbab28961fea4a93a1d57bf107b Detailed log of new commits: Author: Vlad Romashko Date: Mon Dec 14 15:08:35 2020 +0300 0029822: Make TDocStd_Document extensible Two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application to its parent class CDF_Application. In TDocStd_Application these methods remain redefined. These little changes allow creation of a new document only in one virtual method NewDocument(). The methods CreateDocument() in all retrieval drivers are deleted. Modified files: - CDF_Application.hxx and cxx: two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application. The input parameter TDocStd_Document is changed to parent class CDM_Document. - TDocStd_Application.hxx and cxx: redefines new virtual methods NewDocument() and InitDocument() of the parent class CDF_Application. - BinLDrivers_DocumentRetrievalDriver.hxx and cxx, StdLDrivers_DocumentRetrievalDriver.hxx and cxx, XmlLDrivers_DocumentRetrievalDriver.hxx and cxx, PCDM_Reader.hxx : a virtual method CreateDocument() is deleted. - TObj_Application.cxx, XCAFApp_Application.hxx and cxx: down-casting to a descendant class TDocStd_Document is applied. OCCT: CR29822 Products: nothing |
|
|
Please, update the upgrade.md documentation, since you removed a public method that could be used somewhere outside. |
|
|
Branch CR29822_1 has been created by vro. SHA-1: 548c21faeb820b8252a277ecb69acb69c261b32b Detailed log of new commits: Author: Vlad Romashko Date: Tue Dec 15 10:12:26 2020 +0300 0029822: Make TDocStd_Document extensible Two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application to its parent class CDF_Application. In TDocStd_Application these methods remain redefined. These little changes allow creation of a new document only in one virtual method NewDocument(). The methods CreateDocument() in all retrieval drivers are deleted. Modified files: - CDF_Application.hxx and cxx: two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application. The input parameter TDocStd_Document is changed to parent class CDM_Document. - TDocStd_Application.hxx and cxx: redefines new virtual methods NewDocument() and InitDocument() of the parent class CDF_Application. - BinLDrivers_DocumentRetrievalDriver.hxx and cxx, StdLDrivers_DocumentRetrievalDriver.hxx and cxx, XmlLDrivers_DocumentRetrievalDriver.hxx and cxx, PCDM_Reader.hxx: a virtual method CreateDocument() is deleted. - TObj_Application.cxx, XCAFApp_Application.hxx and cxx: down-casting to a descendant class TDocStd_Document is applied. Documentation: - upgrade.md is modified. OCCT: CR29822_1 Products: nothing |
|
|
Reviewed. OCCT: CR29822_1 Products: nothing |
|
|
Branch CR29822_2 has been created by vro. SHA-1: 80618b3d57a94570d44c82ad71acb5ddd423644b Detailed log of new commits: Author: Vlad Romashko Date: Thu Dec 17 10:18:50 2020 +0300 0029822: Make TDocStd_Document extensible Two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application to its parent class CDF_Application. In TDocStd_Application these methods remain redefined. These little changes allow creation of a new document only in one virtual method NewDocument(). The methods CreateDocument() in all retrieval drivers are deleted. Modified files: - CDF_Application.hxx and cxx: two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application. The input parameter TDocStd_Document is changed to parent class CDM_Document. - TDocStd_Application.hxx and cxx: redefines new virtual methods NewDocument() and InitDocument() of the parent class CDF_Application. - BinLDrivers_DocumentRetrievalDriver.hxx and cxx, StdLDrivers_DocumentRetrievalDriver.hxx and cxx, XmlLDrivers_DocumentRetrievalDriver.hxx and cxx, PCDM_Reader.hxx: a virtual method CreateDocument() is deleted. - TObj_Application.cxx, XCAFApp_Application.hxx and cxx: down-casting to a descendant class TDocStd_Document is applied. Documentation: - upgrade.md is modified. OCCT: CR29822_2 Products: nothing |
|
|
> ... > Documentation: > - upgrade.md is modified. > > OCCT: CR29822_2 > Products: nothing Vlad, branches information is necessary only for Mantis, not for git message - please remove it. |
|
|
Ok. |
|
|
Branch CR29822_3 has been created by vro. SHA-1: 50d58d184e9a8677c5df5130916f8b47da9f32a6 Detailed log of new commits: Author: vro Date: Thu Dec 17 14:34:47 2020 +0300 0029822: Make TDocStd_Document extensible Two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application to its parent class CDF_Application. In TDocStd_Application these methods remain redefined. These little changes allow creation of a new document only in one virtual method NewDocument(). The methods CreateDocument() in all retrieval drivers are deleted. Modified files: - CDF_Application.hxx and cxx: two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application. The input parameter TDocStd_Document is changed to parent class CDM_Document. - TDocStd_Application.hxx and cxx: redefines new virtual methods NewDocument() and InitDocument() of the parent class CDF_Application. - BinLDrivers_DocumentRetrievalDriver.hxx and cxx, StdLDrivers_DocumentRetrievalDriver.hxx and cxx, XmlLDrivers_DocumentRetrievalDriver.hxx and cxx, PCDM_Reader.hxx: a virtual method CreateDocument() is deleted. - TObj_Application.cxx, XCAFApp_Application.hxx and cxx: down-casting to a descendant class TDocStd_Document is applied. Documentation: - upgrade.md is modified. |
|
|
Branch CR29822_3 has been updated by vro. SHA-1: d5118669275998b2a5db1e6d614a9c522e31d38e Detailed log of new commits: Author: vro Date: Thu Dec 17 15:51:23 2020 +0300 // upgrade.md is added |
|
|
Reviewed. OCCT: CR29822_3 Products: nothing |
|
|
I attached file with warnings - 28922.txt |
|
|
29822.txt (4,231 bytes) |
|
|
Combination - OCCT branch : WEEK-52 master SHA - 41046145c4a15f5cedf5f3c5877952ee00d568b4 a206de37fbfa0bf71bd534ae47192bbec23b8522 Products branch : WEEK-52 SHA - 290e5c74e8fef71947cadf90acb8e43c81ed10a1 was compiled on Linux, MacOS and Windows platforms and tested in optimize mode. Number of compiler warnings: No new/fixed warnings Regressions/Differences/Improvements: No regressions/differences CPU differences: Debian80-64: OCCT Total CPU difference: 17722.770000000193 / 17710.950000000055 [+0.07%] Products Total CPU difference: 12416.490000000109 / 12412.520000000126 [+0.03%] Windows-64-VC14: OCCT Total CPU difference: 19321.5625 / 19274.6875 [+0.24%] Products Total CPU difference: 13844.421875 / 13853.046875 [-0.06%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
|
Branch CR29822 has been deleted by inv. SHA-1: febc630bcebe2fbab28961fea4a93a1d57bf107b |
|
|
Branch CR29822_1 has been deleted by inv. SHA-1: 548c21faeb820b8252a277ecb69acb69c261b32b |
|
|
Branch CR29822_2 has been deleted by inv. SHA-1: 80618b3d57a94570d44c82ad71acb5ddd423644b |
|
|
Branch CR29822_3 has been deleted by inv. SHA-1: d5118669275998b2a5db1e6d614a9c522e31d38e |
|
occt: master 5fd0b800 2020-12-17 11:34:47 Committer: bugmaster Details Diff |
0029822: Make TDocStd_Document extensible Two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application to its parent class CDF_Application. In TDocStd_Application these methods remain redefined. These little changes allow creation of a new document only in one virtual method NewDocument(). The methods CreateDocument() in all retrieval drivers are deleted. Modified files: - CDF_Application.hxx and cxx: two virtual methods NewDocument() and InitDocument() are moved from TDocStd_Application. The input parameter TDocStd_Document is changed to parent class CDM_Document. - TDocStd_Application.hxx and cxx: redefines new virtual methods NewDocument() and InitDocument() of the parent class CDF_Application. - BinLDrivers_DocumentRetrievalDriver.hxx and cxx, StdLDrivers_DocumentRetrievalDriver.hxx and cxx, XmlLDrivers_DocumentRetrievalDriver.hxx and cxx, PCDM_Reader.hxx: a virtual method CreateDocument() is deleted. - TObj_Application.cxx, XCAFApp_Application.hxx and cxx: down-casting to a descendant class TDocStd_Document is applied. Documentation: - upgrade.md is modified. |
Affected Issues 0029822 |
|
| mod - dox/upgrade/upgrade.md | Diff File | ||
| mod - src/BinLDrivers/BinLDrivers_DocumentRetrievalDriver.cxx | Diff File | ||
| mod - src/BinLDrivers/BinLDrivers_DocumentRetrievalDriver.hxx | Diff File | ||
| mod - src/CDF/CDF_Application.cxx | Diff File | ||
| mod - src/CDF/CDF_Application.hxx | Diff File | ||
| mod - src/PCDM/PCDM_Reader.hxx | Diff File | ||
| mod - src/StdLDrivers/StdLDrivers_DocumentRetrievalDriver.cxx | Diff File | ||
| mod - src/StdLDrivers/StdLDrivers_DocumentRetrievalDriver.hxx | Diff File | ||
| mod - src/TDocStd/TDocStd_Application.cxx | Diff File | ||
| mod - src/TDocStd/TDocStd_Application.hxx | Diff File | ||
| mod - src/XCAFApp/XCAFApp_Application.cxx | Diff File | ||
| mod - src/XCAFApp/XCAFApp_Application.hxx | Diff File | ||
| mod - src/XmlLDrivers/XmlLDrivers_DocumentRetrievalDriver.cxx | Diff File | ||
| mod - src/XmlLDrivers/XmlLDrivers_DocumentRetrievalDriver.hxx | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2018-05-29 03:49 | Vico Liang | New Issue | |
| 2018-05-29 03:49 | Vico Liang | Assigned To | => mpv |
| 2018-05-29 16:28 |
|
Note Added: 0076409 | |
| 2018-05-29 16:28 |
|
Assigned To | mpv => Vico Liang |
| 2018-05-29 16:28 |
|
Status | new => feedback |
| 2018-05-30 04:24 | Vico Liang | Assigned To | Vico Liang => bugmaster |
| 2018-05-30 04:41 | Vico Liang | Note Added: 0076423 | |
| 2018-05-30 04:44 | Vico Liang | Assigned To | bugmaster => mpv |
| 2018-05-30 04:44 | Vico Liang | Status | feedback => assigned |
| 2018-05-30 14:32 |
|
Note Added: 0076437 | |
| 2018-05-30 14:33 |
|
Assigned To | mpv => Vico Liang |
| 2018-05-30 14:33 |
|
Status | assigned => feedback |
| 2018-05-30 16:28 | Vico Liang | Note Added: 0076441 | |
| 2018-05-30 16:40 | Vico Liang | Note Added: 0076442 | |
| 2018-05-30 16:57 |
|
Note Added: 0076444 | |
| 2018-05-30 16:57 |
|
Note Edited: 0076444 | |
| 2018-05-30 17:07 | Vico Liang | Note Added: 0076445 | |
| 2018-05-30 17:11 | Vico Liang | Note Added: 0076446 | |
| 2018-05-30 17:36 |
|
Note Added: 0076448 | |
| 2018-05-30 19:10 | Vico Liang | Note Added: 0076455 | |
| 2018-05-31 03:32 | Vico Liang | Assigned To | Vico Liang => mpv |
| 2018-05-31 03:32 | Vico Liang | Status | feedback => assigned |
| 2019-07-10 09:01 |
|
Target Version | 7.4.0 => 7.5.0 |
| 2020-09-03 15:19 |
|
Target Version | 7.5.0 => 7.6.0 |
| 2020-12-14 15:08 | git | Note Added: 0097573 | |
| 2020-12-14 16:49 | kgv | Summary | Make TDocStd_Document extensible => Application Framework - Make TDocStd_Document extensible |
| 2020-12-14 17:13 | vro | Status | assigned => resolved |
| 2020-12-14 17:13 | vro | Steps to Reproduce Updated | |
| 2020-12-15 09:34 |
|
Note Added: 0097599 | |
| 2020-12-15 09:34 |
|
Assigned To | mpv => vro |
| 2020-12-15 09:34 |
|
Status | resolved => assigned |
| 2020-12-15 10:12 | git | Note Added: 0097600 | |
| 2020-12-15 11:38 | vro | Assigned To | vro => mpv |
| 2020-12-15 11:38 | vro | Status | assigned => resolved |
| 2020-12-15 11:38 | vro | Steps to Reproduce Updated | |
| 2020-12-15 13:49 |
|
Note Added: 0097610 | |
| 2020-12-15 13:49 |
|
Assigned To | mpv => bugmaster |
| 2020-12-15 13:49 |
|
Status | resolved => reviewed |
| 2020-12-17 10:18 | git | Note Added: 0097653 | |
| 2020-12-17 10:32 | kgv | Note Added: 0097654 | |
| 2020-12-17 10:32 | kgv | Assigned To | bugmaster => vro |
| 2020-12-17 10:32 | kgv | Status | reviewed => assigned |
| 2020-12-17 10:32 | kgv | Severity | minor => feature |
| 2020-12-17 10:36 | vro | Note Added: 0097655 | |
| 2020-12-17 12:40 | vro | Assigned To | vro => mpv |
| 2020-12-17 12:40 | vro | Status | assigned => resolved |
| 2020-12-17 12:40 | vro | Steps to Reproduce Updated | |
| 2020-12-17 14:34 | git | Note Added: 0097681 | |
| 2020-12-17 15:50 | git | Note Added: 0097690 | |
| 2020-12-17 15:52 |
|
Note Added: 0097691 | |
| 2020-12-17 15:52 |
|
Assigned To | mpv => bugmaster |
| 2020-12-17 15:52 |
|
Status | resolved => reviewed |
| 2020-12-18 14:22 | bugmaster | Status | reviewed => feedback |
| 2020-12-21 14:30 | bugmaster | Note Added: 0097832 | |
| 2020-12-21 14:30 | bugmaster | Assigned To | bugmaster => vro |
| 2020-12-21 14:30 | bugmaster | Status | feedback => assigned |
| 2020-12-21 14:30 | bugmaster | File Added: 29822.txt | |
| 2020-12-23 19:34 | bugmaster | Assigned To | vro => mpv |
| 2020-12-23 19:34 | bugmaster | Status | assigned => resolved |
| 2020-12-23 19:34 | bugmaster | Assigned To | mpv => bugmaster |
| 2020-12-23 19:34 | bugmaster | Status | resolved => reviewed |
| 2020-12-26 12:15 | bugmaster | Note Added: 0097917 | |
| 2020-12-26 12:15 | bugmaster | Status | reviewed => tested |
| 2020-12-26 12:21 | bugmaster | Test case number | => Not required |
| 2020-12-26 14:40 | bugmaster | Changeset attached | => occt master 5fd0b800 |
| 2020-12-26 14:40 | bugmaster | Status | tested => verified |
| 2020-12-26 14:40 | bugmaster | Resolution | open => fixed |
| 2020-12-26 14:52 | git | Note Added: 0097927 | |
| 2020-12-26 14:52 | git | Note Added: 0097928 | |
| 2020-12-26 14:52 | git | Note Added: 0097929 | |
| 2020-12-26 14:52 | git | Note Added: 0097930 |
