Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARQGRA-345: GrapheneElement can not be used as @Root - fixed #90

Merged
merged 1 commit into from Oct 4, 2013

Conversation

jhuska
Copy link
Member

@jhuska jhuska commented Oct 3, 2013

  • GrapheneElement can be now used as field type for Page Fragment @root
  • It required couple of refactorins: I had to extract interface from
    GrapheneElement, because proxy which is created for it needs that
    interface in the list of interfaces which it should implement
  • I had to rename the implementation to GrapheneElementImpl
  • GrapheneElement is wrapped element, I had to add to the
    WrapperEnricher conditional code which would take into account the
    GrapheneElement - it is ugly bit imo only only solution right now

* GrapheneElement can be now used as field type for Page Fragment @root
* It required couple of refactorins: I had to extract interface from
  GrapheneElement, because proxy which is created for it needs that
  interface in the list of interfaces which it should implement
* I had to rename the implementation to GrapheneElementImpl
* GrapheneElement is wrapped element, I had to add to the
  WrapperEnricher conditional code which would take into account the
  GrapheneElement - it is ugly bit imo only only solution right now
@buildhive
Copy link

Object wrapper;
try {
wrapper = createWrapper(grapheneContext, field.getType(), WebElementUtils.findElementLazily(rootBy, localSearchContext));
Class<?> type = field.getType().equals(GrapheneElement.class) ? GrapheneElementImpl.class : field.getType();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are the ugly parts. IMHO it can be refactored with DI somehow. Personally I would leave that to the time of resolving:
ARQGRA-302 - page fragments injectable by interfaces

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it comes to idea of @ImplementedBy - this method should allow us to use single interface for instantiating classes by their types, e.g. it's API:

Instantiator#instantiate(Class<?> clazz);

We use custom approach in several API classes, because their respective implementations are implemented in Impl module, e.g.:

https://github.com/arquillian/arquillian-graphene/blob/master/api/src/main/java/org/jboss/arquillian/graphene/context/GrapheneContext.java#L108

It would be beneficial to use single interface which will allow to mark an interface with @ImplementedBy which refers to the implementation type name or the class (which is more refactoring-prone).

@lfryc lfryc merged commit 27d4c73 into arquillian:master Oct 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants