On Wed, 2011-05-25 at 17:16 +0200, Christophe Laprun wrote:
On May 25, 2011, at 4:34 PM, Matt Wringe wrote:
> On Wed, 2011-05-25 at 12:07 +0200, Christophe Laprun wrote:
>> On May 25, 2011, at 10:35 AM, Boleslaw Dawidowicz wrote:
>>
>>> I need more time to give more detailed feedback but some quick comments
inline.
>>>
>>> On May 25, 2011, at 9:44 AM, Christophe Laprun wrote:
>>>>>
>>>>> First I want to reiterate that for the scope of next release we said
we
>>>>> stick with:
>>>>> - application registry features
>>>>> - page/navigation node metadata
>>>>> So
>>>>> - creating a page
>>>>> - adding apps to a page
>>>>> - managing users
>>>>> is out of scope for the coming release. (will be for the release
after)
>>>
>>> For users if we drop the ID part of Chris proposal it can be decoupled from
the API. Actually I would strongly argue to not complicate this part and leave user and
group reference with simple string ("john" and "/platform/users").
>>
>> You know my point of view on this… :)
>> Again, the Id classes don't need to be as complex as they are in my
>> current proposal. I just wanted to flesh out some generic Id ideas
>> that I had to see if they were actually feasible.
>>
>> Plus, the complexity is not that high from a user perspective.
>> Contrast:
>> Groups.getGroup("/platform/users");
>> with:
>> Groups.getGroup(Ids.getGroupId("platform", "users"));
>
> but what about Groups.getGroup("platform", "users");?
Would work for this particular use case… but see below.
> I don't know if we really need to have a separate complex id system. The
> id system should just be doing the parsing anyways and passing that on
> to the portal.
Id system? Again, it doesn't need to be complex, it only needs to be encapsulated.
>> for example. How does that complicate things? It is a little more
>> complicated for us to implement but it's simpler for users because
>> they don't need to know what the format of a group name is. They don't
>> need to concatenate and/or parse Strings anytime they need to deal
>> with a group name, etc.
>>
>> In the first version, you need to let users of the API know about the
>> internal format of a group name and basically commit to it for a long
>> time: it becomes an implicit part of the publish API. In the second
>> case, you have a slight overhead of creating an intermediate object
>> but one that you won't need to parse again. Most importantly, you
>> never expose the internals of how groups are named, which makes it
>> easier to evolve the internal implementation without impacting
>> existing code. Also, the validation code goes into the Id creation and
>> you can then assume in the getGroup method that the Id you get is
>> actually valid which makes it easier to implement and not have to
>> duplicate validation code all over the place. This also would help
>> against things like XSS or injection attacks for example.
>
> Shouldn't the actual components be checking that the identifiers passed
> to them are correct? I don't know if we want to have a complex id
> verification system in the api. The verification process should be done
> in only one location.
I meant checking the format. If you have several methods that take Strings id as
parameters, each method will first need to call some validation of the format first.
Checking that the id actually maps to a valid component is then up to the backend, not the
component itself.
> Things like XSS may need to be dealt with in the api.
Not sure what you mean here that is different from what I said :)
>> I would really like to hear a compelling argument against Ids as
>> separate objects because so far, the only arguments I've heard are: I
>> don't like them without any solid arguments as to why we shouldn't use
>> them. Please enlighten me as I only see advantages to them! ;)
>
> I don't know why they are really needed since:
>
> getElement(ID.createElementID("element1", "elementTypeA"))
>
> can be handled more simply with:
>
> getElement("element1", "elementTypeA");
Except that Ids don't have all the same formats so that prevents
generic methods. For example, you couldn't have
ContentRegistry.getContent("warName", "portletName") because that
would conflict with ContentRegistry.getContent("invokerName",
"portletHandle") for the WSRP case.
But that is just an implementation detail.
applicationRegistry.getPortletRepository(LOCAL).getPortlet("warName",
"portletName");
applicationRegistry.getPortletRepository(wsrp.SELF).getPortlet("portletHandle");
[well, I guess if wsrp uses portletHandles, I need to relook at how my
portlet repository class currently works]
Bypassing the Id object would only work in non-ambiguous cases and
I'm fine with doing so.
We can always write the api so its going to be non-ambiguous with
respect to this.
However, there are cases where it wouldn't work. Also, again,
it
would mean that each method that needs to work with an identifier
would need to first perform the intermediate step (though obviously
that code could be mutualized internally, but if we're going this
step, why not have Ids? ^_^).
Plus, T getElement(Id<T> id) would allow us to have typesafety
at the
id and returned value level, something you wouldn't be able to do with
getElement(String... ).
This is a valid point. However, it's still not a very strong
argument
against Id as separate objects. What is the big issue in your opinion
with having separate Id objects?
Its cumbersome to always have to think and create ids to do something,
when it can be done more directly without them. Its also confusing if
some elements require these ids while others don't (ie a category
doesn't need a separate id class, since it can be differentiated by a
string).
I don't think it makes sense to have things like
Id id = Id.generateId("foo", "bar");
Element element = something.getElement(id);
instead of just:
Element element = something.getElement("foo", "bar");
Ok, so lets say we need to have more complex ids:
PortletId pId = Id.generatePortletID("foo", "bar");
WSRPId wId = Id.generateWSRPPortletId("foo", "bar");
GadgetId gId = Id.generateGadgetID("foo", "bar");
Portlet portlet = getApplication(pId);
Portlet portlet = getApplication(wId);
Gadget gadget = getApplication(gId);
We can still handle it using something like:
getApplication(Application.Portlet, "foo", "bar");
getApplication(Application.WSRP, "foo", "bar");
getApplication(Application.Gadget, "foo", "bar");
but in this case I would argue that we use getPortlet("foo", "bar")
and
getGadget("foo","bar"). But for the wsrp case, its get a bit more
tricky
(which is why I have multiple portlet repository classes).
Cordialement / Best,
Chris
==
Principal Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
Follow GateIn:
http://blog.gatein.org /
http://twitter.com/gatein
Follow me:
http://metacosm.info/metacosm /
http://twitter.com/metacosm