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
Translate CND to RDFS, and expose it over the REST API #101
Conversation
Looks good-- do you want to hybridize with the ontology I did earlier? |
This is a great addition. What are your concerns? and what do you think needs to be added to make it ready for primetime? |
Blocked by MODE-1997. Expecting MODE 3.4 to land soon. |
+1 to @awoods' praise. This is exactly the direction we should be going. |
@awoods I'd prefer if we kept these as two separate commits. The type-safe iterator has utility beyond the JCR => RDFS translation. |
@ajs6f can you review the RdfStream work? Is it in line with your thinking? (also, I left the Namespaced => Resource logic in my class for now. We can break it out after you land your refactor, i think) |
@cbeer, sounds good. Are you associating this branch with both of the following tickets: |
@awoods nope. #53804961 has been complete and in master for some time. |
Looks okay. I would have done a few things differently: Instead of building up Collections, I would have introduced Functions and used transform() and our ZippingIterator. That would make things lazier. I would be using the @mock annotation on fields a bit in the tests, to cut out some repetition. I would have done the translation of types with a static Map and Function, and not a switch. See: I don't think any of these are crucial. |
I'll take a stab at the ZippingIterator. I thought about it and decided I wanted to get it implemented first. Going back shouldn't be too hard. I think I've @mocked as much as I can (unless there's some annotation magic that'd let me do the extra interfaces piece.. maybe I should look harder.) |
Sorry, @cbeer, I spoke vaguely. I didn't mean you could mock more, I meant you could mock once (as opposed to a couple places where you seem to be recreating the same mock). But I could also be misreading some of that. Let me know if you want any help with the ZippingIterator. Just remember, it's all about a pair of iterators, one of stuff, one of functions on that kind of stuff, and what you get back is an iterator of the stuff with the functions applied. |
@Component | ||
@Scope("prototype") | ||
@Path("/fcr:nodeTypes") | ||
public class FedoraRepositoryNodeTypes extends AbstractResource { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minimally, please add a class description and @author javadoc annotation.
@cbeer : ItemDefinitionToTriplesTest shows multi-interface mocking via @mock. I really prefer the annotations (see @Injectmocks in that same test). But I'm okay either way. More interestingly, I don't actually see any unit tests in the kernel for the new stuff in the kernel. Did I miss that, or are you getting coverage only from the tests on HTTP endpoints? |
@ajs6f Clever. Works for me. The stuff in kernel is covered by NodeTypeRdfContextTest (and NodeTypeIteratorTest, of course). Testing the Functions individually seemed underwhelming |
@cbeer : Somehow, I completely failed to notice that guy (NodeTypeRdfContextTest)! Sorry. If it's okay by you, I'll do a little work with him to use the MockitoAnnotations and push that. |
+1 |
@cbeer, looks like the build is failing on checkstyle violations. |
Translate CND to RDFS, and expose it over the REST API
Resolve: https://www.pivotaltracker.com/story/show/57813074