[Server] Reduce API reflection usage

[Pasquale Congiusti]

Hi folks,

while working on a feature to include filtering on server REST API, I've had some discussion with some of you about the fact that the original generic filters may be actually not used. I think we should widen the discussion to the group here.

The problem we're facing is that, according to the original design, all APIs that want to list something are inheriting from the Lister class [1]. The default implementation is making a massive usage of reflection. I've run some experiments [2] in order to confirm how much of those API parameters are used by our UI, and I can see that the only params used are the pagination ones (and those are not using reflection). There is only one special case for OpenAPI interface using the "query" parameter.

• I'd like to confirm with anybody more involved on the UI front, if indeed those parameters are useful at this moment.
• Are we all good on removing any useless generic parameter from the API?
• In particular, can we remove those ReflectiveFilterer and make any filtering/sorting need explicit of each entity?

Thanks for your collaboration.

Pasquale.

[1] https://github.com/syndesisio/syndesis/blob/master/app/server/endpoint/src/main/java/io/syndesis/server/endpoint/v1/operations/Lister.java

[2] https://github.com/syndesisio/syndesis/compare/master...squakez:experiment/remove_filter_reflection?expand=1

[Zoran Regvart]

Hi Pasquale,
I see this is not getting any traction, I did a:

grep -r --include='*.ts*' --exclude-dir=node_modules
--exclude-dir=coverage --exclude-dir=__tests__ --exclude-dir=dist
--exclude-dir=stories 'query'

in `app/ui-react`, and I found single use of the `query` parameter here:

https://github.com/syndesisio/syndesis/blob/32e8aa4d2c7c79fb9c2db9c1fe2922e696fc29cb/app/ui-react/packages/api/src/WithApiConnectors.tsx#L30

This corresponds to this server API implementation (mixed in from
Lister), which could be refactored:

https://github.com/syndesisio/syndesis/blob/32e8aa4d2c7c79fb9c2db9c1fe2922e696fc29cb/app/server/endpoint/src/main/java/io/syndesis/server/endpoint/v1/handler/connection/ConnectorHandler.java#L78

Perhaps someone more involved with the development of UI might comment
with more authority.

zoran

> --
> You received this message because you are subscribed to the Google Groups "Syndesis" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syndesis+u...@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/CAKJcvVMsA%2B1vt_cOonx7n8Rs9aS9f8y5kUFe3Tv38DBSuPbxsA%40mail.gmail.com.



--
Zoran Regvart

[Stan Lewis]

Yep meant to reply to this and it totally slipped  off my radar, sorry!

I can confirm, that parameter is not used except on the API client connector list view, and suspect that was going to have to change slightly anyways to include SOAP client connectors if they're associated with a different connector ID.

To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/CABD_Zr87SwwkTx7%2BePa-E1rpz-BWa6Na7Hz8U3DHWcriZ%2BdaTg%40mail.gmail.com.

[Pasquale Congiusti]

Thanks for your replies!

I think it should be easy to update that one with the new API, I can try myself. If there is no objection I will open a GH issue in order to describe the requirements provided here and go ahead with the development.

Regards,

Pasquale.

[Pasquale Congiusti]

Hey folks,

I'm almost done with the refactory. I have a question though. Is anybody aware of other clients consuming server API apart UI component? As the change I've done is to remove unused (at least from UI) query parameters, I prefer not to break any other known client that may be using such parameters.

Thanks in advance and regards,

Pasquale.

> To unsubscribe from this group and stop receiving emails from it, send an email to syndesis+unsubscribe@googlegroups.com.

> To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/CAKJcvVMsA%2B1vt_cOonx7n8Rs9aS9f8y5kUFe3Tv38DBSuPbxsA%40mail.gmail.com.



--
Zoran Regvart

--
You received this message because you are subscribed to the Google Groups "Syndesis" group.

To unsubscribe from this group and stop receiving emails from it, send an email to syndesis+unsubscribe@googlegroups.com.

[Zoran Regvart]

Hi Pasquale,
we have an API that's deemed as public[1], if we can remain compatible
with those that's a plus. The 2.0, being a major release allows us to
break this compatibility.

Other than the UI, there's also data virtualization that calls into
the API[2][3].

zoran

[1] https://github.com/syndesisio/syndesis/blob/master/app/server/runtime/src/main/resources/openapi-public.yaml#L18-L20
[2] https://github.com/syndesisio/syndesis/blob/master/app/dv/src/main/java/io/syndesis/dv/openshift/SyndesisConnectionMonitor.java
[3] https://github.com/syndesisio/syndesis/blob/master/app/dv/src/main/java/io/syndesis/dv/openshift/TeiidOpenShiftClient.java

>>>> > To unsubscribe from this group and stop receiving emails from it, send an email to syndesis+u...@googlegroups.com.

>>>> > To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/CAKJcvVMsA%2B1vt_cOonx7n8Rs9aS9f8y5kUFe3Tv38DBSuPbxsA%40mail.gmail.com.
>>>>
>>>>
>>>>
>>>> --
>>>> Zoran Regvart
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google Groups "Syndesis" group.

>>>> To unsubscribe from this group and stop receiving emails from it, send an email to syndesis+u...@googlegroups.com.

>>>> To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/CABD_Zr87SwwkTx7%2BePa-E1rpz-BWa6Na7Hz8U3DHWcriZ%2BdaTg%40mail.gmail.com.
>

> --
> You received this message because you are subscribed to the Google Groups "Syndesis" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to syndesis+u...@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/c205e15a-397b-423a-811f-bd6d660acb7a%40googlegroups.com.



--
Zoran Regvart

[Pasquale Congiusti]

Thanks Zoran.

Apparently neither those clients are using such parameters, so we should be fine. I fear that remaining API compatible is not an option as the main goal is to get rid of those wide open parameters (query and sort) that were making use of reflection.

Moving the API to v2 is what I was thinking about, however there is a little challenge: the way we're defining packages in server endpoint define also the version (ie, io.syndesis.server.endpoint.v1.handler.extension). I assume that we do not want to maintain different versions of the API at the same time. So, in my opinion, we should refactor removing the v1 from the package definition and just update the V1Application and V1Configuration to be Vx. Leaving the v1 version in the package would be misleading. Upgrading to v2 will let us in the same situation for a future bump.

Any thought about this? was it already discussed by any chance in the past?

Cheers,

Pasquale.

>>>> > To unsubscribe from this group and stop receiving emails from it, send an email to synd...@googlegroups.com.

>>>> > To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/CAKJcvVMsA%2B1vt_cOonx7n8Rs9aS9f8y5kUFe3Tv38DBSuPbxsA%40mail.gmail.com.
>>>>
>>>>
>>>>
>>>> --
>>>> Zoran Regvart
>>>>
>>>> --
>>>> You received this message because you are subscribed to the Google Groups "Syndesis" group.

>>>> To unsubscribe from this group and stop receiving emails from it, send an email to synd...@googlegroups.com.

>>>> To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/CABD_Zr87SwwkTx7%2BePa-E1rpz-BWa6Na7Hz8U3DHWcriZ%2BdaTg%40mail.gmail.com.
>
> --
> You received this message because you are subscribed to the Google Groups "Syndesis" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to synd...@googlegroups.com.

[Zoran Regvart]

Hi Pasquale,
I don't think we need to drive this further than it needs to be, I
think changing the API version is more destructive than modifying the
way we handle few seldomly used parameters.

zoran

> To unsubscribe from this group and stop receiving emails from it, send an email to syndesis+u...@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/syndesis/21cc3ae2-d57c-45e5-be47-efa1c26a9ef5%40googlegroups.com.



--
Zoran Regvart