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

Refactor CRUD in FedoraNodes and FedoraContent for transparent hierarchy support #354

Closed
wants to merge 6 commits into from

Conversation

Projects
None yet
2 participants
@lsitu
Copy link
Contributor

commented May 13, 2014

https://www.pivotaltracker.com/s/projects/684825/stories/71230608
My local tests showing that the CRUD operations for objects and files are working as expected now. So I think we are ready to move ahead to evaluation the transparent auto-hierarchy feature. Noted that as mentioned during the standup meeting this morning, there are several existing test cases in FedoraNodesTest and FedoraContentTest classes are failed and ignored at this time, which produce a NullPointerException.

lsitu added some commits May 7, 2014

Apply auto-hierarchy with the HierarchyConverter for objects, and fixed
the index out-of-bound exception bug in the rest webapp.
Fixed the HierarchyConverter to produce a consistency result;
Implemented to convert/revert the path for all path segments.
Removed the continue keywords, updated code flavor and corrected typos
as suggested. Refoctor codes for content files implementation.
@lsitu

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2014

Hi Andrew,
Just FYI, when I test it manually, I am using the following command to start jetty and observe the path input/output from the HierarchyConvertor and the incoming/outgoing paths for each request:
java -Dlog.fcrepo.http.api=trace -jar target/fcrepo-webapp-4.0.0-alpha-6-SNAPSHOT-jetty-console.war

@@ -40,7 +40,8 @@
* multi-part identifier to ensure efficient performance of the JCR.
*
* @author ajs6f
* @date Mar 26, 2014
* @author lsitu
* @date May 9, 2014

This comment has been minimized.

Copy link
@awoods

awoods May 14, 2014

Member

Do not change date.

}
return on(separator).join(allSegments);
String pathConverted = on(separator).join(jcrPathSegments);

This comment has been minimized.

Copy link
@awoods

awoods May 14, 2014

Member

"final"

}
return on(separator).join(concat(firstSegments, lastSegment));
String parthConverted = on(separator).join(pathSegments);

This comment has been minimized.

Copy link
@awoods

awoods May 14, 2014

Member

Should be (note "final" and typo correction):
final String pathConverted = on(separator).join(pathSegments);

if (seg == null || seg.length() == 0) {
// If the segment null or empty, must be double slash or something wrong.
// Add it as empty that will produce an slash for now?
pathSegments.add("");

This comment has been minimized.

Copy link
@awoods

awoods May 14, 2014

Member

Need to add a unit test for this case in HierarchyConverterTest.java

@awoods

This comment has been minimized.

Copy link
Member

commented May 14, 2014

The last commit in this block should be a completely different PR. It does not belong with the first five commits in this changeset.
lsitu@9b58599

@awoods

This comment has been minimized.

Make method arguments "final"

@cbeer cbeer force-pushed the fcrepo4:master branch from 42441f8 to 7dc0fcd Oct 13, 2014

@lsitu lsitu closed this Nov 6, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.