WebAPI refactoring

[pavgra]

1. Structure (domain vs type of file) TBD

2. Controller-service-repository

   • Would be great to move closer to Spring terminology and use controller for a collection of REST endpoint, service for a collection of functions implementing application business logic and repository for a collection of persistance functions.
   • Use the three terms as postfixes for the appropriate files

3. Clean-up of some services / etc to make them more atomic and granular E.g. there are some services having N thousand of lines of code, which not only contain too big amounts of code for a single file but also have nested classes which could be moved into separate files.

[chrisknoll]

On Point 1: I found this I thought was a thoughtful read, describing both approaches (Package By Layer, Package by Function).
https://dzone.com/articles/project-package-organization

With that article in mind, here's my thoughts;

I like the grouping by feature (which I've been saying 'domain' up to this point @pavgra , but let's call it 'Package by Feature'.

With Package by Feature, it will let us reference 'slices' of the application by the feature, not the layer. In other words, we'd more likely want to look at classes by the features they are involved with rather thant he layer that they are involved with. Furthermore, package protection of classes/fields becomes more functional if the pacakges are oriented around the features they support vs. the layers (all services in the same layer of 'services' will have the same package-level access to each other, which is probably not how you want to separate your code).

Using 'Package By Feature', we think more in 'micro services' mindset (where the micro-service provides a feature) while if we divide it up by layers (service...repository...controller...util are the 'layers) this is a bit harder to manage.

So, My vote for dealing with the topic of structure, is to orient the code around features, not layers.

[pavgra]

A couple of things before they are forgotten:

• explicit CONSTRAINTs in migrations
• Transactional services
• Lazy fetching & entity graphs

[chrisknoll]

Another one: Standrization of REST-ish endpoints to the singular or plural, to have a set naming convention for people to follow.

Alternatively, decide if pure-rest is not the approach we want to follow, and instead think in terms of 'WebServices' as HTTP interaction with a back-end API.

[pavgra]

Enhancements for security:

• how generation and result routes are protected (shiro for entity access check + annotation for source access check)
• renaming of shiro package to security package
• removing tons of duplication in AtlasSecurity classes
• overall whitelist approach instead of blacklist
• encapsulation of creatorPermissionTemplates in appropriate modules
• clarify whether AuthFilter or Realm should execute actual auth - today we have a mix

[pavgra]

Integers for IDs

[pavgra]

Think on common utils for SAA / circe https://github.com/OHDSI/circe-be/pull/56#discussion_r220545352

[pavgra]

+ linking https://github.com/OHDSI/WebAPI/issues/505 to not forget

[wivern]

There are several descendants of Criteria In the OHDSI/circe library that has the same properties, e.g. codesetId. That would be nice to implement some abstract classes in the hierarchy and/or interfaces to simplify Criteria hierarchy handling. This is actual for Diagnostics Checks and for Feature Extraction Analysis on WebAPI.

[vantonov1]

• It is better to use in-memory embedded DB, like HSQLDB, Derby or H2, for module testing
• It is better to store all properties in application.properties, not in pom

[pavgra]

All logging should be done via org.slf4j.LoggerFactory