Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/mc 9631 - Branch and merge Model Data Type #223

Merged
merged 21 commits into from
Jan 25, 2022
Merged

Feature/mc 9631 - Branch and merge Model Data Type #223

merged 21 commits into from
Jan 25, 2022

Conversation

aaronforshaw
Copy link
Contributor

@aaronforshaw aaronforshaw commented Dec 22, 2021 •
edited

  • Fix bugs in the path retrieval of VersionedFolder.
  • Add a method to the PathService to find a resource by its Path only, with no security check on the securable resource which owns the resource.
  • When branching a Versioned Folder, update Model Data Types in the branched folder so that they point to the branched model.
  • Update ModelDataType.diff so that it only returns a diff if the paths of the pointed to resource in source and target are different after ignoring differences in the branch and version. The diff returned is on 'modelResourcePath' rather than modelResourceId and modelResourceDomainType. This 'modelResourcePath' is computed as a fully qualified path by recursing up all parents of the model.
  • Add a method to DomainService which enables subclasses to state that they will provide special handling for the patching a modification into a Versioned Folder
  • In ModelDataTypeService, override the above method and use the 'modelResourcePath' provided in the diff to look up the relevant resource to point to when merging.
  • Ditto the above two points, but for merging when not in a Versioned Folder
  • Add test data
  • Add tests for (i) branching of Model Data Type in a VF (ii) merge a Model Data Type when it is pointing to a resource in the source folder (iii) merge a Model Data Type when it is pointing to a resource outside of the VF
  • Add test for branching and merging of a Data Model containing a Model Data Type when not in a VF

olliefreeman
olliefreeman requested changes Dec 22, 2021
View reviewed changes
Copy link
Contributor

@olliefreeman olliefreeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the methodology is right,

All the stuff in path and path node and model, can you replace it all with the use of boolean matchesIdentifier(PathNode otherPathNode, String modelIdentifierOverride = null) as thats why that method exists, you can pass in a branch name or model version number as the override and it will do everything you've coded up for you.

...ils-app/domain/uk/ac/ox/softeng/maurodatamapper/datamodel/item/datatype/ModelDataType.groovy Outdated Show resolved Hide resolved
...ils-app/domain/uk/ac/ox/softeng/maurodatamapper/datamodel/item/datatype/ModelDataType.groovy Outdated Show resolved Hide resolved
mdm-core/src/main/groovy/uk/ac/ox/softeng/maurodatamapper/core/model/ModelService.groovy Outdated Show resolved Hide resolved
@aaronforshaw
Copy link
Contributor Author

@olliefreeman Please see updated description at the top of this PR.

@aaronforshaw aaronforshaw marked this pull request as draft January 18, 2022 08:25
@aaronforshaw
mc-9631 repoint model data types when branching versioned folders
89c78e0
@aaronforshaw
mc-9631 only try to repoint if data model and model resource are in t…
25fa3c7 
…he same VF
@aaronforshaw
mc-9631 ignore diffs from a Model Domain Type with the same path othe…
53b7e83 
…r than model identifier
@aaronforshaw
mc-9631 tolerate situations in which domain class not loaded
2f16aaa
@aaronforshaw
mc-9631 remove branchName argument from updateCopiedCrossModelLinks
ffa9ec3
@aaronforshaw
mc-9631 use matches() method
d869df8
@aaronforshaw
mc-9631
c2743f5 
- fix pathing of VersionedFolder
- diff of ModelDataType returns a modelResourcePath
- generic approach to handle a modification patch using a special method
- use modelResourcePath in the patch to find the relevant modelResourceId and modelResourceDomainType
@aaronforshaw
mc-9631 rebase changes
587253a
@aaronforshaw
mc-9631 Add method to find resource by Path without checking access
a86dd10
@aaronforshaw
mc-9631 Add a Model Data Type which points to one of two Terminologie…
706dbb9 
…s in an external non-versioned folder
@aaronforshaw
mc-9631 Fully qualified Path in the diff
434a236
@aaronforshaw
mc-9631 Handle merge of model data type pointing to model external to…
b4058d4 
… the VF
@aaronforshaw
mc-9631 Simplify testing by using the bootstrapped terminologies
c333e09
@aaronforshaw
mc-9631 Use findResourceByPath so that the find works when in a subfo…
c411c9e 
…lder
@aaronforshaw
mc-9631 Restructure tests to make better use of merge builders
9375ee7
@aaronforshaw
mc-9631 Remove the dependence on terminologies endpoint from the Data…
0026fa3 
…ModelPluginMergeBuilder
@aaronforshaw
mc-9631 special handling of merge fields when using a path
64a201e
@aaronforshaw
mc-9631 test merging of Model Data Type in a DM
4714979
@aaronforshaw
mc-9631 Model always has a folder
7e1be8e
@aaronforshaw
mc-9631 fix failing test after rebase
0b811e8
@aaronforshaw aaronforshaw marked this pull request as ready for review January 20, 2022 12:38
olliefreeman
olliefreeman approved these changes Jan 25, 2022
View reviewed changes
Copy link
Contributor

@olliefreeman olliefreeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the changes in commit im happy with this now

@olliefreeman
Some tweaks as a review
c768226 
* Remove the addtl methods into CreatorAware as these already exist as part of PathNode where a pathnode can be altered using a modelIdentifier, this reduces the number of places where changes need to be made if we alter anything
* Remove the coding relience on the concept of VFs, rather have one method for patch handling which uses the target to determine what happens. Allows for greater depth if we need to add any addtl handling later
* MDTs are always keyed to models so use ModelServices list rather thae DomainServices list as its less to search and therefore faster
* Make a handy method which can extract a model for a MDT or throw an exception thereby reducing the code needed
@olliefreeman
Copy link
Contributor

failing test is a known flaky test so merging

@olliefreeman olliefreeman merged commit 812e7bb into develop Jan 25, 2022
@olliefreeman olliefreeman deleted the feature/mc-9631 branch January 25, 2022 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@olliefreeman olliefreeman olliefreeman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@aaronforshaw @olliefreeman