Marketplace servlet initial progress

[apetro]

Initial progress towards a Servlet view on Marketplace. Cf. UP-4080 (Marketplace Servlet "epic").

Partial progress

This is an initial proof of concept proposed for merged into a feature branch for collaboration and continued work, not yet proposed for merge to master because it is so very much not baked.

You should not adopt in production this functionality as it is here implemented.

Context and Vision

Where we're really trying to go here:

• compelling views on Marketplace suitable for Google search indexing and discovery via Google search
• somewhat inspired by https://one.iu.edu/ .

We're actually shockingly close to being able to deliver an experience like https://one.iu.edu/ as one view on, one way of accessing uPortal. Personally, I (@apetro) don't think a Marketplace is the only way to navigate portal content, but I do think it's one way, and this feature branch is about making that possible in uPortal. This is just the first increment of progress on the marketplace_servlet_feature branch. Hang on to your seats, it's going to be a great ride.

Features

Marketplace servlet view: responsive!

A Servlet view on Marketplace renders, and it's even responsive.

Categories show

Categories and portlets show, even filtered according to the user's BROWSE permission.

Marketplace entries render

Complete with screenshots, etc.

At a pretty URL

Marketplace entries are addressed as a path element. Very hipster.

Filtering works

Yay Datatables / JavaScript (but boo the brute force I'm doing to include the necessary dependencies!)

Works even when not logged in

Respects permissions of logged-in-user or of guest if not logged in. Just works.

Implementation thoughts

Arguably this could have been a usage of the DETACHED portlet mode on the existing Marketplace portlet instead, but

• Servlet views on uPortal need exploring
• This is a step towards making the Marketplace service re-usable by being re-used, and so a step towards JSON APIs (and eventually All The Things will be JavaScript rendering JSON APIs!)
• Servlet controller and view gives a lot more control about URL structure and markup towards the goal of search engine indexability.

Known shortcomings and technical debt

This just ain't baked. Proof-of-concept-deep only.

Unmaintainable horrible brute force styling

This changeset takes the brute force approach of providing styling to the servlet JSPs, but doesn't use a CDN or ResourceServer appropriately and doesn't respect skin customizations and basically what it's doing is an unacceptable brute force first pass to prove the concept that these JSPs can work. Backlogged as UP-4081.

Links don't work

(Almost) none of the hyperlinks work in the servlet views, their having been de-featured to get to inital proof-of-concept rendering. That's both navigational links and action links. (Backlogged as UP-4082.

The UI doesn't gracefully degrade to reflect permissions on those links that don't render (as in, it doesn't even try to suppress the favorite/go link if you don't have permission to subscribe the portlet). Backlogged as UP-4083.

Abusing /api namespace

The Servlet view on Marketplace isn't an /api, but it's at the /api path since I'm abusing the mvcController that runs the existing /api JSON APIs. Better would be to invent a /web path or so for shiny new Servlet views on uPortal stuff.

I took a stab at adding a parallel MVC controller to implement /web and I'm embarrassed to say I couldn't figure it out in an appropriate timebox, so, backlogged as UP-4084.

Also, addressing as /api/marketplace/ works, but /api/marketplace doesn't, and that'll really annoy you when you trip over it! (Also backlogged in UP-4084.

Abstraction violations and duplication

While this benefits from some refactoring of Marketplace functionality into a service layer, the Controllers still directly depend on DAOs when really they should be going through the Service layer for that, and refactoring to that architecture will allow further de-duplication of functionality across the servlet and portlet Controllers over this functionality. Backlogged as UP-4085.

No consideration of performance

Still ain't got no caching. :) That's backlogged as UP-4072. In practice if you let a search engine crawl this as is, it would probably beat the heck out of your portal.

Category BROWSE permission not reflected in Marketplace entry view

Marketplace entries don't filter out Categories that the user doesn't have permission to BROWSE. Backlogged as UP-4086.

[timlevett]

I think if you remove the last / then api/marketplace will work.

[apetro]

Hope so. :) Backlogged in UP-4084, with link from there to this to inform that work.

[timlevett]

Not used?

[apetro]

Interesting. I'd intended marketplaceService as a JavaBean property and so it reflexively got a getter to go with its setter, but looking around at other @autowired Controllers in uPortal, looks like the pattern is to have setters without accompanying getters. E.g. ActivityController, ChannelListController, EntitiesRESTController. Will drop the un-used getters.

[apetro]

Done, in 938654a and ce38832 .

The ce38832 part of that that applies to master, that is, on the MarketplaceService and on the portlet Controller, proposed for merge to master at #314 .

[timlevett]

not used. Remove? Same goes for all around here for getters.

[timlevett]

Idea here, we could pull out shared content from the portlet into separate JSPs then utilize jsp:include.

[apetro]

Could. Probably requires URLs parameterized so the included JSP gets them right whether invoked in a portlet or servlet context. Backlogged as UP-4089.

[timlevett]

I wonder if we could pull these from rs instead? At a minimum I would suggest adding in a parameter for the context that is available on the JSP.

[apetro]

Certainly need to do something less hacky than what's here. Backlogged the RS consideration as UP-4081. Backlogged the broken assumption of /uPortal/ context as UP-4090.

[timlevett]

Duplicate?

[apetro]

Good catch. Removed in 78bcd77 .

[timlevett]

hard coded servlet context.

[apetro]

Yes. Truly horrible. Backlogged as UP-4090.

[timlevett]

This also might be the reason /api/marketplace doesn't work

[apetro]

Might well. I think I remember having trouble with it when I was less specific in the @RequestMapping(), but that might have been some other ghost in the machine. Anyway, backlogged in UP-4084.

[timlevett]

I do appreciate the providing an update for what you are working on but I find it confusing to review. I'm not sure what I should comment on or what I should let pass because it is labeled as half baked. I would rather just review something when it is fully baked. I don't see a problem of having a Jasig/uPortal feature branch, but I'm not sure if we need a PR for said event.

Noticed Travis failed due to a metadata.xml malfunction again. Might want to kick it again.

[apetro]

@timlevett thanks for the feedback; backlogged most and addressed a little. I appreciate being able to use this pull request to get early review of changeset as it goes into that marketplace_servlet_feature branch.

Pull Requests have some nice features for providing a place to document at a higher level than individual commit messages what the bigger picture of a changeset is, with screenshots. I rather hope this worked well as a way to articulate what's here so far and what all is already known still needs done on this feature branch.