Only resolve links when needed

[artlowel]

This PR fixes #558 by adding the _links section from HAL responses to the domain models, and no longer resolving those links automatically.

Instead you can specify which links you want to resolve whenever you fetch an object from a dataservice. This will ensure we only resolve what's needed, and it will also integrate well with projections, as it will allow us to determine which projection to use based on the links that need to be resolved in a component.

Since the regular domain models will no longer contain resolved links, they are already "normalized" so this means we no longer have a need for separate normalized models.

Due to inconsistent implementations in a few places, it is likely that this PR when finished won't be able to remove all normalized models, but leave a few deprecated ones to cover cases where an bigger rewrite would be necessary to work in the new way.

This PR is currently a draft. It is nearly done, the work left to do is mainly adding an fixing a few more tests, and documentation, and merging with the latest master.

[tdonohue]

@artlowel : I gave this a thorough code review today. It honestly looks great to me overall, and I have to say I really like the simplification to the codebase (especially the removal of all the Normalized* objects in favor of using annotations). The new followLink() method is a great improvement too.

That said, I did find a few areas with missing Javadocs and/or small questions. These all seem like minor cleanup to me though (see inline comments below).

In testing this PR, I noticed that yarn start (AoT compilation) currently fails cause of an error in thumbnail.component.html. It's still calling errorHandler() with an event param, but you removed that param. See this line of code: https://github.com/DSpace/dspace-angular/blob/master/src/app/thumbnail/thumbnail.component.html#L2 After fixing that single line, yarn start works great.

I tested out the Angular UI with this PR installed. I didn't notice any problems. But, I did notice that every page seems to load a bit quicker than before!

Finally, this is a sidenote to this PR...but, I'm noticing more and more that different PRs seem to be reordering (resorting) our import statements in various typescript files. This PR does a fair amount of that itself in any class you touch (likely automated by your IDE's settings). In a future PR, I wonder if we need to look at finding a way to force everyone to use a specific ordering for these imports (maybe via tslint, if that's possible)? It'd definitely make future PRs easier to review if there were less moving around of imports.

[artlowel]

@tdonohue Thanks for the thorough code review!

In testing this PR, I noticed that yarn start (AoT compilation) currently fails cause of an error in thumbnail.component.html. It's still calling errorHandler() with an event param, but you removed that param. See this line of code: https://github.com/DSpace/dspace-angular/blob/master/src/app/thumbnail/thumbnail.component.html#L2 After fixing that single line, yarn start works great.

Yeah, that was a change I made as I wrote the tests for it and realized it wasn't used anymore. I neglected to re-test the AoT build afterwards, because I didn't think I did anything that could impact it. Thanks for picking that up!

I tested out the Angular UI with this PR installed. I didn't notice any problems. But, I did notice that every page seems to load a bit quicker than before!

Also note that, because I had to refactor the entire codebase, and didn't have the time to look at each case in depth, I erred on the side of adding more followLinks. For each one that we can remove without breaking anything, the performance should improve.

At least with this PR we have a convenient way of doing that.

Now that the domain models have all the links, requests that were only made to retrieve that info can also be left out. One such example is the clearRelatedCache method in RelationshipService. I'm sure there are more places where this kind of optimization can happen.

Finally, this is a sidenote to this PR...but, I'm noticing more and more that different PRs seem to be reordering (resorting) our import statements in various typescript files. This PR does a fair amount of that itself in any class you touch (likely automated by your IDE's settings). In a future PR, I wonder if we need to look at finding a way to force everyone to use a specific ordering for these imports (maybe via tslint, if that's possible)? It'd definitely make future PRs easier to review if there were less moving around of imports

Yes that's due to the IntelliJ's optimize imports feature, it ensures no unused classes are being imported, and it will sort imports first by module, and alphabetically within a module. It is triggered automatically every time we tell IJ to add a new import.

IJ allows you to disable both sort options independently. So we could:

• Leave it as is and ask everybody to conform to these rules
• Sort alphabetically, but not by module and ask everybody to conform to these rules.
• Sort by module but not alphabetically, but that doesn't make a whole lot of sense
• Turn off sorting imports altogether.

I've briefly looked at the editorconfig spec and it doesn't look like you can enforce import ordering using it

[tdonohue]

👍 I've reviewed all the recent updates, and this now looks good to me. I've already successfully tested this PR yesterday (see previous comment) and it's working great for me as well. Thanks @artlowel !

UPDATE: Regarding the import question, if we cannot easily enforce it in some generic way (across IDEs) then it's likely not going to matter much (it's hard to get devs to sync IDE settings, and sometimes hard to get the same settings in different IDEs). So, we may just have to live with it for now. On the Java side, we've been able to force devs to order import statements in the same order every time using checkstyle (and the Maven checkstyle plugin). If there's not an equivalent plugin yet for Angular / npm / yarn, then we may just need to leave it be for now. I'll look around a bit myself to see if I find anything as well, and if so create a ticket/PR to enable it.

UPDATE 2: It's possible to use tslint to force ordering of import statements. It touches a lot of files though, so I created a ticket to do this at some later time in the future, see #583

[paulo-graca]

I've noticed a substantial speed up in responses. DSpace is faster with this PR. Once again, thank you @artlowel for this important contribution.

[tdonohue]

Merging, as this is at +2. Thanks @artlowel (and team) for these great improvements!