You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fortress@directory.apache.org by "Pike, Christopher" <cl...@psu.edu> on 2018/12/19 20:10:12 UTC
FC-250
This commit is a potentially breaking change. (https://github.com/apache/directory-fortress-core/commit/2d8a53071b8d8b5bb3b6256f084a9e12d4a7cc10#diff-280616e1526a17336cdb4ff1855d7439)
Specifically
@JsonTypeInfo(use=Id.CLASS, include=As.PROPERTY, property="fqcn", visible=false)
public abstract class FortEntity
now requires an id field to be added to all fortress entities when serializing / de-serializing. Can someone provide insight into what problem this is solving?
~Chris P.
Re: FC-250
Posted by Shawn McKinney <sm...@apache.org>.
> On Dec 19, 2018, at 3:10 PM, Pike, Christopher <cl...@psu.edu> wrote:
>
> We use the fortress model java classes for our fortress rest service (both server and client side). With the addition of that annotation it requires there to now be a property named "fqcn" on every object or de-serialization fails. This means we either have to update all of our code at once (not feasible), or do some jackson specific things to our code to ignore that field (mixins) or have our own branch that comments that annotation out. (was trying to get away from this,
I’m sure we’ll figure out something other than massive changes or separate branch.
>
> On Dec 19, 2018, at 3:10 PM, Pike, Christopher <cl...@psu.edu> wrote:
>
> FYI, I submitted a few PRs)
Curious why a PR rather than just committing directly? Submitting PR’s are for contributors w/out committer privileges. You’re a committer, we trust you. :-)
Although for large changes, please come and discuss here beforehand, so we understand the need, design, and to build consensus.
>
> On Dec 19, 2018, at 3:10 PM, Pike, Christopher <cl...@psu.edu> wrote:
>
> What is the scenario where you don't know the type and need it explicitly specified in the JSON?
Here is where we’ll need Kiran to chime in. I don’t recall what the issue he was facing.
—Shawn
Re: FC-250
Posted by "Pike, Christopher" <cl...@psu.edu>.
We use the fortress model java classes for our fortress rest service (both server and client side). With the addition of that annotation it requires there to now be a property named "fqcn" on every object or de-serialization fails. This means we either have to update all of our code at once (not feasible), or do some jackson specific things to our code to ignore that field (mixins) or have our own branch that comments that annotation out. (was trying to get away from this, FYI, I submitted a few PRs)
What is the scenario where you don't know the type and need it explicitly specified in the JSON?
________________________________
From: Shawn McKinney <sm...@apache.org>
Sent: Wednesday, December 19, 2018 3:57:03 PM
To: fortress@directory.apache.org
Subject: Re: FC-250
Hey Chris,
Changes made by Kiran to support a JSON message format via Fortress REST component.
JSON is used by the new UI, Embrasure.
Obviously if this is breaking some previous behavior we need to provide a solution.
Can you elaborate on the problem… you mentioned serialization, what is the scenario that worked before that no longer works?
Thanks,
—Shawn
> On Dec 19, 2018, at 2:10 PM, Pike, Christopher <cl...@psu.edu> wrote:
>
> This commit is a potentially breaking change. (https://github.com/apache/directory-fortress-core/commit/2d8a53071b8d8b5bb3b6256f084a9e12d4a7cc10#diff-280616e1526a17336cdb4ff1855d7439)
>
>
> Specifically
>
>
> @JsonTypeInfo(use=Id.CLASS, include=As.PROPERTY, property="fqcn", visible=false)
> public abstract class FortEntity
>
>
> now requires an id field to be added to all fortress entities when serializing / de-serializing. Can someone provide insight into what problem this is solving?
>
>
> ~Chris P.
Re: FC-250
Posted by Shawn McKinney <sm...@apache.org>.
Hey Chris,
Changes made by Kiran to support a JSON message format via Fortress REST component.
JSON is used by the new UI, Embrasure.
Obviously if this is breaking some previous behavior we need to provide a solution.
Can you elaborate on the problem… you mentioned serialization, what is the scenario that worked before that no longer works?
Thanks,
—Shawn
> On Dec 19, 2018, at 2:10 PM, Pike, Christopher <cl...@psu.edu> wrote:
>
> This commit is a potentially breaking change. (https://github.com/apache/directory-fortress-core/commit/2d8a53071b8d8b5bb3b6256f084a9e12d4a7cc10#diff-280616e1526a17336cdb4ff1855d7439)
>
>
> Specifically
>
>
> @JsonTypeInfo(use=Id.CLASS, include=As.PROPERTY, property="fqcn", visible=false)
> public abstract class FortEntity
>
>
> now requires an id field to be added to all fortress entities when serializing / de-serializing. Can someone provide insight into what problem this is solving?
>
>
> ~Chris P.
Re: FC-250
Posted by Shawn McKinney <sm...@apache.org>.
> On Mar 5, 2019, at 1:08 AM, Kiran Ayyagari <ka...@apache.org> wrote:
>
> I missed to commit the changes made to pom.xml in realm, the build should
> pass now.
> Thanks Shawn.
Ah silly me, I could have bumped the version of core the realm was using myself.
In any case, the tests all pass now so I’d say we’re good to go.
Thanks,
—Shawn
Re: FC-250
Posted by Kiran Ayyagari <ka...@apache.org>.
On Tue, Mar 5, 2019 at 1:14 AM Shawn McKinney <sm...@apache.org> wrote:
>
> > On Mar 3, 2019, at 6:17 AM, Kiran Ayyagari <ka...@apache.org> wrote:
> >
> > I have finally got a chance to merge the relevant parts of this code from
> > FC-247 branch.
> > Local build passed after merge but I only ran "mvn clean install" but I
> > don't think this runs the whole IT suit.
> >
> > I request anyone with the whole LDAP server setup to give it a try and
> let
> > me know if something breaks.
>
> Hi Kiran,
>
> I will be happy to test. I pulled the latest enmasse code from trunk,
> getting some compile errors, missing changes in the core. Currently I have
> latest from trunk on core. Are there some updates somewhere that need to
> be applied?
>
I missed to commit the changes made to pom.xml in realm, the build should
pass now.
Thanks Shawn.
>
> Please advise.
>
> Here’s the trace
>
> [INFO] --- maven-compiler-plugin:3.3:compile (default-compile) @
> fortress-rest ---
> [INFO] Changes detected - recompiling the module!
> [INFO] Compiling 17 source files to
> /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/target/classes
> [INFO] -------------------------------------------------------------
> [ERROR] COMPILATION ERROR :
> [INFO] -------------------------------------------------------------
> [ERROR]
> /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/AbstractMgrImpl.java:[47,34]
> cannot find symbol
> symbol: method getHttpStatus()
> location: variable se of type
> org.apache.directory.fortress.core.SecurityException
> [ERROR]
> /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/FortressResponseInterceptor.java:[60,88]
> cannot find symbol
> symbol: method getHttpStatus()
> location: class org.apache.directory.fortress.core.model.FortResponse
> [INFO] 2 errors
> [INFO] -------------------------------------------------------------
> [INFO]
> ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Total time: 4.821 s
> [INFO] Finished at: 2019-03-04T18:00:19-06:00
> [INFO] Final Memory: 34M/378M
> [INFO]
> ------------------------------------------------------------------------
> [ERROR] Failed to execute goal
> org.apache.maven.plugins:maven-compiler-plugin:3.3:compile
> (default-compile) on project fortress-rest: Compilation failure:
> Compilation failure:
> [ERROR]
> /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/AbstractMgrImpl.java:[47,34]
> cannot find symbol
> [ERROR] symbol: method getHttpStatus()
> [ERROR] location: variable se of type
> org.apache.directory.fortress.core.SecurityException
> [ERROR]
> /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/FortressResponseInterceptor.java:[60,88]
> cannot find symbol
> [ERROR] symbol: method getHttpStatus()
> [ERROR] location: class
> org.apache.directory.fortress.core.model.FortResponse
> [ERROR] -> [Help 1]
> [ERROR]
> [ERROR] To see the full stack trace of the errors, re-run Maven with the
> -e switch.
> [ERROR] Re-run Maven using the -X switch to enable full debug logging.
> [ERROR]
> [ERROR] For more information about the errors and possible solutions,
> please read the following articles:
> [ERROR] [Help 1] http://cwiki.apache.org/confluence/disp
>
> —Shawn
Kiran
Re: FC-250
Posted by Shawn McKinney <sm...@apache.org>.
> On Mar 3, 2019, at 6:17 AM, Kiran Ayyagari <ka...@apache.org> wrote:
>
> I have finally got a chance to merge the relevant parts of this code from
> FC-247 branch.
> Local build passed after merge but I only ran "mvn clean install" but I
> don't think this runs the whole IT suit.
>
> I request anyone with the whole LDAP server setup to give it a try and let
> me know if something breaks.
Hi Kiran,
I will be happy to test. I pulled the latest enmasse code from trunk, getting some compile errors, missing changes in the core. Currently I have latest from trunk on core. Are there some updates somewhere that need to be applied?
Please advise.
Here’s the trace
[INFO] --- maven-compiler-plugin:3.3:compile (default-compile) @ fortress-rest ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 17 source files to /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/target/classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/AbstractMgrImpl.java:[47,34] cannot find symbol
symbol: method getHttpStatus()
location: variable se of type org.apache.directory.fortress.core.SecurityException
[ERROR] /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/FortressResponseInterceptor.java:[60,88] cannot find symbol
symbol: method getHttpStatus()
location: class org.apache.directory.fortress.core.model.FortResponse
[INFO] 2 errors
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 4.821 s
[INFO] Finished at: 2019-03-04T18:00:19-06:00
[INFO] Final Memory: 34M/378M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:compile (default-compile) on project fortress-rest: Compilation failure: Compilation failure:
[ERROR] /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/AbstractMgrImpl.java:[47,34] cannot find symbol
[ERROR] symbol: method getHttpStatus()
[ERROR] location: variable se of type org.apache.directory.fortress.core.SecurityException
[ERROR] /home/smckinn/GIT/fortressDev/directory-fortress-enmasse/src/main/java/org/apache/directory/fortress/rest/FortressResponseInterceptor.java:[60,88] cannot find symbol
[ERROR] symbol: method getHttpStatus()
[ERROR] location: class org.apache.directory.fortress.core.model.FortResponse
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/disp
—Shawn
Re: FC-250
Posted by Kiran Ayyagari <ka...@apache.org>.
I have finally got a chance to merge the relevant parts of this code from
FC-247 branch.
Local build passed after merge but I only ran "mvn clean install" but I
don't think this runs the whole IT suit.
I request anyone with the whole LDAP server setup to give it a try and let
me know if something breaks.
On Thu, Dec 20, 2018 at 9:58 PM Stefan Seelmann <ma...@stefan-seelmann.de>
wrote:
> On 12/20/18 3:59 PM, Kiran Ayyagari wrote:
> > On Thu, Dec 20, 2018 at 6:45 PM Pike, Christopher <cl...@psu.edu>
> wrote:
> >
> >>
> >> 1. Is there a way to give me permission to accept the PRs from
> github?
> >>
> >> The code is already in the FC-247 branch, so should be accessible to all
> > committers. I am gonna merge the relevant parts of this
> > branch into master this weekend, this probably makes it easier for you
> and
> > others.
>
> Chris is committer :)
> As the Fortress Git repos are not yet migrated to gitbox but uses the
> old repos [2] there is no way to merge PR because it's a read-only
> mirror. Once migrated to Gitbox PRs can be merged by committers.
>
> Until then you have to merge locally to master, then push to the Apache
> repo, URLs are listed at [1].
>
> Kind Regards,
> Stefan
>
> [1] https://directory.apache.org/sources.html
> [2] https://git.apache.org/
>
Kiran
Re: FC-250
Posted by Stefan Seelmann <ma...@stefan-seelmann.de>.
On 12/20/18 3:59 PM, Kiran Ayyagari wrote:
> On Thu, Dec 20, 2018 at 6:45 PM Pike, Christopher <cl...@psu.edu> wrote:
>
>>
>> 1. Is there a way to give me permission to accept the PRs from github?
>>
>> The code is already in the FC-247 branch, so should be accessible to all
> committers. I am gonna merge the relevant parts of this
> branch into master this weekend, this probably makes it easier for you and
> others.
Chris is committer :)
As the Fortress Git repos are not yet migrated to gitbox but uses the
old repos [2] there is no way to merge PR because it's a read-only
mirror. Once migrated to Gitbox PRs can be merged by committers.
Until then you have to merge locally to master, then push to the Apache
repo, URLs are listed at [1].
Kind Regards,
Stefan
[1] https://directory.apache.org/sources.html
[2] https://git.apache.org/
Re: FC-250
Posted by Kiran Ayyagari <ka...@apache.org>.
On Thu, Dec 20, 2018 at 6:45 PM Pike, Christopher <cl...@psu.edu> wrote:
>
> 1. Is there a way to give me permission to accept the PRs from github?
>
> The code is already in the FC-247 branch, so should be accessible to all
committers. I am gonna merge the relevant parts of this
branch into master this weekend, this probably makes it easier for you and
others.
>
> 1.
> 2. Understand that the fqcn field is automatically included, but the
> issue is that clients and servers with different versions of the FortEntity
> class are now incompatible. A client with the old version won't include the
> field, so the server will fail on de-serialization, or a client with the
> new version expects it to be there when de-serializing, so will fail if the
> server doesn't provide it.
>
> So I am assuming that the FortEntity is being exchanged in JSON format
(otherwise this issue won't happen), are you using a custom JSON provider
than com.fasterxml.jackson.jaxrs.json.JacksonJsonProvider? because I didn't
encounter any issue with "fqcn" mapping when JacksonJsonProvider was used
>
> 1.
> 2. It's been a while since I looked at the fortress REST api, but it
> seems more xml rpc that REST. I've attached a screen shot that shows a high
> level of how we have our REST api structured.
>
> The attachment didn't make it to the list, but yes it is more like XML rpc
but to drive the effort for a simplified web admin UI the JSON support was
added.
>
> 1.
>
> ------------------------------
> *From:* Kiran Ayyagari <ka...@apache.org>
> *Sent:* Thursday, December 20, 2018 12:39:50 AM
> *To:* fortress@directory.apache.org
> *Subject:* Re: FC-250
>
> On Thu, Dec 20, 2018 at 1:40 AM Pike, Christopher <cl...@psu.edu> wrote:
>
> > This commit is a potentially breaking change. (
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fdirectory-fortress-core%2Fcommit%2F2d8a53071b8d8b5bb3b6256f084a9e12d4a7cc10%23diff-280616e1526a17336cdb4ff1855d7439&data=02%7C01%7Cclp207%40psu.edu%7C17cfc158227245fdd4c708d6663da470%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636808812280341605&sdata=C%2FJEfLe02bSjX0iiqcxkcGzLpzXPqVelJw3khxYOxEI%3D&reserved=0
> > )
> >
> >
> > Specifically
> >
> >
> > @JsonTypeInfo(use=Id.CLASS, include=As.PROPERTY, property="fqcn",
> > visible=false)
> > public abstract class FortEntity
> >
> > This was added to emit the fully qualified name of the class so that JSON
> deserializer can detect the underlying sub-type of FortEntity
> present in the FortRequest received by the server.
>
> For example in XML the FortRequest for searching users is represented as:
> <FortRequest>
> <entity xsi:type="user" xmlns:xsi="
>
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.w3.org%2F2001%2FXMLSchema-instance&data=02%7C01%7Cclp207%40psu.edu%7C17cfc158227245fdd4c708d6663da470%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636808812280351615&sdata=%2FpQz3mZGdXiTG5h07j1rDyUnckLbuLphNrhpYxTnKm8%3D&reserved=0
> ">
> </entity>
> <contextId>HOME</contextId>
> </FortRequest>
>
> In JSON the same request is represented as:
> {
> "entity": {
> "fqcn": "org.apache.directory.fortress.core.model.User"
> },
> "contextId": "HOME"
> }
>
> Here the "fqcn" attribute is equivalent to "xsi:type" attribute in XML
>
> >
> > now requires an id field to be added to all fortress entities when
> > serializing / de-serializing. Can someone provide insight into what
> problem
> > this is solving?
> >
> Though this field is required while deserializing, none of the model
> classes need to declare it (in fact no model classes in fortress-core were
> updated to include this) and this field is automatically included while
> serializing.
>
> This is all taken care of by the JacksonJsonProvider coupled with a custom
> field only mapper, both configured in applicationContext.xml of
> fortress-rest. This piece of code and config changes are present in the
> git branch named "FC-247". This branch is not yet merged into the
> master.
>
> >
> >
> > ~Chris P.
> >
> Kiran
>
Kiran
Re: FC-250
Posted by Shawn McKinney <sm...@apache.org>.
Maybe some code will help explain how it works.
All Fortress Rest services pass in:
https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/model/FortRequest.java
and return:
https://github.com/apache/directory-fortress-core/blob/master/src/main/java/org/apache/directory/fortress/core/model/FortResponse.java
Which then contains elements like FortEntity.
There are objects like this, perform the translation, before calling the APIs. e.g. AdminMgr:
https://github.com/apache/directory-fortress-enmasse/blob/master/src/main/java/org/apache/directory/fortress/rest/AdminMgrImpl.java
Shawn
> On Dec 20, 2018, at 9:55 AM, Kiran Ayyagari <ka...@apache.org> wrote:
>
> On Thu, Dec 20, 2018 at 9:15 PM Pike, Christopher <cl...@psu.edu> wrote:
>
>> FortEntity really just provides some common fields for every entity
>> correct (modId, modCode, sequenceId)? When do you make a request for some
>> fortress data where you don't know what type will be returned? Or does the
>> api just treat everything as a FortEntity?
>>
>> This is due to the format of FortRequest, when a FortRequest is received
> by the server in JSON format there must be a way to identify the
> concrete types of FortEntity represented by "entity" and "entity2" fields
> without which deserialization fails.
>
> Note that the original aim was to only add support for JSON data format
> without shuffling too many things. The invocation mechanism still uses
> FortRequest just like JAXB.
>
>> ________________________________
>> From: Kiran Ayyagari <ka...@apache.org>
>> Sent: Thursday, December 20, 2018 10:25:52 AM
>> To: fortress@directory.apache.org
>> Subject: Re: FC-250
>>
>> On Thu, Dec 20, 2018 at 8:51 PM Pike, Christopher <cl...@psu.edu> wrote:
>>
>>> I can certainly add appropriate code to remove the field on both ends. I
>>> was more questioning why the type information is necessary as I don't
>> have
>>> a use case where I need the type information specified in the object, so
>> it
>>> is extra unnecessary data IMO.
>>>
>> There is no other way to specify the sub-type of FortEntity in JSON format
>> for the deserialization to work.
>>
>>>
>>>
>>> Try this link
>>>
>>>
>>>
>>>
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithubcom%2Fpike1212%2Ffortress-rest-service%2Fblob%2Fmaster%2Ffortressrestapi.png&data=02%7C01%7Cclp207%40psu.edu%7C43f23ce52e8d42187d5608d6668f771b%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636809163699525418&sdata=GLLSu4JIGHRQDNDgVskDltRzW8St7HxKWC5vpKXkLr8%3D&reserved=0
>>>
>>>
>>> ________________________________
>>> From: Shawn McKinney <sm...@apache.org>
>>> Sent: Thursday, December 20, 2018 10:07:30 AM
>>> To: fortress@directory.apache.org
>>> Subject: Re: FC-250
>>>
>>>
>>>> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>>>>
>>>> • Is there a way to give me permission to accept the PRs from
>>> github?
>>>
>>> Don’t know. I’d post that question to the dev list.
>>>
>>>>
>>>> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>>>>
>>>> • Understand that the fqcn field is automatically included, but
>>> the issue is that clients and servers with different versions of the
>>> FortEntity class are now incompatible. A client with the old version
>> won't
>>> include the field, so the server will fail on de-serialization, or a
>> client
>>> with the new version expects it to be there when de-serializing, so will
>>> fail if the server doesn't provide it.
>>>
>>> We can fix the 2nd problem (client with new version) by adding the code
>> to
>>> handle the attribute to the rest component.
>>>
>>> The 1st problem is going to be tricker. How big of a problem is this,
>> i.e
>>> how many old clients are we talking about? 1, 5, 50, 500? When will
>> they
>>> update to latest?
>>>
>>> From the project perspective (rationale) we want to support JSON for
>>> obvious reasons that extend beyond the new UI Embrasure. Fortress Rest's
>>> usage of Apache CXF makes this change practically seamless, other than
>> what
>>> we’re discussing here. It’s literally flipping a few switches.
>>>
>>> There’s another option, but it’s not pretty. We can add an annotation to
>>> ignore unknown fields in the request. We’d prefer not to go there, but if
>>> there is no other way, it should be considered (as a band-aid).
>>>
>>> That’s why I asked the earlier question. Can we apply a band-aid until
>>> you patch the old clients.
>>>
>>>>
>>>> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>>>>
>>>> • It's been a while since I looked at the fortress REST api, but
>>> it seems more xml rpc that REST. I've attached a screen shot that shows a
>>> high level of how we have our REST api structured.
>>>
>>> I’d say that’s a fair characterization. Fortress REST are merely
>> XML/JSON
>>> wrapped fortress APIs. There’s a one-to-one correspondence between a
>>> Fortress REST service and API. The same entities are passed in /
>> returned
>>> as in the APIs.
>>>
>>> Your screenshot did not come through. Can you post it somewhere?
>>>
>>> Thanks,
>>> —Shawn
>>>
>>>
>>> Kiran
>>
> Kiran
Re: FC-250
Posted by Kiran Ayyagari <ka...@apache.org>.
On Thu, Dec 20, 2018 at 9:15 PM Pike, Christopher <cl...@psu.edu> wrote:
> FortEntity really just provides some common fields for every entity
> correct (modId, modCode, sequenceId)? When do you make a request for some
> fortress data where you don't know what type will be returned? Or does the
> api just treat everything as a FortEntity?
>
> This is due to the format of FortRequest, when a FortRequest is received
by the server in JSON format there must be a way to identify the
concrete types of FortEntity represented by "entity" and "entity2" fields
without which deserialization fails.
Note that the original aim was to only add support for JSON data format
without shuffling too many things. The invocation mechanism still uses
FortRequest just like JAXB.
> ________________________________
> From: Kiran Ayyagari <ka...@apache.org>
> Sent: Thursday, December 20, 2018 10:25:52 AM
> To: fortress@directory.apache.org
> Subject: Re: FC-250
>
> On Thu, Dec 20, 2018 at 8:51 PM Pike, Christopher <cl...@psu.edu> wrote:
>
> > I can certainly add appropriate code to remove the field on both ends. I
> > was more questioning why the type information is necessary as I don't
> have
> > a use case where I need the type information specified in the object, so
> it
> > is extra unnecessary data IMO.
> >
> There is no other way to specify the sub-type of FortEntity in JSON format
> for the deserialization to work.
>
> >
> >
> > Try this link
> >
> >
> >
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpike1212%2Ffortress-rest-service%2Fblob%2Fmaster%2Ffortressrestapi.png&data=02%7C01%7Cclp207%40psu.edu%7C43f23ce52e8d42187d5608d6668f771b%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636809163699525418&sdata=GLLSu4JIGHRQDNDgVskDltRzW8St7HxKWC5vpKXkLr8%3D&reserved=0
> >
> >
> > ________________________________
> > From: Shawn McKinney <sm...@apache.org>
> > Sent: Thursday, December 20, 2018 10:07:30 AM
> > To: fortress@directory.apache.org
> > Subject: Re: FC-250
> >
> >
> > > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> > >
> > > • Is there a way to give me permission to accept the PRs from
> > github?
> >
> > Don’t know. I’d post that question to the dev list.
> >
> > >
> > > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> > >
> > > • Understand that the fqcn field is automatically included, but
> > the issue is that clients and servers with different versions of the
> > FortEntity class are now incompatible. A client with the old version
> won't
> > include the field, so the server will fail on de-serialization, or a
> client
> > with the new version expects it to be there when de-serializing, so will
> > fail if the server doesn't provide it.
> >
> > We can fix the 2nd problem (client with new version) by adding the code
> to
> > handle the attribute to the rest component.
> >
> > The 1st problem is going to be tricker. How big of a problem is this,
> i.e
> > how many old clients are we talking about? 1, 5, 50, 500? When will
> they
> > update to latest?
> >
> > From the project perspective (rationale) we want to support JSON for
> > obvious reasons that extend beyond the new UI Embrasure. Fortress Rest's
> > usage of Apache CXF makes this change practically seamless, other than
> what
> > we’re discussing here. It’s literally flipping a few switches.
> >
> > There’s another option, but it’s not pretty. We can add an annotation to
> > ignore unknown fields in the request. We’d prefer not to go there, but if
> > there is no other way, it should be considered (as a band-aid).
> >
> > That’s why I asked the earlier question. Can we apply a band-aid until
> > you patch the old clients.
> >
> > >
> > > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> > >
> > > • It's been a while since I looked at the fortress REST api, but
> > it seems more xml rpc that REST. I've attached a screen shot that shows a
> > high level of how we have our REST api structured.
> >
> > I’d say that’s a fair characterization. Fortress REST are merely
> XML/JSON
> > wrapped fortress APIs. There’s a one-to-one correspondence between a
> > Fortress REST service and API. The same entities are passed in /
> returned
> > as in the APIs.
> >
> > Your screenshot did not come through. Can you post it somewhere?
> >
> > Thanks,
> > —Shawn
> >
> >
> > Kiran
>
Kiran
Re: FC-250
Posted by "Pike, Christopher" <cl...@psu.edu>.
FortEntity really just provides some common fields for every entity correct (modId, modCode, sequenceId)? When do you make a request for some fortress data where you don't know what type will be returned? Or does the api just treat everything as a FortEntity?
________________________________
From: Kiran Ayyagari <ka...@apache.org>
Sent: Thursday, December 20, 2018 10:25:52 AM
To: fortress@directory.apache.org
Subject: Re: FC-250
On Thu, Dec 20, 2018 at 8:51 PM Pike, Christopher <cl...@psu.edu> wrote:
> I can certainly add appropriate code to remove the field on both ends. I
> was more questioning why the type information is necessary as I don't have
> a use case where I need the type information specified in the object, so it
> is extra unnecessary data IMO.
>
There is no other way to specify the sub-type of FortEntity in JSON format
for the deserialization to work.
>
>
> Try this link
>
>
>
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpike1212%2Ffortress-rest-service%2Fblob%2Fmaster%2Ffortressrestapi.png&data=02%7C01%7Cclp207%40psu.edu%7C43f23ce52e8d42187d5608d6668f771b%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636809163699525418&sdata=GLLSu4JIGHRQDNDgVskDltRzW8St7HxKWC5vpKXkLr8%3D&reserved=0
>
>
> ________________________________
> From: Shawn McKinney <sm...@apache.org>
> Sent: Thursday, December 20, 2018 10:07:30 AM
> To: fortress@directory.apache.org
> Subject: Re: FC-250
>
>
> > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> >
> > • Is there a way to give me permission to accept the PRs from
> github?
>
> Don’t know. I’d post that question to the dev list.
>
> >
> > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> >
> > • Understand that the fqcn field is automatically included, but
> the issue is that clients and servers with different versions of the
> FortEntity class are now incompatible. A client with the old version won't
> include the field, so the server will fail on de-serialization, or a client
> with the new version expects it to be there when de-serializing, so will
> fail if the server doesn't provide it.
>
> We can fix the 2nd problem (client with new version) by adding the code to
> handle the attribute to the rest component.
>
> The 1st problem is going to be tricker. How big of a problem is this, i.e
> how many old clients are we talking about? 1, 5, 50, 500? When will they
> update to latest?
>
> From the project perspective (rationale) we want to support JSON for
> obvious reasons that extend beyond the new UI Embrasure. Fortress Rest's
> usage of Apache CXF makes this change practically seamless, other than what
> we’re discussing here. It’s literally flipping a few switches.
>
> There’s another option, but it’s not pretty. We can add an annotation to
> ignore unknown fields in the request. We’d prefer not to go there, but if
> there is no other way, it should be considered (as a band-aid).
>
> That’s why I asked the earlier question. Can we apply a band-aid until
> you patch the old clients.
>
> >
> > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> >
> > • It's been a while since I looked at the fortress REST api, but
> it seems more xml rpc that REST. I've attached a screen shot that shows a
> high level of how we have our REST api structured.
>
> I’d say that’s a fair characterization. Fortress REST are merely XML/JSON
> wrapped fortress APIs. There’s a one-to-one correspondence between a
> Fortress REST service and API. The same entities are passed in / returned
> as in the APIs.
>
> Your screenshot did not come through. Can you post it somewhere?
>
> Thanks,
> —Shawn
>
>
> Kiran
Re: FC-250
Posted by Kiran Ayyagari <ka...@apache.org>.
On Thu, Dec 20, 2018 at 8:51 PM Pike, Christopher <cl...@psu.edu> wrote:
> I can certainly add appropriate code to remove the field on both ends. I
> was more questioning why the type information is necessary as I don't have
> a use case where I need the type information specified in the object, so it
> is extra unnecessary data IMO.
>
There is no other way to specify the sub-type of FortEntity in JSON format
for the deserialization to work.
>
>
> Try this link
>
>
>
> https://github.com/pike1212/fortress-rest-service/blob/master/fortressrestapi.png
>
>
> ________________________________
> From: Shawn McKinney <sm...@apache.org>
> Sent: Thursday, December 20, 2018 10:07:30 AM
> To: fortress@directory.apache.org
> Subject: Re: FC-250
>
>
> > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> >
> > • Is there a way to give me permission to accept the PRs from
> github?
>
> Don’t know. I’d post that question to the dev list.
>
> >
> > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> >
> > • Understand that the fqcn field is automatically included, but
> the issue is that clients and servers with different versions of the
> FortEntity class are now incompatible. A client with the old version won't
> include the field, so the server will fail on de-serialization, or a client
> with the new version expects it to be there when de-serializing, so will
> fail if the server doesn't provide it.
>
> We can fix the 2nd problem (client with new version) by adding the code to
> handle the attribute to the rest component.
>
> The 1st problem is going to be tricker. How big of a problem is this, i.e
> how many old clients are we talking about? 1, 5, 50, 500? When will they
> update to latest?
>
> From the project perspective (rationale) we want to support JSON for
> obvious reasons that extend beyond the new UI Embrasure. Fortress Rest's
> usage of Apache CXF makes this change practically seamless, other than what
> we’re discussing here. It’s literally flipping a few switches.
>
> There’s another option, but it’s not pretty. We can add an annotation to
> ignore unknown fields in the request. We’d prefer not to go there, but if
> there is no other way, it should be considered (as a band-aid).
>
> That’s why I asked the earlier question. Can we apply a band-aid until
> you patch the old clients.
>
> >
> > On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
> >
> > • It's been a while since I looked at the fortress REST api, but
> it seems more xml rpc that REST. I've attached a screen shot that shows a
> high level of how we have our REST api structured.
>
> I’d say that’s a fair characterization. Fortress REST are merely XML/JSON
> wrapped fortress APIs. There’s a one-to-one correspondence between a
> Fortress REST service and API. The same entities are passed in / returned
> as in the APIs.
>
> Your screenshot did not come through. Can you post it somewhere?
>
> Thanks,
> —Shawn
>
>
> Kiran
Re: FC-250
Posted by "Pike, Christopher" <cl...@psu.edu>.
I can certainly add appropriate code to remove the field on both ends. I was more questioning why the type information is necessary as I don't have a use case where I need the type information specified in the object, so it is extra unnecessary data IMO.
Try this link
https://github.com/pike1212/fortress-rest-service/blob/master/fortressrestapi.png
________________________________
From: Shawn McKinney <sm...@apache.org>
Sent: Thursday, December 20, 2018 10:07:30 AM
To: fortress@directory.apache.org
Subject: Re: FC-250
> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>
> • Is there a way to give me permission to accept the PRs from github?
Don’t know. I’d post that question to the dev list.
>
> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>
> • Understand that the fqcn field is automatically included, but the issue is that clients and servers with different versions of the FortEntity class are now incompatible. A client with the old version won't include the field, so the server will fail on de-serialization, or a client with the new version expects it to be there when de-serializing, so will fail if the server doesn't provide it.
We can fix the 2nd problem (client with new version) by adding the code to handle the attribute to the rest component.
The 1st problem is going to be tricker. How big of a problem is this, i.e how many old clients are we talking about? 1, 5, 50, 500? When will they update to latest?
From the project perspective (rationale) we want to support JSON for obvious reasons that extend beyond the new UI Embrasure. Fortress Rest's usage of Apache CXF makes this change practically seamless, other than what we’re discussing here. It’s literally flipping a few switches.
There’s another option, but it’s not pretty. We can add an annotation to ignore unknown fields in the request. We’d prefer not to go there, but if there is no other way, it should be considered (as a band-aid).
That’s why I asked the earlier question. Can we apply a band-aid until you patch the old clients.
>
> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>
> • It's been a while since I looked at the fortress REST api, but it seems more xml rpc that REST. I've attached a screen shot that shows a high level of how we have our REST api structured.
I’d say that’s a fair characterization. Fortress REST are merely XML/JSON wrapped fortress APIs. There’s a one-to-one correspondence between a Fortress REST service and API. The same entities are passed in / returned as in the APIs.
Your screenshot did not come through. Can you post it somewhere?
Thanks,
—Shawn
Re: FC-250
Posted by Shawn McKinney <sm...@apache.org>.
> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>
> • Is there a way to give me permission to accept the PRs from github?
Don’t know. I’d post that question to the dev list.
>
> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>
> • Understand that the fqcn field is automatically included, but the issue is that clients and servers with different versions of the FortEntity class are now incompatible. A client with the old version won't include the field, so the server will fail on de-serialization, or a client with the new version expects it to be there when de-serializing, so will fail if the server doesn't provide it.
We can fix the 2nd problem (client with new version) by adding the code to handle the attribute to the rest component.
The 1st problem is going to be tricker. How big of a problem is this, i.e how many old clients are we talking about? 1, 5, 50, 500? When will they update to latest?
From the project perspective (rationale) we want to support JSON for obvious reasons that extend beyond the new UI Embrasure. Fortress Rest's usage of Apache CXF makes this change practically seamless, other than what we’re discussing here. It’s literally flipping a few switches.
There’s another option, but it’s not pretty. We can add an annotation to ignore unknown fields in the request. We’d prefer not to go there, but if there is no other way, it should be considered (as a band-aid).
That’s why I asked the earlier question. Can we apply a band-aid until you patch the old clients.
>
> On Dec 20, 2018, at 7:15 AM, Pike, Christopher <cl...@psu.edu> wrote:
>
> • It's been a while since I looked at the fortress REST api, but it seems more xml rpc that REST. I've attached a screen shot that shows a high level of how we have our REST api structured.
I’d say that’s a fair characterization. Fortress REST are merely XML/JSON wrapped fortress APIs. There’s a one-to-one correspondence between a Fortress REST service and API. The same entities are passed in / returned as in the APIs.
Your screenshot did not come through. Can you post it somewhere?
Thanks,
—Shawn
Re: FC-250
Posted by "Pike, Christopher" <cl...@psu.edu>.
1. Is there a way to give me permission to accept the PRs from github?
2. Understand that the fqcn field is automatically included, but the issue is that clients and servers with different versions of the FortEntity class are now incompatible. A client with the old version won't include the field, so the server will fail on de-serialization, or a client with the new version expects it to be there when de-serializing, so will fail if the server doesn't provide it.
3. It's been a while since I looked at the fortress REST api, but it seems more xml rpc that REST. I've attached a screen shot that shows a high level of how we have our REST api structured.
________________________________
From: Kiran Ayyagari <ka...@apache.org>
Sent: Thursday, December 20, 2018 12:39:50 AM
To: fortress@directory.apache.org
Subject: Re: FC-250
On Thu, Dec 20, 2018 at 1:40 AM Pike, Christopher <cl...@psu.edu> wrote:
> This commit is a potentially breaking change. (
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fdirectory-fortress-core%2Fcommit%2F2d8a53071b8d8b5bb3b6256f084a9e12d4a7cc10%23diff-280616e1526a17336cdb4ff1855d7439&data=02%7C01%7Cclp207%40psu.edu%7C17cfc158227245fdd4c708d6663da470%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636808812280341605&sdata=C%2FJEfLe02bSjX0iiqcxkcGzLpzXPqVelJw3khxYOxEI%3D&reserved=0
> )
>
>
> Specifically
>
>
> @JsonTypeInfo(use=Id.CLASS, include=As.PROPERTY, property="fqcn",
> visible=false)
> public abstract class FortEntity
>
> This was added to emit the fully qualified name of the class so that JSON
deserializer can detect the underlying sub-type of FortEntity
present in the FortRequest received by the server.
For example in XML the FortRequest for searching users is represented as:
<FortRequest>
<entity xsi:type="user" xmlns:xsi="
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.w3.org%2F2001%2FXMLSchema-instance&data=02%7C01%7Cclp207%40psu.edu%7C17cfc158227245fdd4c708d6663da470%7C7cf48d453ddb4389a9c1c115526eb52e%7C0%7C0%7C636808812280351615&sdata=%2FpQz3mZGdXiTG5h07j1rDyUnckLbuLphNrhpYxTnKm8%3D&reserved=0">
</entity>
<contextId>HOME</contextId>
</FortRequest>
In JSON the same request is represented as:
{
"entity": {
"fqcn": "org.apache.directory.fortress.core.model.User"
},
"contextId": "HOME"
}
Here the "fqcn" attribute is equivalent to "xsi:type" attribute in XML
>
> now requires an id field to be added to all fortress entities when
> serializing / de-serializing. Can someone provide insight into what problem
> this is solving?
>
Though this field is required while deserializing, none of the model
classes need to declare it (in fact no model classes in fortress-core were
updated to include this) and this field is automatically included while
serializing.
This is all taken care of by the JacksonJsonProvider coupled with a custom
field only mapper, both configured in applicationContext.xml of
fortress-rest. This piece of code and config changes are present in the
git branch named "FC-247". This branch is not yet merged into the
master.
>
>
> ~Chris P.
>
Kiran
Re: FC-250
Posted by Kiran Ayyagari <ka...@apache.org>.
On Thu, Dec 20, 2018 at 1:40 AM Pike, Christopher <cl...@psu.edu> wrote:
> This commit is a potentially breaking change. (
> https://github.com/apache/directory-fortress-core/commit/2d8a53071b8d8b5bb3b6256f084a9e12d4a7cc10#diff-280616e1526a17336cdb4ff1855d7439
> )
>
>
> Specifically
>
>
> @JsonTypeInfo(use=Id.CLASS, include=As.PROPERTY, property="fqcn",
> visible=false)
> public abstract class FortEntity
>
> This was added to emit the fully qualified name of the class so that JSON
deserializer can detect the underlying sub-type of FortEntity
present in the FortRequest received by the server.
For example in XML the FortRequest for searching users is represented as:
<FortRequest>
<entity xsi:type="user" xmlns:xsi="
http://www.w3.org/2001/XMLSchema-instance">
</entity>
<contextId>HOME</contextId>
</FortRequest>
In JSON the same request is represented as:
{
"entity": {
"fqcn": "org.apache.directory.fortress.core.model.User"
},
"contextId": "HOME"
}
Here the "fqcn" attribute is equivalent to "xsi:type" attribute in XML
>
> now requires an id field to be added to all fortress entities when
> serializing / de-serializing. Can someone provide insight into what problem
> this is solving?
>
Though this field is required while deserializing, none of the model
classes need to declare it (in fact no model classes in fortress-core were
updated to include this) and this field is automatically included while
serializing.
This is all taken care of by the JacksonJsonProvider coupled with a custom
field only mapper, both configured in applicationContext.xml of
fortress-rest. This piece of code and config changes are present in the
git branch named "FC-247". This branch is not yet merged into the
master.
>
>
> ~Chris P.
>
Kiran